368 Commits

Author	SHA1	Message	Date
xarmian	e12feb46cb	fix(copy): authorize attachment references in cross-workspace copy (TASK-2408) ... Cross-workspace copy authorized the source item and the destination collection but never the individual attachments it cloned. PlanAttachmentCopy scoped every lookup to `workspace_id = SourceWorkspaceID AND deleted_at IS NULL` — but the workspace is not the caller, so a restricted member who could edit any item in the source workspace could paste `pad-attachment:<uuid>` for an attachment on an item they could not see, copy that item into a workspace they own, and read the bytes through the ordinary blob endpoint (BUG-2407). The planner now consults an AttachmentAuthorizer supplied by the caller, applied to every row it resolves: the referenced rows, the parents it adopts as clone roots, and the variants it follows. A denial DELETES the row from the resolution map, so it is indistinguishable from a row that was never there — the reference lands in UnresolvableRefs beside dangling, soft-deleted and foreign ids, and attachment_count / attachment_bytes / unresolvable_ref_count read identically. The preflight's numbers stay oracle-free. It is a callback because the rule is the read path's — resolve the parent, reject a foreign or non-live one, check item visibility, apply the orphan rule — and every input to it lives in package server. It cannot run BEFORE planning either: the copy re-reads the source content under its locks and computes destination fields inside its transaction, so a reference set enumerated beforehand is not the set the planner resolves. Authorizing the rows the planner actually resolved keeps the dry run and the copy on one path, which is the property DR-11 exists to protect. Both endpoints take the authorizer off the same shared resolution (resolveAuthorizedCopy), so what the preview calls unresolvable is what the copy refuses to clone. Mutation-verified: without the authorizer the secret PNG is cloned into the destination, referenced by the rewritten body, and served byte-identical to the attacker through the destination workspace.	2026-08-02 04:54:31 +00:00
xarmian	2318a17e49	fix(attachments): classify derived rows after authorization on delete ... Found by the convergence sweep of this branch, which enumerated every attachment-touching path and compared each against its siblings' gates. The delete handler answered 400 derived_attachment as soon as it saw a ParentID, before any visibility, restriction, role or edit gate. That 400 is reachable only for a row that exists and is live, so a guessed thumbnail UUID answered 400 while an absent, foreign, or deleted id answered the shared 404 — and a caller who could not see the parent, or was restricted out of its collection, learned about the row anyway. Fifth instance of this handler family's existence oracle. Moved after the authorization switch. The classification is a usage error, so it may only be reported to someone already entitled to act on the row; the test pins BOTH halves, so the fix cannot regress into blanket-404ing a legitimate mistake by an authorized caller. Mutation-verified: restoring the previous position makes the restricted caller receive 400 again. Gates: make check exit 0, make test-pg exit 0, zero failures.	2026-08-02 01:50:34 +00:00
xarmian	ba848af85f	fix(attachments): check restriction before the role gate on orphan delete ... Found by the convergence review of this branch. The orphan branch of the delete path called requireMinRole("editor") before attachmentCallerIsRestricted, so a restricted member who guessed a live orphan's UUID got 403 while a bad UUID got 404 — confirming the row exists. Fourth instance of the same existence oracle on this branch, and the one path whose gate ORDER the refactor did not re-check. Notable because attachmentCallerIsRestricted's own contract, added in the previous commit, states that callers must apply it ahead of any role gate that would answer 403. Centralizing the invariant did not fix call-site ordering; only re-reviewing did. Test covers both restricted roles: a viewer and an editor answer differently at the role gate (403 vs success), and NEITHER may be distinguishable from the lookup miss. Mutation-verified — restoring the previous order yields exactly "status = 403, want 404". Gates: make check exit 0, make test-pg exit 0, zero failures.	2026-08-02 01:21:31 +00:00
xarmian	1da96106e8	refactor(attachments): centralize parent resolution, close orphan-read and delete-denial gaps ... Per the final full-diff review of this branch. The six task commits each added authorization to a different attachment path, and each was reviewed CLEAN on its own — but they hand-rolled the same invariant four ways, and the drift between them opened two real gaps that no per-task review could see. Root cause: the blob read, transform, thumbnail derivation and delete paths each loaded the parent item, checked workspace identity and checked liveness in their own shape. resolveAttachmentParentItem is now the one place that invariant lives, returning a four-way outcome (orphan / ok / gone / foreign) so callers keep their own denial behaviour — which is deliberate, not accidental: the HTTP paths must not distinguish the outcomes (any split is an existence oracle), derivation logs a distinct WARN per outcome (greppable ahead of PLAN-2397's repair), and delete passes includeArchived because the storage listing intentionally surfaces archived-parent rows so their quota can be reclaimed. Gaps the drift opened, both closed here: - Orphan GET lacked the full-access gate transform and delete apply, so a restricted member who guessed an orphan attachment's UUID could download it — while transform, delete and the listing all refused. Now shared as attachmentCallerIsRestricted, applied ahead of any role gate, since a 403 reached only for rows that exist is itself the oracle. - The delete path still routed invisible parents through requireItemVisible ("Item not found") while missing and foreign attachments got "Attachment not found" — the same existence oracle already closed twice on this branch, left inconsistent on the one path the tasks did not touch. Every delete denial now goes through the shared writer, asserted byte-identical. Also folds in the live-parent write invariant on upload, which had been applied to transform only: upload validated the item before spooling and then inserted with plain CreateAttachment, so archiving during the upload window bound a row to an archived parent. Derivation deliberately still does NOT take the lock — that trade is documented on deriveThumbnails. Gates: make check exit 0, make test-pg exit 0 (zero failures). Both new guards mutation-verified; attachment authz suite clean under -race -count=2.	2026-08-02 00:55:20 +00:00
xarmian	90eb871da3	fix(attachments): skip derivation for an archived parent (TASK-2404) ... deriveThumbnails checked only that the parent ATTACHMENT row was live and then copied parent.ItemID verbatim into every derived row. After TASK-2401's read gate that is a waste with a cost: a variant of an archived item's attachment is quota-counted storage that the blob path (DR-13) refuses to serve, so the bytes are written, charged, and unreadable until the item is restored. The same holds for a malformed item_id — the column has no FK and no same-workspace constraint, so a row can name a foreign-workspace item or no item at all. Derivation now resolves the parent item at entry, before the blob is even opened, and skips when it is soft-deleted, unresolvable, or in another workspace. GetItem, not GetItemIncludeDeleted, so "live" means the same thing here as on the read path. Orphan rows (item_id NULL) have no item to check and still derive. This is internal background work with no HTTP response, so there is no 404 shape to match: it skips and logs a WARN alongside the existing decode/resize/persist skip logs, with the malformed cases carrying distinct messages so they are greppable ahead of PLAN-2397's repair. The post-check window is DELIBERATELY ACCEPTED, and the comment on thumbnailParentItemLive says so at length so the next reader does not file it as a bug. The check is point-in-time — item deletion commits in its own transaction and the read/decode/resize/encode/Put in between is unbounded work — so an item archived mid-flight can still get a variant. Transform (TASK-2402) closes its equivalent window with store.CreateAttachmentForLiveItem; derivation deliberately does NOT, and makes the opposite trade: transform is user-initiated and low-volume, whereas derivation is a background worker fanning out from every image upload, so an item lock here is disproportionate to the harm. What leaks through is a thumbnail — small, unreadable for as long as its item stays archived, and tombstoned by the delete cascade with its parent attachment. Tests cover the sequential cases only: already-archived (with a sanity check that DeleteItem really is a soft delete), unresolvable item_id, and a foreign-workspace item_id that resolution alone would accept. The raced case is deliberately not asserted — it is permitted behaviour, and pinning it either way would constrain what the design leaves free. Two controls keep the skips honest: a live parent and an orphan row must both still derive from the same fixture and the same bytes, so a fixture that stopped deriving at all would fail loudly rather than pass the skip assertions vacuously. All three skip tests were mutation-verified against a short-circuited guard, and the file passes -race -count=3 and make test-pg. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC	2026-08-01 23:27:34 +00:00
xarmian	380b75e12c	fix(attachments): gate transform on item visibility (TASK-2402) ... handleTransformAttachment opened with a flat requireMinRole("editor") and never looked at the attachment's parent item at all. A restricted editor — one whose collection access excludes that item — could transform an attachment on an item they cannot see, given only the attachment id: the handler read the source blob and returned output metadata plus a new row. The output URL inherits ItemID and is gated by TASK-2401's read gate, so this was not direct byte exfiltration, but it crossed the same boundary and leaked processing behaviour and metadata for an invisible item. The handler now authorizes per-attachment, in the order the read path uses (PLAN-2391 DR-10): load the row -> workspace identity -> load the parent with GetItem -> parent workspace identity -> checkItemVisible -> edit permission -> transform. Every denial goes through writeAttachmentNotFound, so a missing attachment, a foreign parent, a soft-deleted parent and an invisible item are byte-identical; a distinguishable code or message would be an existence oracle. Malformed non-null parents that resolve nowhere are rejected by the same guard. Edit permission is requireEditPermission rather than the flat editor role: an item- or collection-grant editor can already attach to the item (BUG-1661), so refusing them a rotate on their own upload would be an inconsistency, not a boundary. Orphan rows keep the flat editor gate and, matching the DELETE path (PLAN-2382 DR-4), require unrestricted workspace access — the storage listing hides orphans from restricted members, so the transform must not confirm one exists. DR-14's race is closed, not narrowed. The parent check is point-in-time: item deletion commits in its own transaction, and the blob read, decode, transform, encode and Put in between are unbounded work, so the item can be archived mid-flight and the insert then writes a quota-counted live row against an archived item whose bytes DR-13 refuses to serve. The new store.CreateAttachmentForLiveItem re-checks the parent under a row lock inside the insert's own transaction: the row is written against a live item or not written at all. FOR NO KEY UPDATE, not FOR UPDATE — DeleteItem's UPDATE touches no key column so the archival still blocks, while the many tables with a REFERENCES items(id) foreign key (comments, stars, the Yjs op-log) keep taking FOR KEY SHARE on the parent uncontended. SQLite skips the clause: _txlock=immediate already serializes writers there. Tests fail against the pre-fix code: the restricted-editor transform returns 404 with a body byte-identical to the missing-attachment body, and the mid-flight test archives the item from inside the processor's Encode — between the up-front check and the insert — asserting the hook actually ran so it cannot pass vacuously. The Postgres lock test polls pg_stat_activity until the statement is registered as lock-blocked rather than sleeping, and watches the completion channel so a missing lock fails immediately. Both were mutation-verified. Recorded, not fixed here: a refused insert leaves a rowless blob on disk, and the orphan GC is row-driven so nothing reclaims it. Pre-existing on the upload and thumbnail paths too; filed as BUG-2406 with the dedupe guard a correct fix needs. The comment claiming GC reclaims a transform's original was wrong and is corrected — only an orphan original is GC-eligible. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC	2026-08-01 23:05:26 +00:00
xarmian	6e2b972fb0	fix(attachments): gate blob reads on item visibility (TASK-2401) ... handleGetAttachment opened with a flat requireMinRole("viewer"). roleLevel("guest") is 0, below viewer's 1, so every grant-based guest was rejected before any item-level check ran and inline images broke in items shared with them (BUG-2386). The handler now authorizes per-attachment, in the order PLAN-2391 DR-10 fixes: load the row -> verify the parent item's workspace identity -> check item visibility -> serve. Orphan rows keep the flat viewer+ gate; the workspace-wide storage listing is untouched. Also closes two defects sitting immediately around that gate: DR-16 - GetAttachmentVariant scoped on parent_id/variant/deleted_at but not workspace_id, so a foreign-workspace variant sharing a parent id would be served after the local parent was authorized. Fixed at the store API rather than in the handler because the other caller, thumbnail derivation, has its own stake in the scope: an unscoped "does this variant exist?" probe lets a foreign row suppress generation of a legitimate local one. DR-13 - the parent is loaded with GetItem, so a soft-deleted parent 404s. The DELETE path keeps GetItemIncludeDeleted, unchanged. Denial paths now carry Cache-Control: private, no-store, set as the handler's first statement (writeError calls WriteHeader immediately, so anything later never reaches the wire); the positive private, max-age=3600 is set only after authorization succeeds. Every authorization-dependent refusal goes through one writer so the responses are byte-identical and can't be used as an existence oracle. The MCP image resource pad://workspace/{ws}/attachments/{id} inherits the gate; asserted against a real server rather than assumed. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC	2026-08-01 20:42:28 +00:00
xarmian	27b71fe4f6	fix(attachments): resolve item_id across both upload channels (TASK-2400) ... The upload handler read item_id from two places with different rules: authorization resolved only the query-string value, while the association step fell back to the multipart-form value and persisted it verbatim. Since ResolveItem accepts a UUID, a ref, or a slug, a form-supplied ref or a foreign-workspace id could land in attachments.item_id unauthorized and unresolvable — the malformed-row invariant BUG-2387's cross-workspace leak rests on. Three coupled changes (PLAN-2391 DR-2): 1. One effective item_id. Each non-empty channel is resolved in the request workspace and the RESOLVED canonical ids are compared — not the caller's spelling, so query "TASK-12" + form "<uuid>" is agreement, not conflict. Absent and explicitly-empty both mean "no value" (compared after TrimSpace). item.ID is what gets persisted. The form value is read from r.MultipartForm.Value rather than r.FormValue, which merges the query string back in and would collapse the two channels into one. A channel that repeats item_id has every value resolved rather than first-wins, since net/http otherwise silently discards the rest; the value count per channel is capped, because exact-string dedup can't bound the lookups on its own (TASK-7 / task-7 / TASK-0007 resolve alike). 2. Auth ordering. The no-item workspace-editor gate is deferred until after multipart parsing; firing it pre-parse 403'd a form-only item-grant guest (the CLI's shape) before the association that authorizes them was read. The query channel is still resolved and authorized pre-parse so a doomed upload never spools. The route's auth/workspace-access middleware chain is unchanged. 3. Spool cleanup. file.Close() closes the spooled multipart temp file but never removes it; added r.MultipartForm.RemoveAll() on every exit path, including success, where it leaked today too. Status codes (the pinned contract): an item_id that does not resolve in the request workspace → 404 item_not_found on either channel, cross-workspace UUIDs included; two channels — or two values on one channel — that each resolve but to different items → 400 item_id_conflict. Folded in from review: each resolved item is gated on requireItemVisible (404) before the values are compared and before requireEditPermission (403). Without that, the status split is an existence oracle for items a restricted member or ungranted guest can't see — directly via 404-vs-403, or by pairing a visible id with the id being probed and reading 400-vs-404. It also closes requireEditPermission's editor/owner fast path, which never consults collection visibility, so a collection_access="specific" member could otherwise attach to an item in a collection hidden from them. Two intentional behaviour narrowings, both following from DR-2's "reject a non-empty value that does not resolve": an item_id for a soft-deleted item now 404s where a workspace editor previously got a 201 (ResolveItem is live-only) — consistent with DR-13/DR-14 keeping archived parents from accruing new bytes; and an unresolvable item_id no longer falls back to the flat editor gate and silently stores the caller's string. Tests: extends TestUpload_GrantBasedEditorCanAttach with the form-only and both-channel grant-guest cases, the ungranted-item 404, and the paired-probe oracle check; adds canonical-UUID persistence, 404/400 rejection with no row written, repeated conflicting values, the value-count cap, and a >1 MiB isolated-TMPDIR fixture for the spool (a tiny in-memory body never spills to disk, so it would pass either way). The auth-ordering and spool tests were mutation-checked against the pre-fix behaviour. Gates: make check (exit 0), make test-pg (exit 0). Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC	2026-08-01 19:53:12 +00:00
xarmian	e115bb255e	feat(web): delete attachments from the item strip (TASK-2384) ... Adds the first in-item delete path for an attachment (PLAN-2382 phase 2). Before this the only surface was Settings > Storage, which is workspace-wide and disconnected from the item you're looking at. Server: handleDeleteWorkspaceAttachment no longer opens with a flat requireMinRole("editor"). That gate contradicted the UI's grant-aware canEdit (permissions.ts::canEditItem), which is true for a viewer holding an item- or collection-level edit grant -- so that user saw the affordance and got a 403, even though upload already admits them (BUG-1661). Authorization is now per-attachment, mirroring the upload handler: - item-bound: requireItemVisible THEN requireEditPermission. The order is load-bearing -- an attachment on an item the caller can't see must keep returning 404, not the 403 that would confirm it exists. - orphans: unchanged flat editor-role gate plus the guest filter, since there's no item context to authorize against. UI: per-tile delete control, in the DOM unconditionally so it's keyboard reachable (CSS reveals it on hover/focus-within). Gated on ItemDetail's mutationsEnabled, not raw canEdit, so a peeking master stays a complete read-only freeze. Optimistic removal with rollback + toast on failure, fenced so a switch mid-delete can't resurrect A's tile under B. The confirm warns when the id is referenced in this item's body, and deliberately hedges otherwise -- comment bodies, other items' content and fields JSON are not visible client-side, so it says "may still be referenced" rather than claiming non-use. Editor: the attachment-image NodeView assigned img.src with no error path, so a delete left the browser's broken-image glyph until reload -- reading as a network blip for what is a permanent state. It now degrades to the same .attachment-missing placeholder the markdown renderer uses, re-armed on uuid swap so rotate/crop clears a stale placeholder. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC	2026-08-01 12:57:37 +00:00
xarmian	d9d96b85c9	refactor(store): delete two unused item-workspace-move accessors (TASK-2374)	2026-07-31 17:28:05 +00:00
xarmian	98c638fc86	refactor(server): extract resolveAuthorizedCopy shared by preflight and copy (TASK-2370)	2026-07-31 14:43:40 +00:00
xarmian	cfc83e8c57	fix(server): report partial and legacy relationships in the copy dry-run (TASK-2369) ... Two ways the cross-workspace copy preflight told a user "nothing to lose" when there was, both violations of PLAN-2357 DR-17's "none of this may be silent". P1 — the five relationship counters are ACL-filtered by the caller's collection visibility (correct, and TASK-2364 chose it deliberately), but "none" and "none that you can see" rendered identically. A caller with edit rights on the source and none on its relatives could read `children_orphaned: false` and run a MOVE believing nothing was stranded, while hidden children were orphaned in place. The filtering stays; the uncertainty is now surfaced. Every point that drops a relationship for visibility reasons sets a new `warnings.relationships_partial` boolean. It is a BARE BOOLEAN by design: how many are hidden, of what type and in which collection are exactly the facts the filter exists to withhold, and a marker that varied with the hidden count would reinstate the leak DR-10a, DR-10b and the moved-to pointer each closed separately. A negative test asserts byte equality of the whole warnings block across two workspaces that differ only in how much is hidden. It is false for an unrestricted caller AND for a restricted caller with nothing hidden, so the common case renders exactly as it did before. P2 — a child reachable only by a lone legacy `plan` edge was invisible to GetChildItems (its join is restricted to store.ChildLinkTypes), so an incoming `plan` relationship reported `child_count: 0` / `children_orphaned: false` even though archiving the source strands it. The link scan now folds such an edge into the child set, deduplicated against the two mechanisms already covered and subject to the same visibility, liveness and workspace guards. The outgoing direction (the item's own parent) already reported correctly. The mutating copy reports no relationship counters at all (ItemCopyResultWarnings is deliberately narrower), so there is nothing for assertPreflightMatchesCopy to disagree about. CLI renders the qualifier on the five affected lines plus a plain-language explanation; TS types carry the field for Phase 3's dialog. Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT	2026-07-31 04:26:55 +00:00
xarmian	f15ba86db0	docs(server): correct the cross-workspace authz re-check contract per final review ... The helper's doc mandated that a mutating caller "re-apply the check" inside its write transaction. Its only mutating consumer deliberately does not, and is right not to: these functions read through s.store rather than the caller's tx, so under READ COMMITTED the re-check would judge locked resources against authorization state read at several unsynchronised moments — reading as a write-time guarantee while providing none. State what a mutating caller actually owes (re-read the authorized resource IDENTITY in-tx and refuse if it moved) and what it must not do, so the contract and copyResourceInvariantPreCheck no longer disagree. Found by the final full-diff Codex pass over PLAN-2357 (P1: a documented write-time guard was in fact a TOCTOU check). Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT	2026-07-31 02:35:53 +00:00
xarmian	f8ff5742e5	feat(server): add cross-workspace copy endpoint with post-commit fanout (TASK-2365) ... Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT	2026-07-31 00:59:25 +00:00
xarmian	01d640978c	feat(server): add cross-workspace copy dry-run preflight endpoint (TASK-2364) ... Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT	2026-07-30 20:15:00 +00:00
xarmian	1eb1c9eda6	feat(server): expose ACL-gated moved-to pointer on item GET (TASK-2359) ... An item MOVED to another workspace — copied, then archived — can now say where it went. GET on a single item gains an optional `moved_to` block naming each destination in displayable terms (workspace slug + item ref + title + collection slug), so a consumer can render a link without a second call. No HTTP redirect, no resolver change. The ACL gate is the point. A destination is revealed only after the caller independently passes AuthorizeCrossWorkspaceRead (TASK-2358) with an ITEM scope on the destination item itself. Workspace-level access is not sufficient: a restricted member of the destination workspace, or a guest holding one unrelated item grant there, has a role in that workspace while having no right to the copied item's collection. A caller who fails that check sees NO hint a destination exists. The key is omitted entirely — not a null, not an empty array, not a boolean — so the response is byte-identical to an archived item with no move record at all. A structurally distinguishable response is itself the leak. Restore decision: the block is OMITTED for a non-archived source. Restoring a moved-out source leaves two live items with the same content in two workspaces, which is legitimate, but at that instant the source has not moved anywhere and the response must stop asserting that it did. Past-tense provenance is the back-pointer question and applies equally to plain copies, which this field must never claim as moves. Also honored: DR-2a (only archived_source rows feed the pointer; plain copies are back-pointer material only), per-destination filtering over the forward lookup's SET with no short-circuit on the first hit or first denial, newest-first ordering, a scan bound on the per-GET authorization cost, and deliberate isolation of the hand-rolled public share-link DTO — pinned by an explicit negative test that freezes its key set. Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT	2026-07-30 12:20:38 +00:00
xarmian	5804c80146	feat(server): add cross-workspace authorization helper (TASK-2358)	2026-07-30 12:20:38 +00:00
xarmian	51cd6e84e4	fix(collab): op-id durable fence for restore-rollback vs applier-ack race (BUG-2276 residual 2) ... Closes the restore-rollback vs applier-ack clobber race with a durable operation-id correlation instead of a timing heuristic. The client brackets its setContent with an applier_apply_start{request_id} control frame; the server decides whether the external write persisted by reading the per-conn op-log high-water UNDER the same appendMu that sets the restore freeze (finalize-at-freeze — no drain, so a blocked write can't stall the restore; no timing window). Edges handled: unanchored conns are never elected; gate admission spans registration; legacy (pre-bracket) clients negotiate capability and an unconfirmable legacy round-trip returns a retryable 409 applier_ambiguous (fail-safe, never a clobber); the applier callback is synchronous-by-type so nothing can split the bracket. Normal acks stay on a lock-free, latency-identical fast path. Confirming Codex (high effort): redesigned from a timing grace after review; 3 rounds on the op-id design (2 P1 -> 3 P1+P2 -> CLEAN/converging). E2E + Go(PostgreSQL) green; go test -race clean 8x. Go/Web CI red only on the pre-existing dependency advisories (BUG-2278). https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-21 20:51:19 -04:00
xarmian	e601f2b368	fix(collab): reconcile Postgres commit-ack-loss on version restore instead of treating it as rollback (BUG-2276 residual 1) ... On Postgres, a version-restore commit that durably lands but whose ack is lost surfaced as an error and wrongly resumed peers on a stale Y.Doc. ForceRefreshRoom now runs a Postgres-only reconcile after a commit error: two durable signals (content == restored version AND last_restore_seq advanced past a lock-captured baseline) must agree → LANDED (publish fences + reseed, return the restored item + SSE); both false → rolled back (unfreeze); disagree/read-error → UNCERTAIN (invalidate in-memory fences so durable state governs, then plain-close sockets so peers reconnect + re-evaluate). SQLite path unchanged. Confirming Codex (high effort): 3 rounds — false-404, frozen-forever, archive-nil, stale-baseline, stale-in-memory-fence-clobber all closed; real Postgres end-to-end ack-loss + SSE test. make test-pg green. Residual 2 (applier-ack rollback race) follows separately. Go CI red only on the pre-existing govulncheck advisory (BUG-2278). https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-21 18:00:49 -04:00
xarmian	40f88052cd	fix(collab): version restore via prune+reseed (BUG-2264) (#990) ... Version restore didn't reconcile the live Y.Doc: peers kept editing a Y.Doc built on pre-restore ops, and their next collab-snapshot flush clobbered the restored items.content. Reworked restore to prune+reseed — the restored content becomes canonical and every peer converges on it (unflushed edits are discarded, which is exactly restore semantics), replacing the earlier applier/epoch/watermark routing. handleRestoreItemVersion drives RoomManager.ForceRefreshRoom under the per-item lock. Hardened across Codex xhigh review rounds: - Atomicity: pre-prune MAX(op-log), the items.content write, the "Restored from…" version, the op-log wipe, AND both durable restore boundaries all run in ONE store transaction. A failed commit rolls back all of it — no divergent state, no fail-open boundary. - Unambiguous commit signal: UpdateItem reads the updated row WITHIN the tx (getItemTx) before commit, so a read failure can't make a committed update look failed and the returned seq is this restore's. - Restore freeze: conns are paused via a dedicated rc.frozen flag (NOT canWrite) so the auth-revalidation loop can't thaw the freeze mid-restore or promote a viewer; pickApplier + the applier-ack handler reject frozen conns so a concurrent external PATCH can't falsely succeed. - Stale-flush boundary: pre-prune MAX+1 fences in-flight snapshot cursors under the same item lock. - force_refresh fan-out deadlock: per-conn timer-close so a wedged writeLoop can't hang the fan-out + item lock. - Stale-SEED clobber: the client announces the item.seq it seeded from (?content_seq=) on every (re)connect; Join force_refreshes any seed that predates the last restore. Residual #1 (restart-durability) CLOSED durably, for BOTH stale vectors — the in-memory fences didn't survive a restart, so a surviving cursor-0 pre-restore browser tab wasn't fenced on reconnect. Two nullable per-item columns (migration 075 SQLite / pg 053), both stamped in the restore's own tx (atomic with the content write + op-log prune): * items.last_restore_seq — the content generation. Join's stale-seed fence reads it (via store.ItemLastRestoreSeq) when the in-memory fast-path misses (after a restart); if that read errors, Join fails CLOSED via a RETRYABLE plain close (not a force_refresh, which would discard the Y.Doc and spin an unbounded refresh loop) so the client reconnects with backoff, Y.Doc intact. * items.restore_boundary_op_id — the op-log-id boundary. The collab-snapshot flush gate reads it (via store.ItemRestoreBoundaryOpID) when the in-memory RestoreBoundary misses (after a restart), failing closed (409) on a read error, so a surviving tab's stale HTTP flush is fenced too. No SCHEMA_VERSION bump — durable columns are not a Y.Doc node-spec change. Deferred to BUG-2276: (a) a Postgres commit whose ack is lost is treated as rolled-back (needs commit-outcome reconciliation; SQLite unaffected); (b) a restore rollback racing an in-flight external-applier ack can drop the ack and retry/fall back (needs the applier flow serialised under itemLock at a 30s-stall cost). NOTE(BUG-2270): ForceVersion can mint same-second version rows; the item_versions ordering tie-breaker is tracked separately. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-21 15:21:57 -04:00
xarmian	1dbe04399a	fix(store): optimistic concurrency + sibling broadcast for collection settings (BUG-2265) (#989) ... * fix(store): optimistic concurrency for collection settings writes (BUG-2265) Collection-level settings (e.g. quick_actions) were written by reconstructing the whole settings JSON from a caller's local Collection snapshot, and UpdateCollection replaced the column with no concurrency check. Two ItemDetails in the same collection (full-page pane host master + pane) hold independent snapshots and clobbered each other. Mirror the item optimistic-concurrency pattern (IDEA-1480): add CollectionUpdate.ExpectedUpdatedAt; when set, UpdateCollection re-reads updated_at atomically under the workspace write lock (SQLite BEGIN IMMEDIATE / Postgres advisory xact lock) and returns CollectionUpdateConflictError on a mismatch. Empty token keeps the legacy last-write-wins path unchanged for CLI/MCP/API callers. No DB migration — reuses collections.updated_at. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * feat(server): collection.updated broadcast + 409 conflict mapping (BUG-2265) - handleUpdateCollection boundary-validates expected_updated_at (400 on a malformed token) and maps store.CollectionUpdateConflictError to the shared update_conflict envelope (HTTP 409) — byte-identical wire shape to the item path, via the extracted writeUpdateConflictEnvelope helper. - Add the collection_updated EventBus type and publish it after a successful update so sibling ItemDetails / collection pages refresh their independent Collection snapshot proactively, shrinking the 409 window. Routed by Collection (slug) through the existing SSE visibility filter. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(web): 409-aware collection settings writes + sibling refresh (BUG-2265) - CollectionUpdate carries expected_updated_at; add isUpdateConflictError. - QuickActionsMenu sends the token and, on a 409, refetches the collection, re-appends the new action onto the FRESH settings, and retries once — no silent loss, no user-visible error. - EditCollectionModal captures the token at open-time (edge-gated seed so a concurrent broadcast can't wipe in-progress edits) and shows a non-destructive "changed elsewhere, reload" message on 409 rather than auto-merging a full-form edit. - Subscribe to collection_updated over SSE: ItemDetail and the collection page refresh their own Collection snapshot (gen/slug-fenced against the persistent pane host's no-remount switch), so siblings converge before the next save. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(store): harden collection optimistic concurrency + web fetch ordering (BUG-2265, Codex round 1) Address Codex review findings: - [P1] same-second clobber: now() is one-second precision, so two guarded writes in the same second kept an identical token. The accepted write now advances updated_at strictly past the token (only when now() hasn't already moved on), making a stale-token replay deterministically conflict. Add a same-second regression test. - [P1] tokenless-writer race on Postgres: the advisory lock only serialized writers that also took it. Replace it with a `FOR UPDATE` row lock on the in-tx re-read (Postgres) — SQLite's BEGIN IMMEDIATE already serializes every writer — so a concurrent tokenless UpdateCollection can't slip between the re-read and the UPDATE. - [P2] rename broadcast: only publish collection_updated when the slug is unchanged. A rename's old-slug event would make siblings refetch a dead slug (404) and a new-slug event can't reach old-slug visibility snapshots; renames are handled by the existing navigation path. - [P2] out-of-order refreshes: ItemDetail and the collection page now use a dedicated monotonic refresh counter so two rapid collection_updated fetches can't resolve out of order and clobber newer state (loadSeq/loadGeneration only bump on route/item loads). Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(store): make collection updated_at strictly monotonic for ALL writes (BUG-2265, Codex round 2) The previous same-second advance ran only on guarded updates, so a tokenless UpdateCollection could write the current second over a forced expected+1s, regressing the concurrency token and letting a stale guarded client clobber newer data (Codex P1). Route every collection update through one small transaction that re-reads updated_at (FOR UPDATE on Postgres; SQLite BEGIN IMMEDIATE covers it) and derives the new timestamp atomically: strictly advance past the row's current value when now() hasn't already moved on. This makes updated_at a reliable concurrency token for guarded AND tokenless writers. Add a tokenless-monotonic regression test. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: close remaining collection-concurrency gaps (BUG-2265, Codex round 3) - [P1] Board column reordering (handleGroupReorder) rebuilt the full schema from a stale local snapshot and wrote it with no token — a lost-update path identical to the bug being fixed. Now sends expected_updated_at and, on 409, refetches, re-applies the reorder onto the fresh schema, and retries once. - [P2] The workspace settings page seeded EditCollectionModal from a page-load-time collections list, so a change that predated editing produced a false 409. It now refreshes the list on collection_updated (seq-guarded). - [P2] collection.updated is now delivered to item-grant-only SSE subscribers for collections they can see — it's itemless but leak-free (only the slug), so guests' ItemDetail schema/settings snapshots converge too. Filter test extended. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(web): switch-safety + conflict-merge fixes for collection writes (BUG-2265, Codex round 4) - Board column reorder now ABORTS on a 409 (with a "reorder again" toast) instead of replaying a stale option order onto the fresh field, which would silently drop a concurrent option add/remove/rename. Reordering is cosmetic; never worth clobbering a real schema edit. Also captures ws/slug/base before the await and fences the write against a route switch. - QuickActionsMenu captures workspace + collection identity BEFORE the first await, so a mid-save navigation can't make the 409 refetch/retry target the wrong collection (no guaranteed remount). - Settings-page SSE refresh captures the workspace and drops the result if the workspace changed while fetching, so a slow refresh for workspace A can't overwrite workspace B's freshly loaded collection list. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(store): sub-second collection updated_at token, no future drift (BUG-2265, confirming pass #6) The same-second monotonic advance manufactured whole-second FUTURE updated_at values; sustained >1 write/sec on one collection drifted arbitrarily ahead of wall-clock. collections.updated_at is TEXT on both dialects and never compared lexically (only via time.Equal + display), so switch the update write to sub-second nowNano(): same-second collisions become near-impossible, so the token advances naturally. Keep a strict-monotonicity guard but step by a single NANOSECOND on the (now near-impossible) coarse-clock/step-back collision, so any drift is bounded to nanoseconds. Dual-dialect; covered by make test-pg. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(server): sanitize + always-broadcast collection event (BUG-2265, confirming pass #2,#3) - #2 (P1): collection_updated is delivered to item-grant guests, but the event carried ActorName/Source, leaking the owner's identity + edit source. Strip them — publishCollectionEvent now emits workspace + slug (+ new_slug) only. - #3 (P2): always broadcast (including on rename), routed by the OLD slug and carrying the NEW slug via a new Event.NewSlug field, so remote tabs on the old slug can re-target instead of silently 404ing on their next action. Tests: assert no actor/source leak on a settings update; assert a rename routes by old slug + carries new_slug. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(web): decisive switch-safety + rename handling for collection writes (BUG-2265, confirming pass #1,#3,#4,#5) - #1 (P1): EditCollectionModal captures the target collection id/slug/name/ws + updated_at when the form is SEEDED, and handleSave/handleArchive now operate on that captured identity (not the live props). The seed effect re-seeds when the collection IDENTITY changes (not on a same-id broadcast refresh), so a reused route can't leave A's form saving/deleting to B. - #3 (P2): on a rename event the collection route navigates to the new slug (preserving the pane query) and ItemDetail refetches by new_slug; the SSE event type carries new_slug. - #4 (P2): the reorder-conflict path refetches the collection (reseeds the token) before prompting, so a missed SSE event doesn't make every retry 409 forever. - #5 (P2): QuickActionsMenu only invokes oncollectionupdated when the live workspace/slug still match the captured identity, so a reused route can't assign an old response to the newly-navigated page. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(server): nano token round-trip, rename visibility, publish-before-migration (BUG-2265, confirming pass 2) Address the round-2 confirming-pass findings (server-only): 1. (P2) The shared update_conflict envelope formatted actual_updated_at with second precision (time.RFC3339), truncating the now sub-second collection token so the client's retry token never matched — a permanent 409 loop. Format with time.RFC3339Nano. Item tokens are zero-nanosecond, so RFC3339Nano emits no fractional part — the item 409 wire shape is byte-identical and the item path still compares via time.Equal. Added a test that the returned token round-trips as a usable retry token. 2. (P2) Rename events are routed by the OLD slug, but a subscriber that revalidated after the rename only has the NEW slug in visibleSlugSet, so the visibility check dropped the event before the new_slug branch. Accept a rename when EITHER the old slug or the (authorized) NewSlug is visible; downstream item-grant gating uses whichever slug is visible. Filter test extended. 3. (P2) The collection_updated event was published only after field migrations succeeded, but UpdateCollection already committed (updated_at advanced). On a migration failure clients got a 500 and no refresh, leaving siblings with stale tokens that 409 blindly. Publish on the commit (before the migration), so siblings always resync regardless of migration outcome. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: atomic collection update+migration; modal same-id rename retarget (BUG-2265, confirming pass 3) Address the round-3 confirming-pass findings; defer cross-tab rename RE-NAVIGATION to BUG-2272 (placeholder) per coordinator. 1. (P1) Migration atomicity. UpdateCollection committed the schema + concurrency token BEFORE MigrateItemFieldValues ran, so a migration failure returned 500 with the row already changed → the retry was guaranteed to 409 and item values were left inconsistent with the committed schema. Made the two ATOMIC: extracted applyFieldMigrationsTx and run it inside UpdateCollection's own transaction (after taking the workspace seq lock), so a migration failure rolls back the schema AND the token — nothing changes, the retry works. The handler now passes migrations through instead of running them separately, and publishes the event only after the fully-atomic commit. store/tx work → make test-pg run green. 2. (P2) EditCollectionModal same-id rename. The round-1 identity capture ignores same-id prop refreshes (to preserve edits), but a concurrent RENAME changes the slug (not the id), so handleSave/handleArchive PATCHed a dead slug → 404 before the token could 409. On a same-id prop change whose slug changed, the seed effect now retargets the endpoint slug + re-captures the token WITHOUT reseeding the form (in-progress edits preserved). Deferred (BUG-2272, TODO comments added, already broken on main — no regression): - ItemDetail full-page item URL/collSlug not retargeted after a remote rename. - Collection route chained-rename events during SSE replay landing on a dead intermediate slug. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(web): keep seeded token on same-id rename retarget (BUG-2265) On the EditCollectionModal same-id rename branch, retarget the endpoint (slug/name/ws) only — drop the token re-capture. Re-capturing let a later handleSave succeed against the renamed collection and apply the modal's stale pre-rename full form, silently REVERTING the concurrent rename (the exact stale-snapshot clobber BUG-2265 prevents). Keeping the seeded token means a concurrent rename correctly yields a 409 → the non-destructive "collection changed, reload" message. Slug-retarget without token-recapture gives both: no 404 (right URL) and no clobber (409 fires). Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: lock-order deadlock + unified collection-snapshot fences (BUG-2265, confirming pass 4) 1. (P1) DEADLOCK regression. UpdateCollection's atomic migration path took the collection-row FOR UPDATE lock and THEN the workspace seq lock, but item creation takes them in the reverse order (workspace advisory lock first, then the collection-row FK lock on INSERT) — a concurrent item-create + schema-migration ABBA-deadlocks on Postgres. Fix: acquire the workspace seq lock BEFORE the collection-row FOR UPDATE (matching item-create's order). Every store path that locks both now takes them workspace-seq → collection-row (tryCreateItem, UpdateItem, MigrateItemFieldValues, UpdateCollection). Added a concurrency regression test (item-create racing schema-migration); make test-pg green. 2+3. (P2) Cross-generation fence gap. The SSE collection refresh and route/item loads used SEPARATE counters, so a stale in-flight load could complete after a fresh SSE refresh and revert the collection + its concurrency token. Unified to a SINGLE monotonic collection-snapshot generation in BOTH the collection route and ItemDetail — every collection-snapshot write (loadCollection/loadData, the SSE refresh, reorder, and the quick-action/edit-modal callbacks) bumps it on start and gates its assignment on "still latest". ItemDetail's load keeps a switch-escape so a stale refresh for the OLD collection can't block loading a NEW one. Settings page unified the same way over its collections-list writes. 4. (P2) Settings page fed a stale editingCollection to the edit modal after a remote rename (its prop never changed → the same-id-rename retarget never fired → 404). The unified refresh now re-points editingCollection at the refreshed object for the same id, so the modal's retarget fires. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: emit item-changes signal on field migration; QuickActions retry-by-id (BUG-2265, confirming pass 5) 1. (P1) A collection update that runs a field migration mutates item `fields` JSON and advances item `seq`, but only collection_updated was published — open item views refreshed collection METADATA and returned without reconciling the migrated items, so clients kept stale field JSON under the new schema and a later full-fields item update could UNDO the migration (a clobber). UpdateCollection now returns the migrated-item count; when > 0 the handler ALSO emits the existing bulk item-mutation signal (items_bulk_updated, Op=migrate) so open views reconcile via /items-changes. Fires only when the migration touched >= 1 item — a pure settings/quick-actions update emits nothing extra. No store SQL/locking change (Go signature + count plumbing only); make test / make test-pg both green. 2. (P2) QuickActionsMenu's 409 retry GET-by-slug 404s if the competing update renamed the collection. Resolve the fresh collection by STABLE id (list + find by id) before re-appending + retrying, mirroring EditCollectionModal's identity approach; the result-propagation guard is now id-based too so a rename doesn't spuriously drop it. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: uniform sweep of item-grant delivery, rename routing, and 409/404 retries (BUG-2265, confirming pass 6) One pattern-sweep instead of per-site patches. Audited every event this PR publishes and every client retry path, applying three patterns uniformly: PATTERN A (item-grant SSE reconcile) + B (old-slug rename routing): instead of a SEPARATE items_bulk_updated migration event (which carries op/count for items an item-grant subscriber can't see and isn't rename-routed), FOLD a SANITIZED `items_changed` bool onto collection_updated — already item-grant-delivered (round 3) and already old-slug-routed with new_slug (round 2). On it the client triggers a /items-changes deltaSync (server-filtered to the caller's grants) and ItemDetail refetches its open item, so item-grant EDITORS reconcile migrated field JSON — closing the clobber where a stale full-fields update would UNDO the migration. Leak surface: "a collection you can see items in changed [+ renamed + had item changes]" — a bool, no per-item data. Removed the round-5 items_bulk_updated publish. The pre-existing items_bulk_updated (archive/move) is untouched and correctly stays suppressed for item-grant users. PATTERN C (409 AND 404 in retries): a competing RENAME can 404 a slug-targeted write before it can 409, bypassing recovery. Added isNotFoundError / isConflictOrNotFound helpers; every write/retry path now treats BOTH: QuickActions save resolves-by-id and retries on either; board reorder reseeds-by-id and aborts on either; EditCollectionModal save shows the reload prompt and archive resolves-by-id and retries on either. Tests: server asserts collection_updated sets items_changed on migration (not on settings-only) and stays sanitized; the SSE-filter test asserts the migration variant reaches item-grant subscribers for a visible collection; web unit tests assert 404/409 classification and a real component-driven not_found -> resolve- by-id -> retry in QuickActionsMenu. make test / make test-pg / npm run test all green. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: stable collection-ID identity for collection events + request-based items_changed (BUG-2265, confirming pass 7) 1. (P1) Collection events were identified only by MUTABLE, reusable slugs, and events replay — so a stale rename event's old slug, once re-owned by a DIFFERENT collection, could pass a slug-based match and misroute a client (navigate away / load the wrong schema) or leak the new slug. Fix at the ROOT: carry the STABLE CollectionID on collection_updated (Event.CollectionID) and match by ID everywhere: - Server visibility: sseEventVisibleFor matches collection_updated on a new visibleCollIDSet (built from the same VisibleCollectionIDs), not the slug — so an event for a collection the subscriber can't see by ID is dropped even if its (reused) slug is in visibleSlugSet. Filter test proves the slug-reuse drop. - Clients: ItemDetail and the collection route match `event.collection_id === <their collection>.id`, not slug. Slug(s)/new_slug stay only for the rename-navigation URL. Settings refreshes its whole list (already id-safe). 2. (P1) items_changed was keyed off the affected-ROW count, delivered to item-grant subscribers → a subscriber whose own items were unaffected could infer that HIDDEN items matched the migrated value. Now keyed off whether a field MIGRATION WAS REQUESTED (len(input.Migrations) > 0), independent of row count — leaks nothing about hidden item values. Reverted round-5's UpdateCollection count-return (no longer needed). Test: a migration matching ZERO items still sets items_changed. Deferred with markers: - NOTE(BUG-2273) at ItemDetail's reconcile-skip AND updateField: the web editor's full-fields field write lacks item-level OCC (never adopted IDEA-1480/v0.14), so the migration reconcile is best-effort. - TODO(BUG-2272) at the reorder 404 reseed: it refreshes `collection` but not the route `collSlug` (renavigation, deferred). Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix: archive OCC (no destructive wrong-target) + settings load fence (BUG-2265, confirming pass 8) 1. (P1) EditCollectionModal handleArchive resolved the target by stable id but the server DELETE re-resolves by the MUTABLE slug — a rename that re-owned that slug before the delete landed would archive the WRONG collection. Close the TOCTOU with an expected_updated_at OCC on the delete, mirroring the update OCC: DeleteCollection re-reads updated_at under a lock (FOR UPDATE on Postgres) and 409s on mismatch; the handler validates the token + maps the 409; the client sends it as a query param; handleArchive passes the seeded token (and the fresh token on the resolve-by-id retry). A reused slug or a concurrently-changed target now yields a clean 409 → the reload message, never a wrong-collection archive. Server test: stale token 409s (and the collection survives); current token 204s; malformed 400s; no token 204s. 2. (P2) settings load(): the generation was bumped AFTER awaiting setCurrent, so a slow load for workspace A could resume after B's load and clobber B's name/context/collections/members. Capture a dedicated loadGen at load() ENTRY (before any await) and fence EVERY continuation on it; the collections write additionally respects collectionsGen so it can't revert a fresher SSE refresh. Using a dedicated loadGen (not the SSE-shared collectionsGen) means an SSE collections-refresh mid-load doesn't drop the name/members writes. Deferred: TODO(BUG-2272) at the collection route's rename-navigation site — the global collectionStore (sidebar/pickers) isn't refreshed and the workspace layout ignores collection_updated, so the sidebar keeps the dead slug. Layout- level renavigation, deferred. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(web): dedicated item-snapshot fence + id-based rename comparisons in ItemDetail (BUG-2265, confirming pass 9) One comprehensive ItemDetail async-snapshot fence sweep so this file's item/ collection fencing is uniform and ID-based. 1. (P1) The migration item-refetch and loadData shared loadGeneration, so the refetch could apply migrated fields and then a stale loadData response overwrite them (a later full-fields edit then undoes the migration). Added a DEDICATED itemGen (separate from loadGeneration and collectionGen), bumped at the start of BOTH loadData's item load AND the migration refetch, and gated BOTH `item = ` writes on "still latest itemGen" — neither can stale-overwrite the other. Swept the other PASSIVE item snapshot-refreshes onto itemGen too (SSE item_updated/archived/restored, onSync deleted/incremental/full, the collab refresh) so they're ordered against each other and the migration/load. 2. (P2) A settings update that follows a rename before the rename fetch completes requested the OLD slug and bumped collectionGen, cancelling the valid rename fetch. Fetch slug is now `event.new_slug || event.collection || slug`. 3. (P2) The loadData collection fence-escapes compared the stale load's SLUG vs the freshly-renamed snapshot's slug (they differ on a rename → escape let the stale result overwrite). They now compare stable collection IDs; the SSE refresh's post-fetch identity check is id-based too. Audit (site -> generation -> id?): every PASSIVE snapshot-refresh (loadData item+collection, migration refetch, SSE x3, onSync x3, collab) bumps the correct dedicated gen (item->itemGen, collection->collectionGen) and compares identity by id. The DELIBERATE user/action writes (title/field/tag/assignee/role/content/ link/version/restore saves) keep loadGeneration + item-id switch-safety; their item-snapshot concurrency vs the migration refetch is the deferred item-OCC gap (BUG-2273, best-effort) — reordering them last-started-wins is orthogonal to that. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-21 09:43:05 -04:00
xarmian	bcef802335	fix(security): gate collab WebSocket writes on editor role (TASK-265) (#938) ... * fix(security): gate collab WebSocket writes on editor role (TASK-265) The collab WebSocket (GET /api/v1/collab/{itemID}) is mounted outside the /{slug} subrouter, so RequireWorkspaceAccess never runs on it. authorizeCollabAccess gated admission on membership + item visibility but NOT edit role, so a plain workspace VIEWER was admitted and could WRITE: every inbound Yjs sync frame persisted to item_yjs_updates (room.go) and got canonicalized into items.content when a co-present editor's authorized flush ran. The REST write path blocks viewers via requireEditPermission; this closes the equivalent gap on the collab relay. Fix — non-editors become READ-ONLY participants (not hard-rejected, so live view + presence stay intact): - authorizeCollabAccess now returns a collabAccess{canWrite} alongside the admission decision. canWrite is computed once via store.ResolveUserPermission (the same predicate requireEditPermission falls back to): owner/editor membership grants write; a viewer/guest gets write only through a collection/item edit grant. - RoomManager.Join takes a canWrite flag stored per-connection as an atomic.Bool. room.go's readLoop drops a read-only conn's inbound sync frames (not persisted via AppendYjsUpdate, not rebroadcast); awareness (presence) frames still relay so the viewer's cursor stays visible, and outbound broadcasts from editors still reach the viewer. - The handler's periodic revalidation pushes mid-session write-permission changes via a new RoomManager.SetConnWritable, so an editor demoted to viewer becomes read-only without a reconnect (complements the existing CloseConn-on-revocation path). No SCHEMA_VERSION / DefaultSchemaVersion bump: this is an authorization / behavioral change, not a ProseMirror/Y.Doc node-spec change, so the op-log must not be pruned. Tests: TestCollabViewerIsReadOnly (viewer admitted 101, receives an editor's broadcast, but its own sync frame is neither persisted nor broadcast while the editor's is) and TestAuthorizeCollabAccessCanWrite (viewer→canWrite=false, editor→canWrite=true). Verified the E2E test fails with the gate removed. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): close 4 collab read-only gaps from orchestrator review (TASK-265) Independent Codex pass on the collab editor-role gate found four gaps: [P1] Read-only conns were still eligible designated APPLIERS. A viewer (or an editor demoted mid-session) could be elected to apply an external content edit; its resulting sync frames were dropped by the new gate, yet its applier_ack was accepted → ApplyExternalContent reported success → the PATCH handler skipped its direct-write fallback → the external edit was silently lost. Fix: pickApplier now skips non-writers, and handleControlMessage ignores applier_ack from a conn whose canWrite is false (belt-and-suspenders so the fallback fires). [P2] Demotion TOCTOU. readLoop read canWrite=true, then could block on appendMu and persist AFTER SetConnWritable(false) returned. Fix: the canWrite check now runs INSIDE the appendMu critical section, and SetConnWritable stores the flag under the same appendMu — so a frame racing a demotion is either fully persisted before the flip or dropped. [P2] Revalidation could run before the conn was registered. The first jittered tick could fire while Join was still setting up; SetConnWritable would no-op against the unregistered conn and Join then installed the stale canWrite=true until a later tick. Fix: Join takes an onRegistered callback invoked right after addConn; the handler gates the reval loop on it so the first SetConnWritable always finds the conn. [P2] canWrite didn't mirror REST for editors/owners. It was computed purely from ResolveUserPermission, which resolves item/collection GRANTS before membership role — so an editor/owner holding an incidental `view` grant was wrongly made read-only. Fix: mirror requireEditPermission exactly — editor/owner MEMBER short-circuits to canWrite=true BEFORE grant resolution; viewers/guests still fall back to ResolveUserPermission so grants can override an insufficient role. Tests: TestApplyExternalContentSkipsReadOnlyApplier (verified failing without the pickApplier gate), TestHandleControlMessageIgnoresAckFromReadOnlyConn, TestCollabDemotionMakesConnReadOnly (mid-session demotion → read-only without reconnect), and two new TestAuthorizeCollabAccessCanWrite cases (editor+incidental view grant → true; viewer+edit grant → true). Gates: build, gofmt, vet, golangci-lint (0 issues), go test ./internal/server/ ./internal/collab/, and go test -race ./internal/collab/ all pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): safe no-applier direct write for read-only-only collab rooms (TASK-265) Codex round 2 found a P1 introduced by excluding viewers from applier election: in a room whose only peers are read-only, an external content update (PATCH) hits ErrNoApplierAvailable, then PruneAndApply refused to prune because live conns existed (len(r.conns) > 0). After the retry budget the PATCH handler fell through to an UNLOCKED, UN-PRUNED direct write — items.content was updated but the stale op-log survived, so a fresh editor replaying it (or a viewer promoted to editor flushing its stale in-memory Y.Doc) would silently overwrite the external update. Fix: PruneAndApply now blocks only on a live WRITER peer — a read-only peer can never persist, so it doesn't force the unsafe fallback. After the prune + write succeeds it evicts the read-only peers (Room.closeReadOnlyConns: WriteControl close frame + Close, concurrency- safe with writeLoop) so their now-stale Y.Doc can't linger; they reconnect and lazy-seed from the fresh items.content (their old resume cursor is below the pruned op-log's MIN → force_refresh). Mixed rooms (an editor present) are unaffected — the editor is still elected applier and PruneAndApply is never reached. Tests: TestPruneAndApplyEvictsReadOnlyRoom (read-only-only room prunes + evicts, applyFn runs) and TestPruneAndApplyBlockedByLiveWriter (a live writer still yields ErrRoomActiveDuringPrune). Gates: build, gofmt, vet, golangci-lint (0 issues), go test ./internal/server/ ./internal/collab/, and go test -race ./internal/collab/ all pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): fence PruneAndApply read-only eviction under appendMu (TASK-265) Codex round 3 P1: PruneAndApply classified writers, ran applyFn (prune + write), and evicted read-only conns WITHOUT holding room.appendMu. A concurrent viewer→editor revalidation could set canWrite=true after the writer check, append a stale frame during the prune/write, and — now a writer — evade closeReadOnlyConns, racing the prune and leaving a live stale Y.Doc that overwrites the external update. Fix: PruneAndApply now holds room.appendMu across the ENTIRE sequence (writer classification + applyFn + eviction). appendMu is the same lock readLoop takes across its canWrite-check+persist and SetConnWritable takes when flipping canWrite, so a promotion can no longer interleave with the classification/prune. Lock order is itemLock → appendMu → room.mu; no path takes room.mu → appendMu, so no inversion. Also closes the residual "frame already read, blocked on appendMu, then promoted after release" window: roomConn gains a terminal `evicted` atomic flag set by closeReadOnlyConns (under appendMu) and checked in readLoop's persist gate alongside canWrite, so an evicted read-only conn's in-flight frame is dropped even if a racing revalidation promotes it in the same instant. Gates: build, gofmt, vet, golangci-lint (0 issues), go test ./internal/server/ ./internal/collab/, and go test -race ./internal/collab/ all pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * refactor(collab): descope read-only eviction; keep writer-aware prune guard (TASK-265) Per orchestrator scope decision, remove the read-only EVICTION machinery added during review (over-engineering for TASK-265's security goal): - Room.closeReadOnlyConns and its call in PruneAndApply. - roomConn.evicted flag and its check in readLoop's persist gate. - appendMu held across PruneAndApply + the force-close socket I/O. PruneAndApply reverts to no appendMu / no socket I/O, keeping only the LOAD-BEARING writer-aware guard: it blocks (ErrRoomActiveDuringPrune) only on a live WRITER peer, not any conn. An all-viewer room's external edit therefore still prunes + direct-writes safely (op-log pruned, so a fresh editor lazy-seeds from the new items.content) instead of erroring. The residual — a connected read-only peer keeps a possibly-stale Y.Doc until reconnect/refresh, and a viewer promoted to editor before re-sync could push stale content — is a low-severity lost-update edge (a promoted viewer is a legitimate editor), consistent with the pre-existing direct-write contract. Documented on PruneAndApply and tracked in BUG-2103 (proposed fix: proactive re-seed/refresh of remaining read-only peers). Kept unchanged: the authorizeCollabAccess canWrite editor/owner role short-circuit, dropping read-only inbound sync frames under appendMu + the SetConnWritable demotion fence + registration ordering, and the pickApplier / applier_ack read-only exclusions. Tests: replace TestPruneAndApplyEvictsReadOnlyRoom with TestPruneAndApplyAllowsReadOnlyRoom (read-only-only room -> applyFn runs); keep TestPruneAndApplyBlockedByLiveWriter. Gates: build, gofmt, vet, golangci-lint (0 issues), go test ./internal/server/ ./internal/collab/, and go test -race ./internal/collab/ all pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): enforce token write-scope + fence prune promotion on collab (TASK-265) Two logic gaps from the orchestrator's final pass: [P1] canWrite ignored BEARER-TOKEN SCOPE. The collab upgrade is a GET, so a read-scoped PAT/OAuth token passes TokenAuth's method-keyed tokenScopeAllows check, then rode the user's editor role (or the legacy workspace-token grant) to canWrite=true and could persist Yjs mutations over the socket — a read-only-principal-writes bypass via token scope instead of role. REST DOES enforce write-scope (TokenAuth → tokenScopeAllows blocks read-scoped tokens from PATCH/POST/DELETE); the collab GET simply slips the method gate. Fix mirrors REST: TokenAuth now stashes the token scopes (WithTokenScopes, as MCPBearerAuth already does), and authorizeCollabAccess re-applies the write-capability half — canWrite is downgraded to read-only when the caller's token scope doesn't permit writes (http.MethodPost representative verb). Applied to both the legacy workspace-token path and the member/grant path. Non-token principals (cookie / CLI session, fresh install) carry empty scopes → unrestricted → unaffected. Test: an editor with a read / pad:read token gets canWrite=false; with write / * gets canWrite=true. [P2] PruneAndApply's writer-scan was not serialized with SetConnWritable, so a viewer promoted during applyFn could append a stale frame while the op-log is pruned + content written (persist/prune ordering race, distinct from BUG-2103's async residual). Fix: hold room.appendMu across the writer-scan AND applyFn. Safe now that eviction/socket-I/O is gone — applyFn is a pure store op (PruneYjsUpdatesBefore + UpdateItemWithParentLink, the only caller) that never re-enters itemLock / appendMu / room.mu, so no inversion or re-entrant deadlock. Lock order: itemLock → appendMu → room.mu. Gates: build, gofmt, vet, golangci-lint (0 issues), go test ./internal/server/ ./internal/collab/, and go test -race ./internal/collab/ all pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): honor token write-scope in collab fresh-install branch (TASK-265) The zero-user (pre-bootstrap) branch of authorizeCollabAccess returned canWrite=true unconditionally. A legacy workspace token still carries a scope on a fresh instance, so a read-scoped token could persist Yjs mutations over the collab GET upgrade — inconsistent with REST, whose method gate blocks a read token's mutation. Route the branch through collabTokenWriteScopeAllowed, which returns true for the anonymous (no-token) setup caller (empty scopes = unrestricted) and false for a read-scoped token. Adds TestAuthorizeCollabAccessFreshInstallTokenScope. Found by the orchestrator's independent Codex pass. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-15 01:30:12 -04:00
xarmian	2aa4f141e3	fix(security): require workspace membership on SSE subscriptions (TASK-264) (#937) ... The SSE events stream (`GET /api/v1/events?workspace=`) resolves the workspace via resolveWorkspace. The slug form is membership-scoped (GetWorkspacesBySlugForUser → nil → 404), but the UUID form resolves through the GLOBAL, unscoped GetWorkspaceByID. So an authenticated non-member passing another workspace's UUID reached SubscribeIfAllowed with a fully-resolved workspace and no explicit membership check. Events are still fail-closed filtered by computeSSEVisibility and the stream is torn down within ~60s by the revalidation tick, so it is not a data leak — but the open connection is a connection-slot DoS and a workspace-existence oracle (200+connected vs 404), and it diverged from the explicit membership gate enforced by RequireWorkspaceAccess and the collab sibling authorizeCollabAccess. Add an entry membership/grant gate in handleSSE for regular (non-admin) user-context callers: admit only direct workspace members or guest-grant holders; otherwise return 404 (matching the slug path — a 403 would itself be an existence oracle). Admin-via-cookie keeps its platform-wide bypass; admin-via-bearer and the legacy-token / fresh-install paths are gated by the pre-existing branches above and left untouched. Regression test asserts a non-member is rejected with 404 at the SSE entry for BOTH the slug and UUID forms (and consumes no connection slot), while a legitimate member still connects (200 + connected). Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-14 23:43:19 -04:00
xarmian	8e16501e8a	fix(security): scope webhook + token mutations by workspace to close cross-workspace IDOR (TASK-266) (#936) ... * fix(security): scope webhook + token mutations by workspace to close cross-workspace IDOR (TASK-266) handleDeleteWebhook, handleTestWebhook, and handleDeleteToken looked their object up by ID with no workspace-ownership predicate. requireMinRole("owner") only proves the caller owns the URL's workspace — not that the {webhookID} / {tokenID} belongs to it — so any owner of any workspace could delete or test another workspace's webhook, or revoke its API token, given the object ID (cross-workspace IDOR / integrity + DoS + existence oracle). Fix, matching the existing pre-fetch-and-compare idiom used by handleDeleteWorkspaceAttachment / views / comments / links: - webhooks: pre-fetch via GetWebhook and 404 unless hook.WorkspaceID matches the URL workspace (delete + test paths). - tokens: new store.DeleteAPITokenScoped(id, workspaceID) doing DELETE ... WHERE id = ? AND workspace_id = ?, mirroring DeleteUserAPIToken. The unscoped DeleteAPIToken (its only caller) is removed. Adds TestWebhookTokenCrossWorkspaceIDOR: an owner of workspace B cannot delete/test A's webhook or revoke A's token via B's URL (404, objects survive), while the legitimate owner still can within their own workspace. Verified red-before/green-after. Part of PLAN-259 (pre-open-source security audit). Closes the webhook+token half of TASK-266; views/comments/links were already scoped. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): atomic workspace-scoped webhook delete per Codex review (round 1) handleDeleteWebhook pre-fetched via GetWebhook, which decrypts the HMAC secret. A rotated/missing encryption key or corrupted ciphertext would make the delete return 500, leaving a broken webhook undeletable. Replace the pre-fetch-and-compare with an atomic store.DeleteWebhookScoped(id, workspaceID) (DELETE ... WHERE id = ? AND workspace_id = ?) — same idiom as the token fix, no decrypt on the delete path. The unscoped DeleteWebhook (its only caller) is removed. handleTestWebhook keeps GetWebhook since it needs the decrypted hook to dispatch. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra * fix(security): scope test-webhook lookup before decrypt per Codex review (round 2) handleTestWebhook fetched via GetWebhook (SELECT + decrypt by ID) and only then compared workspace, so an undecryptable foreign webhook returned 500 rather than 404 — a residual cross-workspace existence oracle. Add store.GetWebhookScoped(id, workspaceID) which applies the workspace_id predicate in SQL before decrypting; a foreign/missing ID returns (nil,nil) → 404 without touching the ciphertext. Strengthen the regression test's victim webhook with a non-empty secret so the scoped-before-decrypt path is exercised. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-14 23:24:55 -04:00
xarmian	9f6c1d8f47	fix(security): scope MCP workspace-global reads by OAuth consent allow-list (BUG-2102) (#935) ... The OAuth token consent allow-list (TokenAllowedWorkspaces) was enforced only by RequireWorkspaceAccess, which fires solely for /{slug} path-param routes. Every MCP-reachable read that is workspace-global or takes the workspace as a query/body param bypassed the gate, so a token consented to workspace A could reach data in other co-membership workspaces. Investigation found five bypasses; this closes all of them: - pad_search (HIGH): fan-out (no workspace) searched ALL memberships; naming a workspace returned its item titles + content. Now the fan-out is restricted to the allow-list and a named non-consented workspace returns empty (no existence leak). - pad_workspace.list (the original BUG-2102): filtered by the allow-list. - pad_workspace.deleted: filtered by the allow-list. - pad_workspace.audit-log: platform-wide admin surface; denied for consent-scoped tokens. - pad_workspace.restore: gated by the allow-list (404 for out-of-consent slugs). All gates are no-ops for nil/wildcard allow-lists, so PAT auth, web sessions, and local stdio are unchanged. The allow-set semantics move into internal/server as the canonical TokenAllowedWorkspaceSet(ctx) (promoted from internal/mcp's buildAllowSet); the two mcp call sites (error-hint lister, workspaces resource) and its unit tests migrate with it, so server handlers and MCP filters share one implementation instead of drifting per-surface (the pattern that caused this bug: TASK-977 and TASK-2101 each point-fixed one surface). Tests: per-handler regression tests carrying the WithTokenAllowedWorkspaces context MCPBearerAuth produces; each asserts the consent layer (not membership) drives exclusion, with nil/wildcard baselines guarding against over-blocking. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra	2026-07-14 20:37:03 -04:00
xarmian	c72fe5a663	feat(items): add unparented filtering contract (#926) ... * feat(items): add unparented filtering contract * fix(items): preserve unparented projection state * fix(views): preserve reserved filter on reset * fix(items): resync projection scope changes * fix(items): address PR 926 review findings - localIndex: fetch snapshot before clearing store/cache in resyncProjectionScope (no data-loss window on fetch failure) - items: degrade to committed item when post-parent-link readback fails instead of 500 - items: treat unparented=<non-true> as a field filter so a schema field named unparented still filters - persistence: delete dead persistCursor - mark validateUnparentedListRequest canonical; cross-reference the 3 early-feedback copies * fix(items): resync race + purge safety per Codex review (round 1) - resyncProjectionScope: merge-reconcile instead of blunt clear so a higher-seq upsert/delta racing the snapshot fetch is preserved (not erased) and the cursor never regresses below it - recheck generation after persistWipe so a sign-out/403 purge during the wipe can't resurrect purged rows via persistDelta - snapshot rows authoritatively replace local copies (drop is_unparented on downgrade); mergeRow's projection-preservation is bypassed for resync * fix(items): sanitize projection bit on preserved racing rows per Codex review (round 2) When a projection resync lands a restricted snapshot, strip is_unparented from any racing higher-seq row kept by the seq guards — the old scope no longer grants it. Keep the row itself (dropping it would reintroduce the racing-mutation data loss; server 403 enforces real visibility). * fix(items): transactional cache replace in resync per Codex review (round 3) Replace wipe()+persistDelta() in resyncProjectionScope with a single persistReplace() transaction (clear + write in one tx). Avoids the deleteDatabase() onblocked cross-tab hang where a pending delete stalls the following reopen+write indefinitely, wedging the resync promise. wipe() stays for the sign-out / schema-mismatch full-teardown paths. * fix(items): drop-and-replay resync reconciliation per Codex review (round 4) Rework resyncProjectionScope: drop every row absent from the authoritative snapshot (not just older-than-cursor ones) and pin the cursor to the snapshot cursor. A post-snapshot mutation the client can still see is re-fetched by the next /items-changes?since=cursor under the NEW scope, so visible rows return and old-scope-hidden rows stay gone — no old-scope row survives the resync, and nothing is permanently lost. Present-in-snapshot racing edits are still kept (is_unparented stripped under a restricted scope). * fix(items): continue delta poll after resync so replay actually fires (round 5) The drop-and-replay resync (round 4) pins the cursor to the snapshot cursor so post-snapshot mutations re-fetch under the new scope — but both poll loops broke out / returned immediately after the resync, so the replay never ran until an unrelated sync/reload. Both callers now continue the loop from the pinned cursor; resync already aligned the scope so the branch can't re-fire, and the existing 50-iteration cap bounds it. * fix(items): keep pendingResync set until replay catches up (round 6) resyncProjectionScope cleared pendingResync after installing the snapshot but before the pinned-cursor replay drained. If that replay later failed or hit the 50-page cap, pendingResync stayed false and the next bootstrap() no-opped with racing mutations still missing. Let the reconcile loop's caughtUp logic own the flag instead. * fix(items): set pendingResync when any resync begins (round 7) Round 6 removed the premature clear but only the bootstrap path pre-sets pendingResync; a page deltaSync resync ran with it false, so a failed/capped replay there wouldn't trigger a bootstrap resume. Set pendingResync=true at the start of resyncProjectionScope so any caller marks catch-up pending; the reconcile loop clears it on caughtUp. * fix(items): fence stale optimistic writes + epoch-guard resync catch-up (round 8) Adds a resync-epoch + fenced-id mechanism to close the last two race classes: - fencedIds: a resync records the ids it dropped (hidden under the new scope). upsert() refuses a fenced id, so a stale old-scope create/update response resolving after the resync can't resurrect a now-hidden row that no new-scope delta would evict (P1). An authoritative applyDelta re-add un-fences; the next resync recomputes the set (re-upgrade clears it). Self-contained in the store — no epoch threading through the optimistic callers. - scopeEpoch: bumped when a resync installs a new snapshot. Both reconcile loops capture it before each /items-changes and skip treating a response that raced a concurrent resync as caught-up, so a stale in-flight delta can't clear pendingResync without validating the pinned cursor (P2). Regression test covers fence → reject stale upsert → authoritative re-add un-fences → later edits accepted. * fix(items): bump scope epoch before resync fetch (round 9 P2) scopeEpoch advanced only after listIndex() returned, so a reconcile response racing the fetch saw the old epoch and could clear the pendingResync the resync set at start. Bump the epoch before the network await instead.	2026-07-13 22:46:55 -04:00
xarmian	c8492db29f	fix(e2e): disable rate limiting on the E2E server to stop 429 flakes (BUG-2089) (#922) ... The E2E harness runs the real pad binary with the real rate limiter, and every Playwright test shares one loopback IP (127.0.0.1). The auth limiter (5 logins/min/IP, burst 5) trips as soon as a spec logs in a couple of browser clients — collab-persistence.spec.ts logs in two per test — so browserLogin fails with "in-page login failed with status 429". This was deterministic, not flaky: it failed on TASK-2058's own PR and its push to main, and on every downstream PR since. Add a test-only env knob PAD_DISABLE_RATE_LIMITS: when truthy, New() leaves Server.rateLimiters nil, which RateLimit() already treats as a pass-through (Stop() and the MCP path are already nil-safe). Wire it into the Playwright webServer.env; run-pad.mjs spawns the binary with inherited env so it reaches the pad process. Limiters stay fully active in prod/self-host — the knob is an explicit opt-in only the E2E server sets. Verified: collab-persistence.spec.ts passes locally with the fix; the existing limiter tests still pass (limiters on when the env is unset); new TestRateLimit_DisabledByEnv pins the bypass. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-11 14:30:32 -04:00
xarmian	9938a81328	fix(oauth): default absent authorize scope so consent completes (BUG-2088) (#921) ... An OAuth authorize request may legally omit `scope` (RFC 6749 §3.1.2; our advertised scopes_supported is only advisory). Claude Code does. When it does, ar.GetRequestedScopes() is empty, renderConsent shows zero capability-tier radios, and clicking Authorize dead-ends the /authorize/decide POST with "capability_tier must be 'read', 'write', or 'admin'". Default an absent scope to the requesting client's registered scopes (DCR seeds pad:read/pad:write), falling back to pad:read pad:write. A missing/unknown client_id is left untouched so fosite emits its normal invalid_client error. The default is set on BOTH r.Form (feeds NewAuthorizeRequest → the consent tier radios) AND r.URL.RawQuery: renderConsent builds the consent form's hidden authorize fields from r.URL.Query(), and /authorize/decide rebuilds the AuthorizeRequest from those hidden fields — so without the URL update the decide POST would reconstruct a scope-less request and reject the chosen tier one step later. Regression test drives the full GET-consent → POST-decide → code flow, extracting the scope from the rendered hidden field so it fails if the field goes missing. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-11 14:05:50 -04:00
xarmian	fce3023bc5	test(server): adopt t.Parallel() in heaviest handler tests (TASK-2059) (#918) ... The internal/server suite ran almost fully serially (only 1 of 113 test files used t.Parallel). The CI -race flake (BUG-1913) is structurally fixed via the copy-once storetest template DB, but the suite regrows toward the -timeout budget as it grows serially. Add t.Parallel() as the first line of every top-level Test* func in the four heaviest files — handlers_items, handlers_oauth, handlers_mcp, handlers_dashboard (159 tests). All build on isolated per-test fixtures (testServer / oauthEnabledTestServer, both backed by storetest.NewSQLite, which copies a fresh template DB into t.TempDir per call), so each test owns its DB, rate limiters, and event bus. Deliberately left serial: t.Run subtests that share the parent's server+workspace and mutate the same rows (e.g. the PatchItem subtests all PATCH one seeded item) — parallelizing those would race. The goroutine-count / timing-sensitive tests in server_test.go are untouched. go test -race ./internal/server/ stays clean (7m16s, exit 0). Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-11 09:38:14 -04:00
xarmian	bfa32dde5a	fix(security): encrypt webhook HMAC secrets at rest, mask in responses (BUG-2057) (#915) ... Webhook signing secrets were stored plaintext in the webhooks.secret column and echoed back in every API response. Encrypt them at rest (reusing the existing AES-256-GCM store helpers, same pattern as TOTP secrets) and return the raw secret ONLY in the creation response; list responses now mask it and expose a has_secret flag instead. - store: encrypt on CreateWebhook, decrypt on Get/ListWebhooks so the dispatcher still signs with the plaintext secret. Reuses the secret column with the "enc:" prefix — no new column/migration. Keyless self-host stays a no-op fallback (encrypt returns plaintext; decrypt passes legacy rows through unchanged). - BackfillEncryptWebhookSecrets encrypts pre-existing plaintext rows on startup once a key is configured (idempotent), mirroring the TOTP backfill. - model: add HasSecret so masked responses still signal presence. - handlers: mask secret on list; document raw-only-on-create. - tests: encrypt-at-rest round-trip + HMAC validity, list decrypt, plaintext backfill/back-compat, and the API mask-except-on-create contract. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-11 00:10:27 -04:00
xarmian	3bb50ab27f	fix(security): make TOTP login codes single-use (BUG-2054) (#914) ... The 2FA verify path accepted a valid TOTP code with no consumed-step tracking, so within a code's ~30s window (plus skew) the same code was replayable, and unlike the recovery-code branch the TOTP branch had no per-challenge attempt cap. Add a nullable users.totp_last_step column and an atomic compare-and-set Store.ConsumeTOTPStep: a code's derived time-step must be strictly greater than the stored watermark, and the winning UPDATE advances it in the same statement so two concurrent requests can't both consume one step. The handler derives the exact step a code matched (pinned within the ±1 skew window, not the current step) and rejects a replay with the same invalid-code response — no replay signal is leaked. Also caps TOTP attempts per challenge token by reusing the existing RecoveryCode limiter. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-10 23:56:59 -04:00
xarmian	8f7b7d551f	fix(security): rate-limit share-link password verification (TASK-2055) (#913) ... Share-link password verification had no dedicated brute-force limiter, so a password-protected /s/{token} link could be ground offline-fast — the resolve handler would bcrypt-compare an unbounded stream of guesses. Add two limiters, both charged BEFORE the bcrypt compare: - SharePasswordIP (5 / 10-per-hour, keyed on SHA-256(share ID)+client IP) caps a single grinder and protects bcrypt CPU; per-IP so one caller can't lock out other viewers, and it's checked first so a single address can't drain the link-wide bucket. - SharePasswordShare (60 / 60-per-hour, keyed on SHA-256(share ID)) caps the aggregate guess rate across a botnet that rotates IPs. Charged pre-compare like login's per-email AuthEmail gate, so an exhausted link blocks even a would-be-correct guess (no password oracle). Its burst is sized so ordinary multi-viewer traffic never trips it, and the per-IP gate ahead of it means exhausting it needs a genuine botnet (self-healing) — the same bounded tradeoff AuthEmail accepts for an unauthenticated shared secret. Both keyed on SHA-256 so no secret hits the limiter map. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-10 23:48:59 -04:00
xarmian	3f69b76b06	feat(security): enforce session UA binding under strict mode (TASK-2056) (#912) ... Session IP/User-Agent binding was log-only by default, so a stolen session token granted durable any-origin access. IP-change enforcement already existed behind PAD_IP_CHANGE_ENFORCE=strict; this extends the same single toggle to also enforce the User-Agent-hash binding. When strict enforce is ON, a request whose client IP OR User-Agent hash no longer matches the session's stored binding now revokes the session (DeleteSessionIfExists) and rejects the request (401 for API, revoked-passthrough for public/browser paths), killing the stolen token. When enforce is OFF (default), behavior is unchanged: UA mismatch is logged (slog only, no new audit row) and the request proceeds, so existing self-host users see no behavior change and routine client churn (browser/WebView updates, DevTools emulation, mobile-app rebuilds) is tolerated. The UA hash is stable within a real session, so UA-mismatch enforce carries fewer false positives than IP enforce (mobile roaming, VPN toggles, carrier NAT) — documented in the handler comment. Adds the ActionSessionUAChanged audit action, emitted only in strict mode. No DB migration: reuses the existing IPChangeEnforce config flag and the existing session store primitives. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF	2026-07-10 23:32:22 -04:00
xarmian	bed933d7fd	feat(items): field-level PATCH + conflict envelope + read-only version history (TASK-2022) (#876) ... * feat(items): field-level PATCH + conflict envelope + version history Adds three related item-update primitives (TASK-2022 / IDEA-1480): - Field-level merge: PATCH `fields_patch` shallow-merges onto the item's current fields INSIDE the write transaction (null deletes a key), so concurrent single-field updates no longer clobber each other via the full-blob read-modify-write. `pad item update` and the MCP `pad_item.update` action now send only the changed keys. - Optimistic concurrency: optional `expected_updated_at` on update; on mismatch the store returns *UpdateConflictError and the handler emits the pad-structured-error/v1 conflict envelope (HTTP 409, code=update_conflict). Surfaced on CLI (`--expected-updated-at`) and MCP (`expected_updated_at`). - Read-only version history: `pad item history <ref>` (alias `versions`) and MCP `pad_item.history`, reusing the existing item_versions store + versions endpoint (no new store, no schema change). MCP ToolSurfaceVersion bumped 0.9 -> 1.0 (new action + param; update behavior change). No migration required. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(items): address Codex review — dispatcher fields_patch, OCC ordering, date/required guards Round 1+2 review fixes for TASK-2022: - HTTP MCP dispatcher (dispatch_http_advanced.go) now sends fields_patch (only changed keys) instead of a client-side merged full fields blob, and forwards expected_updated_at — remote MCP callers get the same race-free merge + optimistic concurrency the CLI/HTTP paths do. - ValidatePartialFields rejects null-deleting a schema-declared REQUIRED field (would otherwise persist a blob the full-update validator rejects). - Open-children guard on the fields_patch path merges the patch onto the IN-TX locked row inside the precheck (not a stale pre-lock preview), so a priority-only patch can't false-fire the guard. - Optimistic-concurrency check now runs BEFORE the open-children precheck in the store, so a stale expected_updated_at yields update_conflict (not open_children) — single in-tx re-read shared by both. - Date auto-population on the patch path only fills an EMPTY current date; an existing end_date the caller isn't touching is preserved. Tests added for each fix. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 16:47:34 -04:00
xarmian	c846cff4fd	feat(project): agent-accessible activity feed (pad project activity + MCP action) (#877) ... * feat(project): agent-accessible activity feed (pad project activity + MCP action) Add a non-streaming, bounded activity query so agents can catch up on what other agents/users changed since they last worked — the query counterpart to the live `pad project watch` SSE stream. - CLI: `pad project activity [--limit N] [--actor user|agent] [--since DATE]` backed by the existing GET /workspaces/{ws}/activity feed. - MCP: `pad_project.activity` action (passThrough) + cloud HTTP route. - Extend the activity endpoint with a server-side `since` date filter (handler parse + store SQL clause) so limit/actor/since behave identically across CLI, stdio MCP, and cloud HTTP transports. - Bump ToolSurfaceVersion 0.11 -> 0.12 (drift guard, README, CLAUDE.md, instructions.md) and add a SKILL.md querying-guidance line. Tests: store since-filter test, HTTP dispatch test, catalog action test. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(mcp): mark pad_project.activity read-only in tool surface Add activity to readOnlyActions so the serialized MCP tool surface emits read_only:true (missing entries default to write). Spot-check it in tool_surface_test.go to guard against regression. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 16:26:41 -04:00
xarmian	c127a5f965	fix(playbooks): enforce draft gate server-side + expose status (BUG-2020) (#874) ... * fix(playbooks): enforce draft gate server-side + expose status (BUG-2020) pad_playbook run / POST /playbooks/{ref}/run now refuse a playbook whose status isn't "active" with a structured playbook_not_active error. Adds an allow_draft escape hatch across all surfaces: JSON body field, CLI --allow-draft flag, MCP boolean param, and an "allow-draft" bareword in raw_args (stripped before strict parsing). status is now echoed on both the run and get responses. Bumps ToolSurfaceVersion 0.9 -> 0.10 and updates the drift-guarded docs (instructions.md, README.md) plus CLAUDE.md. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(playbooks): forward allow_draft on WebMCP + refresh mcp serve help Codex review follow-up for BUG-2020: - WebMCP dispatcher + api client now forward allow_draft so the browser surface can use the draft-gate escape hatch the catalog advertises. - `pad mcp serve --help` refreshed from the stale "v0.4 / eight tools" text to the current v0.10 / nine-tool surface (incl. pad_library). Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(playbooks): type playbooks.get() with top-level status (BUG-2020) Codex P3 follow-up: the get response now returns Item & { status }. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 15:28:21 -04:00
xarmian	ff7e7d51cb	fix(server): guard degraded/degraded_sections in bootstrap dashboard (BUG-2072) (#869) ... BUG-2014 (PR #867) added Degraded + DegradedSections to DashboardResponse so callers can tell a failed sub-query apart from a genuinely-empty section. BUG-2072 reported that the slim BootstrapDashboard projection omits them — but BootstrapDashboard embeds *DashboardResponse anonymously, so encoding/json already promotes both fields into the bootstrap wire shape. Verified empirically: the MCP pad_meta.action=bootstrap tool, the pad://workspace/{ws}/bootstrap resource, and the pad_set_workspace embed all serialize this same struct, so partial-failure state already reaches every agent surface. The promotion was untested and undocumented, so a future refactor to an explicit slim projection (like BootstrapCollection / BootstrapRole) could silently drop it. This pins the behavior: - TestBootstrapDashboardCarriesDegraded asserts on the marshaled JSON (not just promoted field access) that degraded=true + the failed section names flow through, and that a healthy dashboard omits degraded_sections. - BootstrapDashboard godoc now documents the promotion + the carry-across requirement for any future explicit projection. No payload change — the fields were already present. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 13:09:26 -04:00
xarmian	a7994fc374	fix(store): make item field + parent-link update atomic (BUG-2013) (#868) ... * fix(store): make item field + parent-link update atomic (BUG-2013) handleUpdateItem committed the field write, then ran SetParentLink/ ClearParentLink as a SEPARATE store transaction. A failure there (cycle discovered late, DB error) returned 500 with half the patch already applied — a code-acknowledged partial-commit window. Fold the parent-link mutation into the SAME transaction as the field write: - store: extract setParentLinkTx / clearParentLinkTx from the public SetParentLink / ClearParentLink (which keep their own tx). Add UpdateItemWithParentLink(id, input, precheck, *ParentLinkUpdate) — UpdateItemWithPreCheck now delegates to it with a nil link. The link write runs after the field UPDATE but before COMMIT, so a failing link write rolls the field write back too. - lock ordering: the NEW parent's advisory key is folded into the update's initial sorted AcquireParentChildrenLocks batch (extraKeys on acquireParentChildrenLocksForUpdate), so setParentLinkTx's later re-lock is an idempotent no-op and the combined update stays deadlock-free. checkParentCycle is parameterized over the queryer so the cycle walk reads inside the tx. - handler: restructured into validate / atomic-write / post-commit stages. The parent-link directive is built once and threaded through all three UpdateItemWithParentLink call sites; the post-commit SetParentLink/ClearParentLink block is removed. Works on both SQLite and Postgres (advisory locks are pg-only; SQLite gets atomicity from BEGIN IMMEDIATE). Adds store tests proving a failing parent-link write rolls back the field change (no partial state) and that the happy path commits both together. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(store): check parent cycle under lock in setParentLinkTx (Codex #868) Codex review flagged a cycle-check TOCTOU: setParentLinkTx walked the ancestor chain BEFORE acquiring the parent-children advisory locks, so two concurrent reparents could each pass on a stale snapshot, block on the lock, then both insert — forming a cycle (A→B→C→A). Move the cycle check to AFTER lock acquisition; under the lock the tx-scoped walk sees the edge the just-unblocked peer committed and rejects the cycle. Pre-existing behavior (the old SetParentLink checked cycles on s.db before even beginning its tx), hardened here since this function was already being refactored. Residual: cycles closed via an edge on an item neither endpoint locks remain possible — a limitation of the per-endpoint lock scheme, tracked separately. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 12:18:01 -04:00
xarmian	aeb80883f0	fix(webhooks): track delivery goroutines + bounded retry (BUG-2012) (#864) ... * fix(webhooks): track delivery goroutines + bounded retry (BUG-2012) Webhook deliveries ran in untracked `go d.deliver(...)` goroutines that write to the store — the BUG-842 shutdown-race class that goAsync was built to prevent — and had no retry. - Inject a `spawn func(func())` into Dispatcher (SetSpawn). Server wires s.goAsync via SetWebhookDispatcher so deliveries are tracked on s.bg (Stop() waits for in-flight deliveries) and inherit goAsync's panic recovery (BUG-2011). Nil spawn falls back to a plain goroutine, so standalone Dispatcher usage is unchanged. - Add a bounded in-goroutine retry: up to 3 attempts with linear backoff on transient failures (network error / timeout / 5xx). Permanent failures (4xx, SSRF block, malformed URL) stop immediately. The final outcome is recorded once via UpdateWebhookFailure. - Tests: delivery runs on the injected spawn; transient 5xx retries to the cap; permanent 4xx does not; a recovered transient records success. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(webhooks): classify redirect-block + non-5xx as permanent (Codex review) Second Codex review of PR #864 found two retry-classification gaps: - P2: an SSRF-blocked (or looping) redirect surfaces as an error from client.Do (via CheckRedirect), which the retry loop treated as transient — so a redirect to an internal target was retried 3x with backoff. Wrap a sentinel (errRedirectRejected) in checkRedirect and match it with errors.Is (url.Error unwraps to it) to classify these as permanent — attempted once, no retries. - P3: the status switch treated every non-2xx/non-4xx as transient. Narrow transient to 5xx only; 4xx/3xx-no-Location/1xx are permanent, matching the stated "network error / timeout / 5xx" retry policy. Adds TestDispatcher_RedirectBlockIsPermanent. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 12:02:49 -04:00
xarmian	8be1ac67da	fix(server): surface dashboard sub-query failures instead of silent empty sections (BUG-2014) (#867) ... * fix(server): surface dashboard sub-query failures instead of silent empty (BUG-2014) buildDashboardResponse assembled several best-effort sections with `if err == nil` / `if err != nil { continue }` and no logging, so a failing sub-query rendered indistinguishably from a genuinely-empty section — a completely silent degradation. Add a `Degraded` bool + `DegradedSections []string` to DashboardResponse. A new markDegraded helper logs each failure (slog.Error with workspace + section) and records the affected section, so partial failures are both diagnosable server-side and visible to the client without changing the all-or-nothing contract for the queries whose failure genuinely invalidates the whole dashboard (those still return an error). Wired into active_plans, attention.stalled, attention.orphaned_tasks, recent_activity, by_role, and starred_items. The has_agent_activity source fallback (which has a valid default) logs a Warn but does not degrade. Mirror the new fields in the TypeScript DashboardResponse type and add a Go test asserting a failed sub-query flips Degraded, names the section, keeps the endpoint at 200, and preserves the healthy sections. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(server): skip orphan detection when GetParentMap fails (BUG-2014 review) A GetParentMap failure previously fell back to an empty parent map and still iterated allTasks, flagging every visible non-done task as an orphaned_task (false positives). Skip orphan detection entirely on that failure — the section is already marked degraded. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * feat(web): show degraded-load banner on the dashboard (BUG-2014) Consume the new DashboardResponse.degraded / degraded_sections signal on the workspace dashboard page. When a best-effort sub-query fails server-side, the affected sections could otherwise render as genuinely empty; surface an amber "some data couldn't be loaded" banner (listing the affected sections) so the partial-failure state is visible instead of silent. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 12:02:12 -04:00
xarmian	a04fd861dc	fix(server): add panic recovery to background sweeper goroutines (BUG-2071) (#865) ... The four long-running sweeper loops (orphan GC, op-log GC, token reaper, workspace purge) spawn their own s.bg-tracked goroutine with a stop-channel lifecycle, so they can't route through goAsync (a fire-and-forget helper that owns the whole goroutine) without breaking shutdown or double-counting s.bg. As a result they had NO recover(): a panic in any sweeper body crashed the single-binary server for every tenant. Add a shared Server.recoverSweeper(name) firewall — mirroring goAsync's recover + debug.Stack slog style — and defer it inside each sweeper goroutine. A panic is now logged with a stack and the goroutine unwinds cleanly; its own deferred s.bg.Done() still fires (recover stops the unwind), so Stop() still drains. No change to any sweeper's loop cadence or stop-signal shutdown. Adds TestTokenReaper_RecoversPanic, which drives a real reaper tick to panic (nil store → nil-pointer deref in the first cleaner) and asserts the panic is logged+recovered and Stop() returns. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 11:58:41 -04:00
xarmian	f7c4cb3287	fix(server): add panic recovery to goAsync background tasks (#863) ... goAsync wrapped fn in a bare goroutine with no recover(); chi's Recoverer only covers request goroutines, not these detached ones. A panic in a background task (e.g. deriveThumbnails hitting a Go image-decoder panic on a crafted upload, or an email send) would unwind past the goroutine and crash the whole single-binary server for every tenant. Add a single deferred recover() inside the goAsync goroutine that logs the panic + stack via slog, covering all 15+ call sites at once. The recover defer is registered after `defer s.bg.Done()`, so it runs first on unwind and Done() still fires — Stop() continues to drain the WaitGroup even when fn panics. Adds TestServer_goAsync_RecoversPanic asserting the process survives a panicking fn and Stop() returns. Fixes BUG-2011. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS	2026-07-08 11:49:01 -04:00
xarmian	15ad930d78	feat(bootstrap): add convention_index for triggered-convention discovery (TASK-2004) (#848)	2026-07-07 17:28:33 -04:00
xarmian	48104a5eff	perf(server): collapse dashboard/bootstrap N+1 into set-based queries (BUG-2002) (#847) ... The dashboard builder (also reused by the bootstrap endpoint and every pad_set_workspace) ran ~1000 queries on a large workspace: one GetItemLinks + per-link GetItem for every non-done item (blocked attention + suggested_next filter), a GetChildItems per active plan (progress + suggested_next), a GetItemIncludeDeleted per recent-activity row, a GetCollection per visible collection, and a per-collection COUNT via ListCollections whose result the dashboard never uses. Replace every per-item/per-row loop with a set-based query: - GetBlocksEdges: one workspace-wide JOIN of blocks-links -> blocker essentials, ordered created_at DESC to preserve the old first-active- blocker selection. Drives both blocked attention and the suggested_next blocked-filter (retires itemBlockedByActive). - GetChildItemsForParents: one IN query grouping all active-plan children by parent (progress + suggested_next; no-content projection). - GetItemsByIDsIncludeDeleted: one IN query batch-hydrating recent-activity items (include-deleted). - ListItemsParams.NoContent: skip loading full markdown bodies on the count/summary scans (allItems, plans, stalled, orphaned). - ListCollectionsMinimal now also selects slug; the dashboard uses it in place of ListCollections, dropping the unused per-collection COUNT N+1 and the GetCollection-per-visible-id loop. Per-item N+1s are gone; query count is now constant in workspace size. Verified byte-identical dashboard + bootstrap JSON against three live workspaces (docapp/claude/apm); dashboard latency ~376ms -> ~198ms on the 1907-item docapp workspace. New store methods are unit-tested.	2026-07-07 17:18:54 -04:00
xarmian	375e3b5369	perf(store): batch parent-lineage enrichment into one scoped query (BUG-2003) (#846) ... enrichItemsWithParent loaded every parent link in the workspace and then called full-row GetItem once per unique parent (151 in the live workspace), despite a comment claiming a bulk fetch. A ?limit=1 list took 34-39ms vs ~2ms for a single GET — a ~20x tax to return one row, hit on every list request including the /items-changes sync endpoint the local-first client polls. Scope the parent IDs to only the parents of the returned item slice, then hydrate title/ref/slug/collection in one skinny WHERE id IN (...) query via the new Store.GetItemLineageByIDs. Enrichment output shape and best-effort (missing parent never fails the list) behavior are preserved; the visibility filter now runs against the batched projection's collection_id.	2026-07-07 16:51:48 -04:00
xarmian	0aa431f132	fix(server,cli,mcp): default item list to per-collection non-terminal filter (BUG-2001) (#845) ... The CLI's default `pad item list` (no --status/--all) sent a hardcoded ~20-status allowlist as the status filter. Collections with custom status vocabularies (blog: drafting/scheduled; human-tasks: todo) fell outside the list and had their open items hidden. MCP inherited the same bug via the CLI default and the HTTP route table's mirrored allowlist. Replace it with a server-side `non_terminal` filter: ItemListParams.NonTerminal resolves each collection's terminal set from its schema's terminal_options (falling back to DefaultTerminalStatuses) and keeps only items NOT in that set — reusing the existing doneFiltersForWorkspace + buildChildrenDoneExpr machinery, applied in both the normal and FTS query paths. The CLI default and both MCP dispatch paths (ExecDispatcher via the CLI, HTTPHandlerDispatcher via mapItemList) now send non_terminal=true. --status X and --all semantics are unchanged.	2026-07-07 16:45:00 -04:00
xarmian	cf5eb8dd3a	feat(cli,mcp): summary-shaped item list with --full opt-in + limit clamp (TASK-2000) (#842) ... `pad item list --format json` returned the full models.Item shape — including each item's rich markdown `content` body (~52% of the bytes) plus UUID plumbing and duplicate join fields — with no default limit, so a bare agent list dumped ~1.4MB (all collections) or 5.3MB (--all) into context. The single biggest agent-token lever. CLI: - JSON output now defaults to a token-light ItemSummary projection: `content` → short `content_preview`, UUIDs (id/workspace_id/collection_id/*_user_id/ parent_id/agent_role_id) and duplicate collection/parent join fields dropped, `fields`/`tags` emitted as nested JSON. ~71% smaller on a real workspace. - `--full` opt-in flag restores the complete models.Item shape. - Default limit (200) + hard-max clamp (1000) so --all/huge lists can't dump unboundedly; a stderr note fires when a table result is capped. MCP: - pad_item.list is now a custom action that injects a default limit (50) and clamps an oversized one (max 300), mirroring the backlinks default/max, so a bare agent list stays bounded on both dispatchers. - ToolSurfaceVersion 0.8 → 0.9 (list result shape + limit behavior change). Server: - Hard-max backstop clamp (1000) on an explicit `?limit=` at the item-list request boundary; no default (internal ListItems callers that fetch every row are untouched). rawJSONOrNil guards against a malformed stored Fields/Tags value breaking the whole list marshal (falls back to a JSON string).	2026-07-07 16:31:59 -04:00
xarmian	f5b437a65f	feat(server): workspace restore + deleted-list endpoints (TASK-1970) (#827) ... Foundation for PLAN-1969 (user-recoverable workspace soft-delete). A workspace delete only stamps workspaces.deleted_at; items/collections/ members are untouched, hidden transitively. Restore clears deleted_at so everything re-surfaces intact. Store (internal/store/workspaces.go): - RestoreWorkspace(slug): UPDATE ... SET deleted_at = NULL WHERE slug=? AND deleted_at IS NOT NULL. Returns sql.ErrNoRows (-> 404) when no soft-deleted row matched (already live or purged). - ListDeletedWorkspaces(userID, cutoff): owner-scoped, deleted_at within the window, ordered deleted_at DESC. Account-deleted workspaces have no live owner, so they never leak. - GetDeletedWorkspaceBySlug(slug): resolves a soft-deleted row (the normal resolvers filter deleted_at IS NULL) so the handler can tell 403 from 404. - Dual-dialect via s.q/s.dialect; no migration (deleted_at already exists). Handlers (internal/server/handlers_workspaces.go): - POST /api/v1/workspaces/{slug}/restore: owner-only; 404 not-restorable, 403 non-owner, 200 + restored workspace; logs a "restored" activity. - GET /api/v1/workspaces/deleted: owner-scoped list with per-entry purge_at + days_left, both derived from workspacePurgeRetention so restore and the purge sweeper share ONE 30-day window (no drift). - Both routed outside the /{slug} RequireWorkspaceAccess subrouter (which resolves only live workspaces); restore enforces owner authz inline. CLI client (internal/cli/client.go): RestoreWorkspace + ListDeletedWorkspaces. TS type (web/src/lib/types/index.ts): Workspace.deleted_at + DeletedWorkspace. Tests: store (resurface-intact; double-restore/live -> ErrNoRows; window boundary 29d IN / 31d OUT + owner-scoping) and handler (owner-only 403, 404 live/unknown, 200 restore, owner-scoped deleted-list). Green on SQLite and Postgres (make test-pg); golangci-lint clean. Closes TASK-1970 Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST	2026-07-06 10:07:34 -04:00
xarmian	b73ba63752	feat(store): hard-purge soft-deleted workspaces after 30 days (TASK-1966) (#825) ... The /privacy policy promises owned workspaces are removed from live systems within 30 days, but DeleteAccountAtomic and DeleteWorkspace only SOFT-delete (workspaces.deleted_at) and nothing ever expunged them — a right-to-erasure gap. Add a scheduled sweeper that hard-purges workspaces soft-deleted longer than a named 30-day retention constant. - Store: ListPurgeableWorkspaces (soft-deleted + past cutoff; never touches live rows), WorkspaceAttachmentBlobs, CountAttachmentsForHash- OutsideWorkspace (content-addressed dedupe guard), and PurgeWorkspace- Data — a transactional cascade that deletes every workspace-scoped child row in FK-dependency order (items/comments/versions/links/ reactions/stars/yjs op-log/wiki-links/grants/transitions/moves/views/ collections/documents+versions/agent_roles/webhooks/invitations/ templates/share_links+views/oauth join rows/report layouts/members/ member access/api tokens/attachments/activities), de-identifies mcp_audit_log, and refuses to touch a non-soft-deleted workspace. - Server: a periodic sweeper modeled on the orphan GC — captures blob keys before the purge, cascades the DB rows, then reclaims blobs through the attachment store abstraction (FS + S3 safe) with the orphan GC's cross-workspace dedupe + in-flight-upload guards. Failure isolated per workspace; idempotent. - Dual-dialect (SQLite + Postgres); partial index on workspaces(deleted_at) — migrations/073 + pgmigrations/051. Both delete paths (account + manual workspace delete) purge on the same 30-day clock: identical deleted_at mechanism, both owner-initiated, and the orphan GC already reclaims their attachment blobs at 30 days. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST	2026-07-05 22:23:12 -04:00
xarmian	86a952768e	test: cover account delete + export contracts and Danger Zone e2e (TASK-1963) (#824) ... Backend contract tests (internal/server): - delete-account success-body: pin the exact {ok:true} envelope the UI consumes (the cascade/skip tests asserted only status 200). - export happy-path: decode the artifact and assert the exact `attachment; filename="pad-export.json"` header, application/json type, and the top-level {user, workspaces} shape with inline collections/items (fills the TASK-508 gap; complements the BUG-1945 gate smoke test). - TOTP paths (enabled+valid/missing/invalid, non-TOTP) were already added in TASK-1958 — confirmed, not duplicated. Web e2e (web/e2e, Playwright): settings Danger Zone — - export download (filename + success line), - delete password branch (real register→login→delete; admin user search confirms the row is gone), - delete cloud OAuth-only typed-confirm branch (session/me flags patched; delete transport stubbed since a self-host server requires a password), - post-delete redirect to /login. Delete specs run desktop-only (viewport-agnostic; avoids doubling IP-rate-limited /auth/login + /auth/register hits that flaked the suite under parallel load). No product code changed. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST	2026-07-05 20:47:30 -04:00