diff options
| -rw-r--r-- | docs/comment-deletion-spec.org | 121 | ||||
| -rw-r--r-- | docs/issue-sort-order-spec.org | 128 | ||||
| -rw-r--r-- | docs/labels-as-org-tags-spec.org | 88 | ||||
| -rw-r--r-- | docs/multi-account-spec.org | 178 |
4 files changed, 411 insertions, 104 deletions
diff --git a/docs/comment-deletion-spec.org b/docs/comment-deletion-spec.org new file mode 100644 index 0000000..0af6f13 --- /dev/null +++ b/docs/comment-deletion-spec.org @@ -0,0 +1,121 @@ +#+TITLE: pearl — Delete Current Comment Spec +#+AUTHOR: Craig Jennings +#+DATE: 2026-05-25 +#+STARTUP: showall + +* Status + +*Ready — reviews incorporated through round 3, live API verified (Codex + Craig, 2026-05-25).* The one gate (the =commentDelete= contract) is resolved by a live check — see Resolved API Facts. The dirty-local-delete policy is final (allow + discard-local-edits wording). No open questions remain; implementation can start. Modified recommendations are in Review Dispositions. + +Companion to [[file:issue-comment-editing-spec.org][issue-comment-editing-spec.org]] (the comment surface, viewer identity, and per-comment drawer this builds on). + +* Problem + +Pearl can render, create, and edit comments, but it can't delete one. The comment surface is =pearl-add-comment= (create), =pearl-edit-current-comment= (update your own), and the rendered headings — there is no delete. Removing a comment means leaving Emacs for the Linear web app. The Stage 2 keymap even leaves a natural empty slot for it (=C-; L d c=, "delete comment", beside =d t= "delete ticket"). + +* Current State + +- *Comments render* as headings under the =**** Comments= subtree, each carrying a drawer: =LINEAR-COMMENT-ID=, =LINEAR-COMMENT-AUTHOR-ID=, =LINEAR-COMMENT-SHA256=. +- *Ownership* is decided by =pearl--comment-editable-p= (author-id = viewer-id; a nil/empty author-id, meaning a bot or external comment, is never editable). The viewer is resolved by =pearl--viewer-async= and cached in =pearl--cache-viewer=. +- *Edit* (=pearl-edit-current-comment=) reads =LINEAR-COMMENT-ID= via =pearl--goto-heading-or-error=, refuses a non-own comment with no delete/update mutation, and pushes via =pearl--update-comment-async= (=commentUpdate=). It also computes a local-dirty check: =hash(pearl--org-to-md body) /= LINEAR-COMMENT-SHA256=. +- *Issue delete* (=pearl-delete-current-issue=) is the shape to follow: capture a marker, confirm with =yes-or-no-p=, fire =pearl--delete-issue-async=, and on success remove the subtree (=org-back-to-heading= -> =org-end-of-subtree= -> =delete-region=) and surface the buffer. +- *No comment delete exists* — no command, no =commentDelete= mutation, no binding. The keymap test currently asserts =C-; L d c= is intentionally unbound. + +* Resolved API Facts (live check, 2026-05-25) + +Verified against the live workspace by creating a throwaway issue + comment, running the raw mutation, and querying the comment back: + +- *Mutation:* =commentDelete(id: String!)= returns a payload with =success=. The live call returned ={"data":{"commentDelete":{"success":true}}}=. So =pearl--delete-comment-async= mirrors =pearl--delete-issue-async= exactly: callback with =(:success BOOL)=, error branch -> =(:success nil)=. +- *Recoverability:* querying =comment(id){ ... }= immediately after the delete returned ="Entity not found: Comment"= (=INPUT_ERROR=, 400) — *not* the soft-archive the published-schema =Comment.archivedAt= field had suggested. There is *no API restore path*: once deleted, the comment is gone from the API and Pearl cannot bring it back. (Linear's web UI may offer a momentary undo toast; that is not reachable from Pearl.) +- *Consequence:* undo/restore from Pearl is *impossible*, not merely deferred (it moves from "vNext if recoverable" to permanently out of scope). The prompt does not promise recoverability. +- *Permission / not-found behavior* was not exercised destructively beyond the own-comment success path; the permission gate (own-only, below) prevents a non-own =commentDelete= call in the first place, and a not-found id returns the same error shape, handled as =(:success nil)=. + +* Proposed Design + +** Command: =pearl-delete-current-comment= + +Runs from the comment heading or its body text before any child heading (the reach of =pearl--goto-heading-or-error=, matching =pearl-edit-current-comment=; it does not climb out of a nested Org heading authored inside the comment body — see Review Dispositions MP2). The flow: + +1. *Locate the comment.* =pearl--goto-heading-or-error=, then read =LINEAR-COMMENT-ID=. If absent, =user-error= "Not on a Linear comment" with no mutation. +2. *Permission gate (no delete mutation for non-own).* Resolve the viewer (=pearl--viewer-async=, reusing the cache; a cold cache costs one read-only lookup, not a mutation). If viewer resolution fails, refuse with no =commentDelete= call and the message "Could not determine your Linear identity; not deleting". If =LINEAR-COMMENT-AUTHOR-ID= is not the viewer (=pearl--comment-editable-p= nil), refuse with no =commentDelete= call ("You can only delete your own comments"). This matches edit's permission model. +3. *Local-dirty check, then confirm.* Compute =hash(pearl--org-to-md body)= against =LINEAR-COMMENT-SHA256=. Match (clean) -> standard wording. Differ (the user edited the body and is deleting instead of saving) -> the stronger discard-local-edits wording. *Missing* =LINEAR-COMMENT-SHA256= (a hand-edited or partially generated comment with no stored hash) -> treat as unknown provenance and take the stronger wording too, since the local state can't be confirmed clean. Delete proceeds on confirmation in every case (decision 2). =yes-or-no-p=, mirroring =pearl-delete-current-issue=. +4. *Delete.* Fire =pearl--delete-comment-async= (=commentDelete=). On =:success=, remove the comment's Org subtree (=org-back-to-heading= -> =org-end-of-subtree t t= -> =delete-region=) and surface the buffer. On failure, message and leave the subtree intact. + +No three-way conflict gate. A delete is unconditional against the remote — it doesn't merge, so remote drift since fetch doesn't change the outcome. The permission gate, the local-dirty wording, and the confirmation are the guards. + +*Prompt wording* (final, given the resolved recoverability) always names both the remote effect and the local removal: +- clean comment: "Delete this comment from Linear and remove it here?" +- dirty or unknown-provenance comment: "Delete this comment from Linear and discard your local edits here?" +Plain "Delete" rather than "Permanently delete": Pearl has no restore path (Resolved API Facts), but we don't claim permanence Linear's own UI may not honor. "remove it here" + the no-undo reality is the honest threshold. +Both name Linear and the local effect, so neither the remote delete nor a discarded local edit can be missed. + +** Async mutation: =pearl--delete-comment-async= + +Mirrors =pearl--delete-issue-async= and =pearl--update-comment-async=: =commentDelete(id: String!) { success }= (verified — see Resolved API Facts), callback with =(:success BOOL)=, error branch -> =(:success nil)=. + +** Callback safety + +The local removal operates on a marker captured *before* the async dispatch. The success callback checks the marker's buffer is still live and does nothing (no signal) if it was killed before the callback returned. At most one viewer lookup and one delete mutation per invocation. + +** Immediate, not part of the save model + +Delete is an immediate mutation, like the field setters and =pearl-delete-current-issue=. It does not fold into =pearl-save-issue= / =pearl-save-all=. It surfaces under the delete prefix and the transient for discoverability only. + +** Surfaces + +- *Keymap (Stage 2 slot):* =C-; L d c= -> =pearl-delete-current-comment=, label "delete comment", beside =d t= "delete ticket". The keymap test that asserts =d c= is unbound is inverted when this ships. +- *Transient:* =K= "delete comment" under the Delete group (=k= is "delete ticket"; =K= pairs with it). + +* Agreed Decisions + +1. Own-only, mirroring =pearl-edit-current-comment='s permission model; a non-own / bot / external comment is refused with no =commentDelete= mutation. +2. Dirty local comment (HP2 — *final*, Craig 2026-05-25): *allow* the delete, switching the confirmation to discard-local-edits wording when =hash(org->md body) /= LINEAR-COMMENT-SHA256= or the hash is missing (unknown provenance). This matches the immediate-delete model rather than refusing. (The rejected alternative was to refuse dirty comments and tell the user to save/revert/refresh first.) +3. "No delete mutation for non-own" is the precise guarantee, not "no network" — a cold viewer cache costs one read-only lookup (MP1). +4. Point reach is the comment heading or its pre-child body, matching current helpers; the ancestor-climbing helper is vNext (MP2). +5. Transient key =K=; keymap slot =C-; L d c= (MP3). + +* Open Questions (Craig) + +None — all resolved. The last gate (the live =commentDelete= contract) was verified 2026-05-25; the dirty-delete policy (decision 2) and the transient key =K= were already settled. + +* Files Touched + +- =pearl.el=: =pearl-delete-current-comment= (command), =pearl--delete-comment-async= (mutation), the transient =K= entry, the =C-; L d c= keymap binding. +- =docs/=: this spec. +- =README.org=: the comment section gains "delete your own comments". +- =tests/=: =tests/test-pearl-comment-deletion.el= (or additions to =test-pearl-delete.el=). + +* Test Plan + +- =pearl--delete-comment-async= success parses the =commentDelete= success payload; a false / GraphQL-error body returns =(:success nil)=. +- Not on a comment / missing =LINEAR-COMMENT-ID= -> =user-error=, no delete mutation. +- Own comment + confirmation accepted + success -> removes only that comment's subtree; sibling comments and the issue body intact. +- Confirmation declined -> no delete mutation, subtree intact. +- Non-own and bot/external comments -> no delete mutation (permission gate). +- Delete mutation failure (=:success nil=) -> subtree intact, failure reported. +- Dirty local comment -> the discard-wording confirmation path (decision 2); a clean comment -> the standard wording. The dirty prompt names both "Linear" and the local-edit discard. +- Missing =LINEAR-COMMENT-SHA256= -> takes the stronger (unknown-provenance) prompt, not the clean path. +- Viewer resolution failure -> the exact refusal message "Could not determine your Linear identity; not deleting", no delete mutation. +- Killed-buffer / lost-marker safety -> if the marker's buffer is gone when the callback fires, no signal and no attempt to surface a dead buffer. +- Keymap: =C-; L d c= binds =pearl-delete-current-comment= (inverts the current "no delete comment yet" assertion); transient contains the =K= delete-comment entry. +- Live-verified 2026-05-25: =commentDelete(id){success}= returned =success: true=, and the comment was not-found immediately afterward (no restore). The only remaining manual check is that a real own-comment delete removes the *right* comment subtree in a live buffer. + +* Review Dispositions + +*Round 1 (Codex, 2026-05-25).* Rubric =Needs research=. Accepted as written: HP1 (verify =commentDelete= before coding — recast as a blocking Implementation Prerequisite, with the partial published-schema finding recorded), MP1 ("no delete mutation for non-own", reworded throughout), MP3 (finalize =K= + the keymap slot), and the architecture / robustness / test-strategy / UX observations (callback-safety rule, prompt naming both effects, the expanded test plan). Two were modified: + +- *HP2 (local dirty comment) — modified to a choice.* The review offered two coherent options (allow-with-discard-wording, or refuse-dirty). Chose allow-with-discard-wording as the v1 decision because it matches the immediate-delete model — an explicit delete is already destructive, so discarding an unsaved body edit with clear wording is consistent, where refusing would make delete the one comment command that blocks on dirtiness. Flagged for Craig's confirmation since it's a data-loss call. +- *MP2 (point location) — modified to scope.* Adopted the narrow promise (reach = the comment heading or its pre-child body, matching =pearl--goto-heading-or-error=) rather than building the ancestor-climbing helper now. The shared climbing helper is recorded as a vNext candidate; building it here would widen the change beyond a thin delete command for a UX (nested headings inside a comment body) that isn't in use. + +Everything else accepted as written. + +*Round 2 (Codex, 2026-05-25).* Rubric still =Needs research= — HP1 (the live =commentDelete= check) is the one hard gate and stays a named prerequisite, unchanged. HP2 (the dirty-delete decided-and-also-open contradiction the response introduced) is resolved: Craig finalized allow-with-discard-wording, so it's now a settled decision and out of Open Questions. MP1 (prompt names both Linear and the local discard), MP2 (missing =LINEAR-COMMENT-SHA256= = unknown provenance -> stronger prompt + test), and MP3 (restore the exact viewer-failure message + test) accepted as written. No modifications or rejects this round. + +*Round 3 (Codex, 2026-05-25).* Rubric =Needs research=, sole blocker HP1 (the live =commentDelete= check), no other edits requested. Resolved by running the live check: created a throwaway issue + comment, ran =commentDelete= (=success: true=), confirmed the comment is not-found afterward (no API restore), and removed the test issue. Folded the verified contract into Resolved API Facts, finalized the prompt verb (plain "Delete"), and moved undo/restore from conditional-vNext to permanently out of scope. Rubric -> =Ready=. + +* Out of Scope + +- Bulk comment deletion. +- Undo / restore of a deleted comment from Emacs — *impossible*, not deferred: the live check found no API restore path (the comment is not-found immediately after =commentDelete=). +- A shared "find enclosing comment heading" helper that works from nested Org headings inside a comment body (vNext; would be shared with =pearl-edit-current-comment=). +- Deleting comments authored by others (Linear permissions forbid it; the gate enforces own-only). diff --git a/docs/issue-sort-order-spec.org b/docs/issue-sort-order-spec.org index f49e869..478b074 100644 --- a/docs/issue-sort-order-spec.org +++ b/docs/issue-sort-order-spec.org @@ -5,61 +5,127 @@ * Status -*DRAFT — design proposal; nothing in =pearl.el= has changed.* Open questions for Craig at the end. +*Reviews incorporated through round 2; rubric =Ready* (Craig, 2026-05-25).* The first draft's "reparse the issue subtrees and rewrite" re-sort would have discarded unsaved edits; this revision makes client-side sort *move whole subtrees by =LINEAR-ID= byte-for-byte*, makes header persistence atomic with the reorder, and pins down Custom View behavior. The three caveats (Custom View refuse-server-sort in v1, toggle default =updated desc=, title-sort uses the visible heading) are *adopted as final v1 decisions*. Modified recommendations are in Review Dispositions. Companion to [[file:issue-query-spec.org][issue-query-spec.org]], which defines the =:sort= / =:order= the saved-query layer already supports. This doc covers changing the order of the *current* view interactively, without hand-editing a saved query. * Problem -v1 supports =:sort= (=updated= / =priority= / =title=) and =:order= (=asc= / =desc=) on saved queries. But to change how the active file is ordered, the user has to edit =pearl-saved-queries= (or the source plist) by hand and re-run. There's no "sort this view by priority, descending" command. For a view you're actually looking at, that's the natural thing to want. +v1 supports =:sort= (=updated= / =priority= / =title=) and =:order= (=asc= / =desc=) on saved queries. But to change how the active file is ordered, the user has to edit =pearl-saved-queries= (or the source plist) by hand and re-run. There's no "sort this view by priority, descending" command for a view you're actually looking at. * Current state -- =pearl--sort-issues= (query spec) applies =:sort= / =:order= client-side at the render boundary, so a refresh always lays headings out the same way. -- =:sort= = =priority= / =title= are client-side; =created= / =updated= map to the server =orderBy= (the only fields Linear's API orders on). The query spec documents this split. -- The active file's =#+LINEAR-SOURCE:= header records the source plist, including any =:sort= / =:order=, so =refresh-current-view= reproduces the ordering. +- =pearl--sort-issues= applies =:sort= / =:order= client-side at the render boundary, so a refresh always lays headings out the same way. +- =:sort= = =priority= / =title= are client-side; =created= / =updated= map to the server =orderBy= (the only fields Linear's API orders on). The saved-query layer stores =:sort= / =:order= as *symbols*. +- The active file's =#+LINEAR-SOURCE:= header records the source plist; =pearl--update-source-header= currently rewrites the count/timestamp fields but intentionally leaves the rest untouched, so there's no header-replace helper yet. +- The merge-refresh path deliberately protects dirty subtrees (never overwrites a subtree with unpushed edits) — the same data-loss concern applies to any in-place reorder. +- *Custom Views:* the =customView(id){ issues }= query carries *no* =orderBy= argument, and =pearl--query-view-async= has no order parameter. A view's order is whatever the server returns for that view. * Proposed design -** The command +** The commands -=pearl-set-sort= (interactive), run in the active file: +=pearl-set-sort= (interactive, in the active file): =completing-read= the sort key (=updated= / =created= / =priority= / =title=), then the order (=asc= / =desc=); update the recorded source; apply the new order. =pearl-toggle-sort-order= flips =asc=/=desc= on the current sort and re-applies. (Transient placement deferred — a plain =M-x= is enough for v1; the menu keys are unsettled, tracked under the transient-review task.) -1. =completing-read= the sort key: =updated=, =created=, =priority=, =title=. -2. =completing-read= (or a toggle) the order: =asc= / =desc=. -3. Update the =:sort= / =:order= in the active file's recorded =#+LINEAR-SOURCE:=. -4. Re-order the view (see below). +** Completion and the symbol contract (MP4) -A =pearl-toggle-sort-order= convenience command just flips =asc=/=desc= on the current sort and re-orders. Both go on the transient menu (a small "Sort" group, or under View). +Completion *displays* strings but the source stores *symbols*: =updated= / =created= / =priority= / =title= and =asc= / =desc=. Interactive input is coerced string -> symbol before it touches the source. An unknown =:sort= / =:order= already in a source produces a =user-error= rather than silently mis-sorting, so the header stays compatible with existing =:sort priority :order asc= data. -** Re-order in place vs refetch +** Sort-key sources — sort what you're looking at (MP1) -The split matters for whether a sort change needs the network: +Client-side keys read the *current local buffer state*, not a refetch: -- *Client-side sorts* (=priority=, =title=): the issues are already in the buffer. Re-sort in place — reparse the issue subtrees, reorder them, rewrite. No refetch. Fast, works offline. -- *Server-side sorts* (=created=, =updated=): the ordering comes from the server =orderBy=, and the fetch may have been truncated at the page cap, so the correct order needs a refetch with the new =orderBy=. Re-run the source (the =refresh-current-view= path) with the updated sort. +- =priority= from the heading's priority cookie / =LINEAR-PRIORITY= drawer as currently displayed; +- =title= from the current heading title, stripped of TODO keyword / priority cookie / tags / identifier the same way title sync strips it (so an unsaved local title edit sorts by what you see). -So =set-sort= decides: client-side key → re-sort the buffer; server-side key → refetch. The command reports which it did. +** Re-order in place: move whole subtrees, never reconstruct (HP1) -** Persistence +Client-side sort (=priority=, =title=) *moves existing issue subtrees*; it never re-renders them from parsed data (that path, via =pearl--format-issue-as-org-entry=, would discard unsaved description/comment edits and normalize user text). The command: -The change updates the active file's =#+LINEAR-SOURCE:= so a later =refresh-current-view= keeps the new order. Whether it also writes back to a named saved query in =pearl-saved-queries= is open question 3 — my lean is no by default (the active file is the scratch view; saved queries are the durable definitions), with an explicit "save this ordering to the query" as a separate step. +1. identifies the top-level issue subtrees (each with a =LINEAR-ID=) under the single parent heading; +2. computes each one's sort key from its current org data (above); +3. reorders the subtree *regions* and rewrites them *byte-for-byte* — descriptions, comments, provenance drawers, and any local edits survive unchanged; +4. leaves a non-issue / malformed subtree (no =LINEAR-ID=) in a defined position (sorted last, stable) rather than dropping it. + +This works on a dirty buffer precisely because it preserves text. A test edits a description and a comment locally, sorts, and asserts the exact edited text is intact afterward. + +** Server-side sort and Custom Views (HP2) + +Server-side keys (=created=, =updated=) come from the server =orderBy=, and the fetch may have been truncated at the page cap, so the correct order needs a refetch with the new =orderBy=: + +- *Filter / saved-query / my-open sources:* re-run the source through the refresh path with the updated =orderBy= (=pearl--query-issues-async= already accepts =order-by=). +- *Custom View sources:* =customView.issues= has no verified =orderBy= in the API or the code, so v1 *refuses* a =created=/=updated= sort on a Custom View with a clear message ("Linear does not support server-side ordering for Custom Views; sort by priority or title to reorder the fetched issues"). Client-side keys (=priority=/=title=) still work on a view — they reorder the fetched (possibly truncated) page in place, the same subtree-move as any other source. (Verifying and adding =customView.issues(orderBy:)= is a vNext research item.) + +** Atomic header persistence (HP3) + +=#+LINEAR-SOURCE= is updated *only after* the reorder actually succeeds, so the header never claims an order the buffer doesn't show (which a later =refresh-current-view= would otherwise reproduce): + +- client-side: write the header only after the subtree move completes; +- server-side: pass the updated source into the fetch; write the header only as part of a successful render/merge; +- on a failed refetch or a dirty-buffer refusal: leave the old header unchanged and message that the sort was not applied. + +This needs a small helper to replace the =:sort=/=:order= in the =#+LINEAR-SOURCE= line (=pearl--source-with-sort= builds the updated plist; a header-replace helper writes it), distinct from today's count/timestamp-only =pearl--update-source-header=. + +** Toggle with no current sort (MP2) + +=pearl-toggle-sort-order= on a source with no =:sort=/=:order= defaults to =updated desc=, then toggles to =updated asc= and back. (Confirm the default.) + +** Outcome messages (UX) + +The command names exactly what it did: "Sorted current buffer by priority ascending" (client-side), "Refetched My open issues ordered by updated descending" (server-side), "Could not sort: this file has no LINEAR-SOURCE header", "Could not sort a Custom View by created; Linear has no server-side ordering for views". * Proposed v1 decisions (this feature) -1. =pearl-set-sort= + =pearl-toggle-sort-order=, both on the transient menu. -2. Client-side keys re-sort the buffer in place; server-side keys refetch. -3. The change updates the active file's recorded source so refresh preserves it. -4. Completion is over the known keys/orders, never free text. +1. =pearl-set-sort= + =pearl-toggle-sort-order= (transient placement deferred). +2. Completion displays strings, the source stores symbols; an unknown source value is a =user-error= (MP4). +3. Client-side sort moves whole issue subtrees by =LINEAR-ID=, byte-for-byte, preserving local edits — never reconstructs from parsed data (HP1). +4. Client-side keys read current buffer state (priority cookie/drawer, stripped heading title) (MP1). +5. Server-side keys refetch with the new =orderBy= for filter/saved-query/my-open sources; Custom Views *refuse* server-side sort in v1 but allow client-side reorder of the fetched page (HP2). +6. =#+LINEAR-SOURCE= is written only after a successful reorder/refetch; failure leaves it unchanged (HP3). +7. =toggle-sort-order= with no sort defaults to =updated desc= (MP2). +8. Active-header only — no write-back to =pearl-saved-queries= in v1 (MP3). -* vNext / out of scope +* Resolved decisions + +The three caveats are final: + +1. *Custom View server sort* (decision 5): refuse =created=/=updated= on a view in v1; client-side keys still reorder the fetched page. Verifying =customView.issues(orderBy:)= is vNext. +2. *Toggle default* (decision 7): =updated desc= when no sort is set. +3. Client-side =title= sort uses the visible heading title, including an unsaved local edit ("sort what I see"). + +* Files touched + +- =pearl.el=: =pearl-set-sort= + =pearl-toggle-sort-order=; pure helpers =pearl--source-with-sort= (updated source plist) and a sort/order symbol validator/coercer; a =#+LINEAR-SOURCE= replace helper; a client-side subtree-region-move helper (keyed by =LINEAR-ID=, computing keys from buffer state); the server-side re-run wired through the refresh path with the Custom View refusal. +- =docs/=: this spec. +- =README.org=: a short "sorting the current view" note. + +* Test plan -- Multi-key sort (e.g. priority then updated). -- Per-heading manual reordering that sticks across refresh. -- Exposing the full Linear =orderBy= surface if the API later un-gates the =[INTERNAL]= per-field sort. +- =pearl--source-with-sort= updates / normalizes =:sort= and =:order= (symbols); an unknown value errors. +- Missing =#+LINEAR-SOURCE= refuses with the named message. +- Client-side priority/title sort moves whole subtrees and *preserves unsaved description and comment edits* (assert exact edited text after sorting). +- Client-side sort updates =#+LINEAR-SOURCE= only after the move succeeds. +- Server-side filter source passes the right =orderBy= + updated source to the refresh/render path. +- Server-side fetch failure leaves the source header unchanged and messages "not applied". +- Custom View + =created=/=updated= refuses with the view message; Custom View + =priority=/=title= reorders the fetched page in place. +- =toggle-sort-order= with no sort/order applies the =updated desc= default. +- Active-header sort change does not persist into =pearl-saved-queries=. +- Malformed / non-issue subtree sorts last, stable, not dropped. -* Open questions for Craig +* Review Dispositions + +*Round 1 (Codex, 2026-05-25).* Rubric =Not ready=. Accepted as written: HP1 (move whole subtrees byte-for-byte rather than reparse/rewrite — the core safety fix), HP3 (atomic header persistence with rollback on failure), MP1 (client-side keys read current buffer state), MP2 (toggle default =updated desc=), MP3 (active-header only, no saved-query write-back in v1), MP4 (string display / symbol storage, unknown -> user-error), and the UX outcome messages, architecture helper shape, and expanded test plan. One was modified: + +- *HP2 (Custom View server sort) — modified to a decision.* The reviewer offered "verify and implement =customView.issues(orderBy:)=" or "refuse server-side sort on views". Chose refuse-in-v1: =customView.issues= has no verified =orderBy= in the API or the code, and a view already carries its own server-defined order, so adding an unverified ordering argument is out of proportion to a v1 sort command. Client-side keys still reorder the fetched page on a view. Verifying =customView.issues(orderBy:)= is recorded as a vNext research item. + +Everything else accepted as written. + +*Round 2 (Codex, 2026-05-25).* Rubric =Ready with caveats=; no new findings — MP1-MP3 were each "confirm decision X" (Custom View refuse-server-sort, toggle default =updated desc=, title-sort-uses-visible-heading), plus a test nudge (a malformed non-issue subtree fixture, already in the test plan). Resolved by adopting all three as final v1 decisions (latitude granted by Craig). Rubric -> =Ready=. + +* vNext / out of scope -1. *Command vs transient-only*: a plain =M-x pearl-set-sort= with two completing-reads, or a dedicated transient sub-menu with one-key sort toggles (=p= priority, =u= updated, =t= title, =o= flip order)? The transient reads faster for a frequent action. -2. *In-place re-sort fidelity*: re-sorting client-side means reparsing and rewriting issue subtrees in the buffer. Acceptable, or prefer always-refetch for simplicity even when a client-side sort wouldn't need it? -3. *Write-back*: should changing the sort offer to persist it to the originating saved query, or only ever update the active file's header? +- Verify + implement =customView.issues(orderBy:)= for server-side sorting of Custom Views. +- Saved-query write-back for sort/order (a separate "save this ordering to the query" step). +- A dedicated sort transient with one-key sort choices. +- Multi-key sort (priority then updated). +- Manual per-heading reordering that survives refresh. diff --git a/docs/labels-as-org-tags-spec.org b/docs/labels-as-org-tags-spec.org index a1bb413..cc15e1c 100644 --- a/docs/labels-as-org-tags-spec.org +++ b/docs/labels-as-org-tags-spec.org @@ -5,7 +5,7 @@ * Status -PROPOSED — awaiting review. Implements the =todo.org= task "Org tags should reflect the issue's Linear labels" (filed 2026-05-24 after a hardcoded personal =#+filetags:= value was removed from the header writer in 952cfe7). +*Review incorporated (Codex, 2026-05-25); rubric =Ready with caveats= -> ready.* The review's caveats — collision semantics, manual-edit overwrite rules, and the =pearl-set-labels= heading-rewrite contract — are folded into the body, and the two open questions are resolved as v1 decisions (preserve Unicode alnum in tags; Pearl owns the entire issue-heading tag set). Implements the =todo.org= task "Org tags should reflect the issue's Linear labels" (filed 2026-05-24 after a hardcoded personal =#+filetags:= value was removed in 952cfe7). Modified recommendations: none — everything accepted; see Review Dispositions. * Problem @@ -15,67 +15,107 @@ Pearl used to stamp a hardcoded personal =#+filetags:= value on every fetched fi - =pearl--format-issue-as-org-entry= renders the heading as =** <STATE> [#P] <title>= with no tags, and writes the labels into the =:LINEAR-LABELS:= drawer as =[name, name]=. - =pearl--normalize-issue= gives each issue =:labels= as a list of plists with =:name=. -- =pearl--issue-title-at-point= already extracts the title with =(org-get-heading t t t t)= — the four flags strip the keyword, priority cookie, *tags*, and comment markers. So title sync is already tag-aware; adding heading tags will not corrupt the title round-trip. -- =pearl-set-labels= changes an issue's labels (completing-read over team labels), pushes, and rewrites the =:LINEAR-LABELS:= drawer. +- =pearl--issue-title-at-point= extracts the title with =(org-get-heading t t t t)= — the four flags strip the keyword, priority cookie, *tags*, and comment markers. So title sync is already tag-aware; adding heading tags won't corrupt the title round-trip. +- =pearl-set-labels= changes an issue's labels (completing-read over team labels), pushes, and rewrites the =:LINEAR-LABELS:= drawer (only the drawer today). * Proposed design ** Tag slugify: label name → org tag -A new pure helper =pearl--label-name-to-tag=. Org tags are restricted to =[[:alnum:]_@#%]= — notably *no hyphens or spaces* (unlike TODO keywords, which allow hyphens). So this slugify differs from the keyword one: +A new pure helper =pearl--label-name-to-tag=. Org tags allow =[[:alnum:]_@#%]= — notably *no hyphens or spaces* (unlike TODO keywords, which allow hyphens). The rule: - Downcase (tags are case-sensitive in Org; lowercase is the convention). - Replace each run of characters outside =[[:alnum:]_]= with a single =_=. - Trim leading/trailing =_=. -Examples: =Bug= → =bug=, =Needs Review= → =needs_review=, =P1= → =p1=, =backend/api= → =backend_api=, =UI/UX= → =ui_ux=. A label that slugifies to empty is dropped. +=[[:alnum:]]= in Emacs matches *Unicode* letters and digits, not just ASCII (MP1 decision): non-ASCII alphanumerics are *preserved*, downcased, since the rest of Pearl already carries label names as Unicode strings. Examples: =Bug= → =bug=, =Needs Review= → =needs_review=, =P1= → =p1=, =backend/api= → =backend_api=, =UI/UX= → =ui_ux=, =naïve café= → =naïve_café=. A label that slugifies to empty (punctuation-only) is dropped *from the heading* but stays in the drawer (MP2). -(Contrast with =pearl--state-name-to-keyword= from the workflow-states spec, which upcases and uses hyphens. Two different targets, two different slugify rules — keep them as separate, clearly-named helpers.) +(Contrast =pearl--state-name-to-keyword= from the workflow-states spec, which upcases and uses hyphens. Two targets, two slugify rules — separate, clearly-named helpers.) + +** Collision semantics (HP1) + +Org tags are a best-effort filter view, not a reversible label representation. If multiple Linear labels slugify to the same tag (=A/B= and =A B= both → =a_b=), the heading renders that tag *once*, at the first colliding label's position. =:LINEAR-LABELS:= remains the only exact, ordered list of label names. This lossiness is one reason bidirectional sync is out of scope. ** Render tags on the issue heading -=pearl--format-issue-as-org-entry= appends the issue's label tags to the heading line in org tag syntax: +A pure =pearl--label-tags= over the normalized label plists: slugify each, drop empties, de-dupe preserving order. =pearl--format-issue-as-org-entry= appends them in org tag syntax: : ** TODO [#B] Fix the thing :bug:backend: -Slugify each label name, de-duplicate (preserving order), and join as =:t1:t2:=. No labels → no tag string. The =:LINEAR-LABELS:= drawer stays as the canonical structured store (it holds the exact Linear names, which the tag form lossily slugifies); the heading tags are the derived, org-native view. +No labels → no tag string. Only *issue* headings get tags — the parent view heading and the Comments / individual-comment headings carry none. + +** Pearl owns the whole issue-heading tag set (HP3) + +On an issue heading, Pearl owns the *entire* tag set — there are no user-owned local tags mixed in (v1). =pearl--set-heading-label-tags= replaces all org tags on the current issue heading with the label-derived set, via Org APIs, preserving the TODO keyword, priority cookie, title text, identifier prefix, and the body/drawer. Clearing an issue's labels removes its heading tags. This single ownership rule is what makes manual edits predictably unsupported (below) and keeps the rewrite contract simple. + +** Manual heading-tag edits are overwritten / ignored (HP2) + +Org tags look editable, so the v1 rule is explicit: hand-editing a heading's tags is an unsupported derived-view edit. Concretely — + +- =pearl-set-labels= rewrites the heading tag set from the labels it *successfully pushed* (and the drawer), so a manually added tag disappears on the next label change; +- fetch / refresh rewrites heading tags from the remote labels (self-healing); +- =pearl-save-issue= / =pearl-save-all= and the dirty scanner *ignore heading tag diffs entirely* — a changed tag is never treated as a dirty field and never pushed. -Only *issue* headings get tags. The parent view heading and the Comments / individual-comment headings carry none. +The way to change labels stays =pearl-set-labels=. -** Keep the drawer authoritative; tags are a view +** Keep the drawer authoritative -The =:LINEAR-LABELS:= drawer remains the source of truth for the issue's labels (it survives slugify collisions and preserves the display names). =pearl-set-labels= continues to push and rewrite the drawer, and additionally re-renders the heading's tag string so the two stay consistent after a label change. +=:LINEAR-LABELS:= remains the source of truth (exact Linear names, order, and labels that slugify lossily or to empty). The heading tags are the derived, org-native view. =pearl-set-labels= updates both after a successful push; a *failed* =pearl-set-labels= leaves both the drawer and the existing heading tags unchanged. + +** #+TAGS and inheritance (MP3, MP4) + +- =#+TAGS:= completion declaration is *deferred to vNext* (decision, not an open question): Org still filters, sparse-trees, and agenda-searches on undeclared tags present on headings, so completion isn't needed for the feature. +- Org tag *inheritance* (on by default) is *accepted* in v1: an issue's label tags inheriting onto its Comments subtree in agenda/sparse-tree contexts is normal, useful Org behavior. No file-local override. A README note only if users are likely to trip on it. ** v1 is render-only (fetch direction) -Editing the heading's tags by hand does *not* push to Linear in v1. The way to change labels stays =pearl-set-labels= (which now updates both the drawer and the heading tags). Bidirectional sync — parse the heading's tags on save and reconcile them against Linear's label set — is deferred (see out of scope); it needs the same kind of conflict gate the description/title/comment syncs have, and label *creation* semantics (a tag with no matching Linear label) to be decided. +Editing the heading's tags by hand does not push to Linear in v1. Bidirectional sync — parse heading tags on save and reconcile against Linear's label set — is deferred: it needs a conflict gate like the description/title/comment syncs, label-*creation* semantics (a tag with no matching Linear label), and a way around the slugify lossiness above. + +* Proposed v1 decisions (this feature) + +1. =pearl--label-name-to-tag=: downcase, non-=[[:alnum:]_]= runs → =_=, trim; Unicode alnum preserved (MP1). +2. Collision → render the shared tag once at the first label's position; drawer keeps both names (HP1). +3. Empty-slug label dropped from the heading, kept in the drawer (MP2). +4. Pearl owns the entire issue-heading tag set; =pearl--set-heading-label-tags= replaces all heading tags, preserving keyword/priority/title/identifier/body/drawer (HP3). +5. Manual heading-tag edits are unsupported: =set-labels= and fetch/refresh rewrite them from authoritative state; save/dirty-scan ignore tag diffs (HP2). +6. Drawer authoritative; failed =set-labels= leaves drawer + tags unchanged. +7. =#+TAGS:= declaration deferred to vNext (MP3); tag inheritance accepted in v1 (MP4). +8. Render-only (fetch direction); bidirectional tag editing is vNext. + +* Open questions for Craig + +The review's two open questions are resolved as decisions 1 and 4 (preserve Unicode alnum; Pearl owns the whole issue-heading tag set), both per Codex's recommendation — flip either if you'd rather ASCII-only tags or room for user-owned local tags. Otherwise none blocking. * Files touched -- =pearl.el=: new =pearl--label-name-to-tag=; =pearl--format-issue-as-org-entry= (append tag string to the heading); =pearl-set-labels= (re-render heading tags after a label change). The =:LINEAR-LABELS:= drawer line is unchanged. -- Tests: new cases for =pearl--label-name-to-tag= (normal, multiword, punctuation, collision, empty); a render test (issue with labels → heading carries the slugified tags); a regression test that =pearl--issue-title-at-point= / title sync still returns the bare title with tags present; a =pearl-set-labels= test that the heading tag string updates. -- =README.org=: note in the Fields / "active org file" section that labels render as heading tags; mention the drawer remains the structured store. +- =pearl.el=: new pure =pearl--label-name-to-tag= and =pearl--label-tags=; =pearl--set-heading-label-tags= (replace the heading's tag set via Org APIs); =pearl--format-issue-as-org-entry= (append the tag string on issue headings); =pearl-set-labels= (rewrite heading tags after a successful push). The =:LINEAR-LABELS:= drawer line is unchanged; the dirty scanner is explicitly *not* extended to tags. +- Tests: see below. +- =README.org=: note that labels render as heading tags for filtering, generated from Linear — "use =pearl-set-labels= to change them"; the drawer stays the structured store. * Test plan -- *Tag slugify*: =Bug=→=bug=, =Needs Review=→=needs_review=, =UI/UX=→=ui_ux=, =P1=→=p1=, collision de-dup, empty-after-slug dropped. -- *Render*: an issue with labels =("Bug" "Backend")= renders =** … :bug:backend:= and still carries =:LINEAR-LABELS: [Bug, Backend]= in the drawer. -- *Title-sync regression*: with tags on the heading, =pearl--issue-title-at-point= returns the bare title (no tags, no keyword), so a no-op title sync still matches and a title edit still round-trips. -- *set-labels*: after changing labels, the heading tag string reflects the new set (and the drawer too). +- *Slugify:* =Bug=→=bug=, =Needs Review=→=needs_review=, =UI/UX=→=ui_ux=, =P1=→=p1=, =naïve café=→=naïve_café= (Unicode preserved), punctuation-only → empty. +- *Render:* labels =("Bug" "Backend")= → =** … :bug:backend:= with =:LINEAR-LABELS: [Bug, Backend]= in the drawer. +- *Collision:* =("A/B" "A B")= → one =:a_b:= tag, drawer holds both names. +- *Empty slug:* a punctuation-only label → no heading tag, drawer still contains the name. +- *Title-sync regression:* with tags present, =pearl--issue-title-at-point= returns the bare title, so a no-op title sync still matches and a title edit round-trips. +- *set-labels:* after a successful change the heading tag set reflects the new labels (and the drawer); a *manually added* extra tag is gone afterward. +- *Clearing labels:* removes the heading tags and sets the drawer to =[]=. +- *Failed set-labels:* drawer and existing heading tags both unchanged. +- *Save isolation:* =pearl-save-issue= / the dirty scanner do not treat a tag-only heading change as a dirty field (no push). +- *Scope:* non-issue (parent view) and comment headings receive no tags. * Migration Additive — no breaking change. Existing files gain heading tags on the next fetch. The removed =#+filetags= is already gone (952cfe7). No defcustom changes. -* Open questions for review +* Review Dispositions -1. *Tag case.* Lowercase (=Bug= → =bug=) for org-tag convention, or preserve Linear's case (=Bug= → =Bug=)? Lowercase is my lean; it matches how most org users tag and avoids =Bug= vs =bug= duplication. -2. *=#+TAGS:= declaration.* Should the file declare =#+TAGS:= with the union of label slugs (for tag-completion in the buffer), the way the workflow-states spec derives =#+TODO:=? Bonus, not required for the feature. My lean: defer to a follow-up to keep v1 focused. -3. *Tag inheritance.* Org tag inheritance is on by default, so an issue's tags apply to its Comments subtree in agenda/inheritance contexts. Harmless (a comment "inheriting" =:bug:= rarely matters), but flagging. Disable inheritance for these files, or accept it? My lean: accept it. -4. *Bidirectional sync* (edit heading tags → push labels) — confirm it's out of scope for v1. +*Round 1 (Codex, 2026-05-25).* Rubric =Ready with caveats=. Everything accepted as written, no modifications or rejects. HP1 (explicit collision semantics — shared tag rendered once, drawer authoritative), HP2 (manual heading-tag edits overwritten by =set-labels=/refresh and ignored by save/dirty-scan), and HP3 (Pearl owns the whole issue-heading tag set; =pearl--set-heading-label-tags= replaces all tags preserving keyword/priority/title/identifier) drove the new design sections. MP1 (preserve Unicode alnum), MP2 (empty-slug label stays in the drawer), MP3 (=#+TAGS:= deferred to vNext), and MP4 (accept tag inheritance) are folded in as decisions. The two review open questions are resolved per Codex's recommendation (Unicode-preserve, Pearl-owns-tag-set); surfaced to Craig as flippable. The architecture, UX, and test recommendations are folded into the body and test plan. * vNext / out of scope - Bidirectional tag editing (heading tags → Linear labels) with a conflict gate and label-creation semantics. - =#+TAGS:= completion declaration. - Color/face mapping from Linear label colors to org tag faces. +- Mixed user-owned local tags alongside Pearl-owned label tags, if a real need appears. diff --git a/docs/multi-account-spec.org b/docs/multi-account-spec.org index 32a7527..bf85c29 100644 --- a/docs/multi-account-spec.org +++ b/docs/multi-account-spec.org @@ -5,9 +5,9 @@ * Status -*DRAFT — design proposal; nothing in =pearl.el= has changed.* Open questions for Craig at the end. +*Reviews incorporated through round 3; rubric =Ready* (Craig, 2026-05-25).* The original "resolve globals on switch, downstream oblivious" model leaked across accounts in an async codebase; this spec replaces it with an explicit account-context layer — dispatch-time snapshots, an implementation-boundary rule for where context is captured/applied, per-buffer =#+LINEAR-ACCOUNT= ownership with wrong/unmarked-buffer guards, a runtime (not persisted) active-account, a mode-line indicator, and an exact startup rule. The five safety calls (finish-into-snapshot, wrong-buffer refuse, unmarked-mutation refuse, saved-query =:account= guard, optional =:url=) are *adopted as final v1 decisions* — each the safer default and round-3-endorsed. Modified recommendations are in Review Dispositions. -Lets one Emacs talk to more than one Linear workspace — a work account and a personal account — and switch between them without re-customizing. Raised by Craig: "I have a work account and a personal account. I would like to choose which account I'm working on easily and switch back and forth between the two fairly straightforwardly." +Lets one Emacs talk to more than one Linear workspace — a work account and a personal account — and switch between them safely. Raised by Craig: "I have a work account and a personal account. I would like to choose which account I'm working on easily and switch back and forth between the two fairly straightforwardly." * Problem @@ -19,104 +19,184 @@ Everything that identifies a workspace is a single global value today: - =pearl-default-team-id= — one team. - the lookup caches (=pearl--cache-teams=, =-states=, =-team-collections=, =-views=, =-viewer=) — global, and *workspace-specific*: a personal account's teams and a work account's teams must never bleed together. -To work the other account you'd re-customize the key (and team, and file) by hand and clear the cache. There's no notion of "which account am I on." +To work the other account you'd re-customize the key (and team, and file) by hand and clear the cache. There's no notion of "which account am I on," and — the danger the reviewer surfaced — nothing stops a work command from running under personal credentials or a work fetch from landing in the personal file. * Current state -- =pearl-api-key= is a plaintext defcustom, optionally loaded from =LINEAR_API_KEY= via =pearl-load-api-key-from-env=. =pearl--headers= reads it directly and errors when unset. +- =pearl-api-key= is a plaintext defcustom, optionally loaded from =LINEAR_API_KEY=. =pearl--headers= reads it directly and errors when unset. - =pearl-org-file-path= defaults to =gtd/linear.org=; one file holds the active view. -- The caches are module-level =defvar=s; =pearl-clear-cache= resets them all. Nothing scopes them to a workspace, so switching keys without clearing the cache would serve stale (wrong-account) teams/views/viewer. +- *Requests and writers read the globals late, not at dispatch.* =pearl--graphql-request-async= reads the key/url when the request starts, and =pearl--update-org-from-issues= reads =pearl-org-file-path= at *callback/render* time — so a global changed mid-flight is read by the in-flight callback. This is the async-leak surface. +- The caches are module-level =defvar=s; =pearl-clear-cache= resets them all. Nothing scopes them to a workspace. - =pearl--cache-viewer= caches "who am I" — inherently per-account. +- Rendered files carry =#+LINEAR-SOURCE= / run-at / filter / count / truncated, but *no account identity*. * Proposed design ** The account model -A new defcustom =pearl-accounts=: an alist of named accounts, each a plist of the per-workspace settings. +A new defcustom =pearl-accounts=: an alist of named accounts, each a plist of per-workspace settings. #+begin_src elisp (setq pearl-accounts - '(("work" :api-key-source (auth-source "api.linear.app" "work") + '(("work" :api-key-source (:auth-source :host "api.linear.app" :user "work") :org-file "~/org/work-linear.org" :default-team-id "TEAM_WORK") - ("personal" :api-key-source (auth-source "api.linear.app" "personal") + ("personal" :api-key-source (:env "LINEAR_PERSONAL_API_KEY") :org-file "~/org/personal-linear.org" :default-team-id nil))) #+end_src -Each entry carries what's currently global: the credential, the org file, the default team, and (rarely needed) the endpoint. =:api-key-source= is how the key is *found*, not the key itself (see Credentials below). +=:api-key-source= is how the key is *found*, not the key itself — a concrete tagged plist: =(:auth-source :host H :user U)=, =(:env "VAR")=, or =(:literal "lin_...")= as an escape hatch. =:org-file= and the optional =:url= round it out. -=pearl-active-account= holds the current account name. =pearl-default-account= picks the one used at startup. +=pearl-default-account= is a defcustom (the user's durable preference for the startup account). =pearl-active-account= is *runtime state*, a plain =defvar= — never a persisted defcustom: the active account is which one you're on right now, and =pearl-switch-account= sets it with =setq=, never =customize-save-variable=. Persisting "active" would make the last switch stick across sessions in a surprising way and fight =pearl-default-account=. + +** Account-context layer (the core of the safety model) + +Rather than mutating globals and hoping downstream is oblivious, account state flows through an explicit context: + +- =pearl--resolve-account= (NAME) → a context plist =(:name :api-key :org-file :default-team-id :url)=, with =:api-key= resolved from =:api-key-source= and =:org-file= expanded. Errors clearly when the account or its key is missing. +- =pearl--current-account-context= returns the active account's context (resolving =pearl-default-account= on first need; see Startup). +- =pearl--with-account-context= (CONTEXT &rest body) dynamically binds the existing globals (=pearl-api-key=, =pearl-org-file-path=, =pearl-default-team-id=, =pearl-graphql-url=) to the context's values for BODY. Most downstream function *bodies* stay unchanged — they still read those globals — but now they read them under a controlled binding rather than a mutable global. (This replaces the draft's "downstream commands are unchanged" claim, which the review correctly rejected.) + +** Account context and async safety (HP1) + +Every API operation *snapshots* its account context at dispatch time. The snapshot is the =pearl--resolve-account= plist captured before the request fires. Render and mutate callbacks act against the *snapshot's* org file / key, never the current global. + +*v1 decision (confirm):* if the active account changed before a callback completes, the callback still finishes into its snapshot — writing the work result to the work file, with a status message naming the account ("Updated work: 24 issues"). It does *not* write to whatever file is now current, and it does *not* silently drop the result the user asked for. (The reviewer offered finish-into-snapshot or drop-stale; finish-into-snapshot is chosen because the fetch was an explicit user request whose data belongs to its own account's file regardless of a later switch, and round 2 endorsed it as the right default.) + +*Implementation boundary (where the snapshot is captured and applied).* So a single missed render path can't reintroduce the leak: + +- every public API/file command resolves =pearl--current-account-context= once at entry; +- async callbacks that render or mutate local state run *inside* =pearl--with-account-context= bound to that captured context — binding at dispatch alone isn't enough, because the late reads (e.g. =pearl--update-org-from-issues= reading =pearl-org-file-path=) happen in the callback body; +- shared render functions take an explicit context or target-file argument where practical, especially =pearl--update-org-from-issues=, rather than relying on the dynamic binding reaching them; +- raw =let= bindings of =pearl-api-key= / =pearl-org-file-path= scattered through command bodies are avoided — the macro and the explicit args are the only two paths, so the safety model stays auditable. + +** Account file ownership (HP2) + +Rendered files carry =#+LINEAR-ACCOUNT: <name>= in the header. Every command that hits the API or writes the file calls a shared guard first, =pearl--require-account-context= (or similar): + +- buffer account == active account → proceed; +- buffer account is configured but *not* active → *refuse* (v1, not auto-switch — auto-switch hides a state transition right before a remote mutation) with a message naming *both* accounts: "This file belongs to work; active account is personal. Run =M-x pearl-switch-account= first." +- *no account marker* while =pearl-accounts= is non-nil → non-mutating render/list commands may proceed (they don't risk wrong-account writes the way a mutation does), but *mutating* commands refuse: "No LINEAR-ACCOUNT in this file; refresh it under an account first." One refresh stamps ownership. (With =pearl-accounts= nil, an unmarked file is just legacy single-account behavior — no guard.) + +The mode-line indicator tells you the active account; this guard enforces it. Visibility alone is insufficient — the file/command context needs enforcement. + +** Active-account indicator (HP3, v1 requirement) + +A visible current-account indicator ships in v1, not as polish. The *v1 requirement* is a global mode-line lighter (=Pearl[work]=) — global, and easy to test after =pearl-switch-account=; a buffer-local header-line on account files is optional polish on top. The user must be able to tell the active account before running a mutating command. Pushing a work edit while thinking you're on personal is the failure mode this whole feature exists to prevent. ** Switching accounts =pearl-switch-account= (interactive): =completing-read= over =pearl-accounts= names, then: -1. Set =pearl-active-account=. -2. Resolve and bind the active account's settings — the key, the org file, the default team — so the existing code keeps reading =pearl-api-key= / =pearl-org-file-path= / =pearl-default-team-id= unchanged (they become "the active account's values"). -3. *Clear the lookup caches* — they're workspace-specific; a switch must invalidate teams/states/collections/views/viewer so the next lookup refetches against the new workspace. -4. Optionally surface the new account's org file. +1. Resolve the account's context (=pearl--resolve-account=). +2. Set =pearl-active-account= and install the context into the globals (the binding the rest of the package reads). +3. *Clear the account-scoped caches* via =pearl--clear-account-scoped-state= (below). +4. Update the indicator and optionally surface the account's org file. -The cleanest implementation keeps the rest of the package oblivious: a single resolver sets the three global-looking variables from the active account, and everything downstream (=pearl--headers=, =pearl--update-org-from-issues=, =new-issue=) works as-is. The accounts layer is a front-end over the existing globals. +** Credentials (MP1) -** Credentials +Keys resolve through =:api-key-source= at context-resolution time. =auth-source= (=~/.authinfo.gpg=) is the documented default; =:env= and =:literal= are supported. Hard rules: a resolved key is assigned with =setq= (or a dynamic binding) only — *never* =customize-save-variable= — so secrets don't land in =custom.el=; and debug/setup output must never log the key (a missing-key error names the account, e.g. "No API key for account work", without dumping the lookup). -Storing two API keys in plaintext customize is the wrong default. *Proposed: resolve keys through =auth-source=* (so =~/.authinfo.gpg= holds them), keyed per account. =:api-key-source= names the host + user to look up. A literal =:api-key "lin_..."= form stays supported as an escape hatch (and for the env-var path), but the documented default is encrypted auth-source. This also fixes a latent issue with the single-key model — the key sits in customize today. +=pearl-load-api-key-from-env= stays legacy-only. With =pearl-accounts= nil it works as today (loads =LINEAR_API_KEY= into the global). With accounts configured it refuses ("Accounts are configured; put =:api-key-source (:env \"...\")= in =pearl-accounts=") so it can't bypass the resolver and overwrite the active account's key in memory. -** Cache isolation +** Cache isolation (MP3) -Two options (open question 1): +Clear-on-switch for v1 (keyed-per-account caches are vNext). =pearl--clear-account-scoped-state= is the single helper that clears every account-scoped cache — the five Linear lookups (=teams=, =states=, =team-collections=, =views=, =viewer=) plus any future account-derived state (e.g. the workflow-state-derived TODO keyword caches, if that feature lands). =pearl-switch-account= calls this helper rather than re-implementing cache semantics, so a newly added account-scoped cache is registered in one place. A test asserts every cache the helper names is nil after a switch. -- *Clear on switch* (simpler): the caches stay global =defvar=s; =switch-account= clears them. Correct as long as every switch goes through the command. Minimal change. -- *Keyed by account* (stricter): the caches become per-account maps, so switching back to "work" reuses its warm cache instead of refetching. More code, better ergonomics for frequent flippers. +** Saved queries (MP2) -Lean: clear-on-switch for v1 (it reuses =pearl-clear-cache=), keyed caches as a vNext nicety. +v1 keeps =pearl-saved-queries= a single shared list, but a query entry may carry an optional =:account= field alongside =:filter= / =:sort= / =:order=: -** The active-file question +#+begin_src elisp +("My work bugs" :account "work" + :filter (:team "ENG" :label "bug" :assignee :me)) +#+end_src + +=pearl-run-saved-query= checks =:account= *before* any name-to-id filter compilation or network call: if =:account= names a different account than the active one, it refuses immediately ("Saved query 'My work bugs' belongs to work; switch accounts first"). The filter's team/state/label names resolve against the active workspace, so "Backlog" can silently mean different things or nothing across accounts — the guard catches that before a wrong-account fetch. The README warns that saved queries without =:account= are shared and resolved against whatever account is active. (Full account-scoping is vNext.) + +** Startup / default resolution (HP4) -Each account gets its own =:org-file= (proposed), so "work" issues and "personal" issues never share a buffer. Switching accounts switches which file is active. The alternative — one file with a per-account section — fights the active-file model (one view at a time) and risks cross-account =LINEAR-ID= collisions in the same buffer. Per-account files keep the existing single-active-view model intact, just parameterized by account. +Exactly one rule: + +- =pearl-accounts= nil → the legacy globals are used unchanged (single-account, today's behavior). +- =pearl-accounts= non-nil and =pearl-active-account= nil → the first command that needs account context resolves =pearl-default-account=; if no default is configured, it errors with a message telling the user to set one or run =pearl-switch-account=. +- =pearl-switch-account= and =pearl--current-account-context= are the only paths that install/return account settings. ** What's account-scoped -Everything that hits the API or writes the file: list/view/saved-query fetches, all the sync/save commands, field setters, comment commands, =new-issue=, the caches, the viewer identity (for comment-edit permission — "your own comments" is per-account). The account is resolved once at switch time into the globals the commands already read, so no command needs to know about accounts individually. +Everything that hits the API or writes the file: list/view/saved-query fetches, the sync/save commands, field setters, comment commands, =new-issue=, the caches, and the viewer identity (comment-edit permission is per-account). All of it flows through the context layer: snapshot at dispatch, bind for the body, guard the buffer. * Proposed v1 decisions (this feature) -1. =pearl-accounts= alist (name → plist of key-source / org-file / default-team / optional url); =pearl-active-account= + =pearl-default-account=. -2. =pearl-switch-account= sets the active account, resolves its settings into the existing global variables, and clears the caches. -3. Credentials resolve through =auth-source= by default; a literal key stays supported. -4. One org file per account (=:org-file=); switching switches the active file. -5. The accounts layer is a front-end over the current globals — downstream commands are unchanged. +1. =pearl-accounts= defcustom (name → plist: =:api-key-source=, =:org-file=, =:default-team-id=, optional =:url=); =pearl-default-account= defcustom; =pearl-active-account= a runtime =defvar=, set with =setq=, never persisted. +2. An account-context layer (=pearl--resolve-account= → plist, =pearl--with-account-context=, =pearl--current-account-context=); downstream bodies stay mostly unchanged behind it, with the implementation-boundary rule (entry resolves context; callbacks run inside the context macro; shared render fns take an explicit target where late reads are dangerous). +3. Dispatch-time context snapshots; a callback finishes into its snapshot's file/context even if the account switched mid-flight, messaging the account. +4. =#+LINEAR-ACCOUNT:= header on rendered files + a shared buffer guard: refuse a command from another account's file (naming both accounts); from an unmarked file under accounts, render/list may proceed but mutations refuse until a refresh stamps ownership. +5. A mode-line lighter is the v1 active-account indicator (header-line optional). +6. Credentials via a tagged =:api-key-source= plist (auth-source default, env + literal supported); resolved keys never persisted via Customize, never logged; =pearl-load-api-key-from-env= is legacy-only and refuses once accounts are configured. +7. Clear-on-switch via =pearl--clear-account-scoped-state= (keyed caches vNext). +8. Saved queries shared with an optional =:account= guard checked before filter compilation + a README warning. +9. Exact startup rule; passive migration. + +Decisions 3, 4, and 8 (and the unmarked-mutation refusal in 4) were the safety calls; they are now *final* — adopted as the safer defaults, endorsed across the reviews. + +* Resolved safety decisions + +The five safety calls are final (adopted as the safer defaults, round-3-endorsed): + +1. *Async-stale callback*: finish into the snapshot's file with an account-named message (not drop). +2. *Wrong-account buffer*: refuse with a message naming both accounts (not auto-switch). +3. *Unmarked file under accounts*: render/list proceed; mutations refuse until a refresh stamps =#+LINEAR-ACCOUNT=. +4. *Saved queries*: shared list + optional =:account= guard checked before filter compilation (not fully account-scoped in v1). +5. *Endpoint* =:url=: optional-but-supported through the same resolver. + +The vNext alternatives (drop-stale, auto-switch, fully account-scoped queries) stay in vNext if a real need appears. * Files touched -- =pearl.el=: =pearl-accounts= / =pearl-active-account= / =pearl-default-account= defcustoms; =pearl-switch-account= + a =pearl--resolve-account= helper that binds the globals; an =auth-source= key resolver; a one-time migration of the legacy single-key config; a transient/keymap entry for switching. +- =pearl.el=: the =pearl-accounts= / =pearl-active-account= / =pearl-default-account= defcustoms; the context layer (=pearl--resolve-account=, =pearl--current-account-context=, =pearl--with-account-context=); the =:api-key-source= resolver; =pearl-switch-account= + =pearl--clear-account-scoped-state=; =#+LINEAR-ACCOUNT= emission in =pearl--build-org-content= and the =pearl--require-account-context= buffer guard wired into the API/file commands; dispatch-time context capture in the async entry points + render/mutate callbacks; the mode-line/header indicator; passive legacy fallback. - =docs/=: this spec. -- =README.org=: a multi-account setup section (auth-source entries, example =pearl-accounts=). +- =README.org=: a multi-account section (auth-source entries, example =pearl-accounts=, the saved-query warning, the indicator). * Test plan -- =pearl--resolve-account=: a named account's plist sets the expected key / org-file / default-team; an unknown name errors. -- =switch-account= clears all five caches (assert each is nil after). -- auth-source resolver: a stubbed =auth-source-search= yields the key; a missing entry gives a clear error naming the account, not a generic "key not set". -- back-compat: with =pearl-accounts= nil and the legacy =pearl-api-key= set, everything works exactly as before (single-account path). -- viewer cache: switching accounts invalidates =pearl--cache-viewer= so comment-edit permission re-resolves against the new account. +- =pearl--resolve-account=: a named account's plist resolves the expected key / org-file (expanded) / default-team / optional url; an unknown name errors. +- =:api-key-source= resolver: stubbed auth-source yields the key; =:env= reads the variable; a missing entry errors naming the account ("No API key for account work") without leaking lookup internals; the resolved key is never persisted through Customize. +- *Async safety (HP1):* dispatch a list fetch under "work", switch to "personal" before invoking the captured callback, assert the result writes the *work* org file (or is dropped, per the chosen rule). +- *Buffer guard (HP2):* render a work file with =#+LINEAR-ACCOUNT: work=, set active to personal, assert mutating commands (=refresh-current-view=, =save-issue=, a field setter, a comment command, =delete-current-issue=) refuse *before* any API call. +- *Startup (HP4):* a command run with accounts configured but no active account resolves =pearl-default-account=, or errors when none is set. +- =pearl--clear-account-scoped-state=: every cache it names is nil after =switch-account=; the viewer cache invalidates so comment-edit permission re-resolves. +- Back-compat: =pearl-accounts= nil + legacy =pearl-api-key= → unchanged single-account behavior. +- =pearl-active-account= is runtime state — set by =switch-account= with =setq=, not persisted through Customize. +- =pearl-load-api-key-from-env= refuses when =pearl-accounts= is non-nil; works as legacy when nil. +- Unmarked buffer + accounts enabled: a render/list command proceeds, a mutating command refuses ("No LINEAR-ACCOUNT…"). +- Saved query with =:account= refuses under a different active account *before* any filter compilation or resolver/network call. +- Indicator: the mode-line lighter updates after =pearl-switch-account=. +- Killed-buffer / lost-marker safety on a render callback that completes after the buffer is gone. + +* Migration (MP4) + +Passive. With =pearl-accounts= unset, the package behaves exactly as today off the legacy globals. When accounts are configured, the README shows how to create a default account. The credential move to auth-source is opt-in (the =:literal= / env forms keep working). Interactive first-run seeding is vNext, not v1. + +* Review Dispositions + +*Round 1 (Codex, 2026-05-25).* Rubric =Not ready=. The four blocking findings (HP1 async cross-account writes, HP2 wrong-account buffer commands, HP3 indicator-is-v1, HP4 startup resolution) are accepted and drove the new account-context layer, the dispatch-time snapshot rule, the =#+LINEAR-ACCOUNT= buffer guard, the required indicator, and the exact startup rule. MP1 (concrete credential plist, setq-only, no logging) and MP4 (passive migration) accepted as written. The architecture, UX, robustness, and test-strategy observations are folded into the body and test plan. Two were modified: -* Migration +- *MP2 (saved queries) — modified to a choice.* The reviewer offered account-scoped lists, an optional =:account= guard, or document-and-warn. Chose the optional =:account= guard plus a shared default and a README warning: it keeps the v1 saved-query store unchanged (one list) while giving a refusal path for queries that must not cross accounts, where full account-scoping would add a persistence/UX layer not yet justified for two workspaces. +- *MP3 (cache clearing) — modified the test framing.* Accepted the helper (=pearl--clear-account-scoped-state= as the single source of truth, listing every account-scoped cache including future derived state). Softened the reviewer's "a test must fail when a new account-scoped cache is added without invalidation" — you can't test for an unwritten future cache directly. Instead the helper centralizes the list so a new cache is registered in one place, and a test asserts each cache the helper names is nil after a switch. -Back-compatible. With =pearl-accounts= unset, the package behaves exactly as today off the legacy =pearl-api-key= / =pearl-org-file-path= / =pearl-default-team-id=. First-run-with-accounts can offer to seed a single "default" account from the legacy values. The credential move to auth-source is opt-in: the literal-key form keeps working, so no one is forced to migrate secrets to adopt accounts. +Also, the reviewer's three open questions and the HP1 finish-vs-drop choice are baked as proposed v1 decisions (finish-into-snapshot, refuse-on-mismatch, =:account= guard) and surfaced under Open Questions for Craig's confirmation rather than left unresolved. Everything else accepted as written. -* Open questions for Craig +*Round 2 (Codex, 2026-05-25).* Rubric moved to =Ready with caveats=. All findings accepted as written, no modifications or rejects: HP1 (the implementation-boundary rule — entry resolves context, callbacks run inside the context macro, shared render fns take an explicit target), HP2 (the precise unmarked-buffer rule — render/list proceed, mutations refuse until a refresh stamps ownership), HP3 (=pearl-active-account= is a runtime =defvar=, not a persisted defcustom — a genuine correction to the round-1 draft), MP1 (=pearl-load-api-key-from-env= legacy-only, refuses under accounts), MP2 (=:account= lives in the saved-query entry and is checked before filter compilation, with an example), MP3 (mode-line lighter is the v1 indicator, header-line optional), plus the UX refusal-message-names-both-accounts and the added tests. The remaining caveats are the safety calls, now endorsed by the reviewer. -1. *Cache strategy*: clear-on-switch (simple, refetches after every flip) or per-account keyed caches (warm on switch-back, more code)? How often do you actually flip — a few times a day, or constantly? -2. *Credential default*: auth-source / =~/.authinfo.gpg= as the documented path, with literal-key as the escape hatch — good? Or do you keep keys somewhere else (a password manager, env vars per account)? -3. *Active file*: one file per account (proposed) versus a single file you re-point — any reason you'd want both accounts' issues reachable without switching? -4. *Switch surface*: a plain =M-x pearl-switch-account=, plus a mode-line indicator of the current account? The indicator matters most — pushing a work edit while you think you're on personal (or vice versa) is the failure mode worth guarding against. -5. *Endpoint*: is =graphql-url= ever actually different per account (self-hosted / EU region), or is per-account =:url= over-engineering for two linear.app workspaces? +*Round 3 (Codex, 2026-05-25).* Rubric =Ready with caveats=; no new findings — MP1-MP5 were each "confirm decision X" on the five safety calls, all endorsed as the safer defaults. Resolved by adopting all five as final v1 decisions (Craig granted the latitude to adopt the review's recommendations directly). Rubric -> =Ready=. * vNext / out of scope -- Per-account keyed caches (if clear-on-switch proves annoying). -- Showing both accounts at once (split buffers / frames) — explicitly out; the model is one active account at a time. -- Per-account =pearl-saved-queries= (v1 shares the saved-query list across accounts; revisit if names collide or filters don't translate). -- Auto-detecting the account from the active file when you visit it. +- Per-account keyed caches (warm switch-back), if clear-on-switch proves annoying. +- Auto-switch (instead of refuse) from a visited account file. +- Showing both accounts at once (split buffers / frames) — the model is one active account at a time. +- Fully account-scoped =pearl-saved-queries=. +- Interactive migration/seeding from the legacy single-account globals. +- Auto-detecting the account from the active file on visit. |