Skip to content

fix(items): surface archived items instead of masking them as missing (BUG-1791) - #733

Merged
xarmian merged 1 commit into
mainfrom
fix/bug-1791-archived-marker
Jun 15, 2026
Merged

fix(items): surface archived items instead of masking them as missing (BUG-1791)#733
xarmian merged 1 commit into
mainfrom
fix/bug-1791-archived-marker

Conversation

@xarmian

@xarmian xarmian commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes BUG-1791. An external user reported (via the hosted MCP) that an item appeared in a collection list (all=true) yet was unresolvable by ref, absent from search, and 404'd on get/update — diagnosed in the report as an "index/FTS desync, needs re-index."

It is not a desync. The item was simply soft-deleted (archived). all=true (IncludeArchived) is the only read path that includes archived rows; every other path (GetItemByRef, FTS, status-filtered list) correctly filters deleted_at IS NULL. The real defect is observability: archived rows came back from all=true with no marker, and get/update returned a bare "Item not found", so an archived item looked like corrupted / lost data.

Root cause

  • scanItems (the shared list scanner) didn't select deleted_at, so archived rows returned by all=true were byte-for-byte indistinguishable from live rows.
  • GET / UPDATE / DELETE / MOVE resolved active-only and returned a generic 404 for an archived ref.

Changes

  • Store: scanItems now scans i.deleted_at; all six feeding SELECTs select it (ListItems, listItemsFTS x2 dialects, getChildItems, ItemsModifiedSince, ListStarredItems). The deleted_at IS NULL-filtered paths are unaffected (value stays NULL there).
  • API: GET resolves include-deleted, so an archived item is returned read-only (200) with its deleted_at marker instead of 404. UPDATE / DELETE / MOVE of an archived ref return a clear 409 archived ("restore first"), visibility-gated identically to the active path (no existence leak).
  • CLI: (archived) marker in item list; an Archived: line in item show.

Tests

  • Store: IncludeArchived returns the archived row with DeletedAt populated; the default (active-only) list excludes it.
  • Server: GET archived -> 200 + deleted_at; UPDATE / MOVE archived -> 409 archived.
  • Gates: make check and make test-pg (dual-dialect Postgres) green. Codex review loop converged to CLEAN (round 1 caught the move path; fixed).

Notes / follow-up

  • The reporter's "it appeared in a status=todo list earlier in the same session" can't be reproduced locally and contradicts the code (an archived row can't match a status-filtered list); likely a misread or a pad-cloud cache/replica artifact — confirming would need the tolkien-workspace row.
  • No agent-facing restore yet (pad item restore / pad_item action=restore) — restore is web-UI / REST only. Filing a follow-up so the "restore first" guidance becomes self-serve for agents.

Closes BUG-1791.

… (BUG-1791)

A soft-deleted (archived) item still appears in include-archived list
results (all=true) but 404'd on get/update/move and was absent from search
and status-filtered lists — all=true is the only read path that includes
archived rows. With no archived marker in list output and a bare "Item not
found" on get/update, this looked like index/FTS corruption (the report's
diagnosis). It is not: every read path was behaving correctly for an
archived item. The root cause is observability, not a desync.

- scanItems now scans i.deleted_at; all six feeding SELECTs select it
  (ListItems, listItemsFTS x2 dialects, getChildItems, ItemsModifiedSince,
  ListStarredItems). Archived rows in include-archived results now carry
  deleted_at so callers can tell them apart from live rows; the
  deleted_at-filtered paths are unaffected (value stays NULL there).
- GET item resolves include-deleted, returning an archived item read-only
  (200) with its deleted_at marker rather than 404 — an agent can read it
  and see it is archived.
- UPDATE/DELETE/MOVE of an archived ref return a clear 409 "archived"
  (restore first) instead of a bare 404; visibility is enforced exactly as
  the active path so an archived item is never revealed to a caller who
  can't see it.
- CLI shows an (archived) marker in lists and an Archived line in detail.

Tests: store IncludeArchived populates DeletedAt; server GET archived -> 200
with deleted_at, UPDATE/MOVE archived -> 409 "archived". Verified on SQLite
and Postgres (make test-pg).
@xarmian
xarmian merged commit 99b4649 into main Jun 15, 2026
7 of 8 checks passed
@xarmian
xarmian deleted the fix/bug-1791-archived-marker branch June 15, 2026 18:45
xarmian added a commit that referenced this pull request Jun 15, 2026
Adds the agent-facing restore surface so an archived item discovered via
`pad item list --all` can be recovered without dropping to the web UI. The
server already had restore end-to-end (Store.RestoreItem + handleRestoreItem
at POST /items/{ref}/restore, used by the web UI and bulk ops); this wires
the two missing surfaces:

- CLI: `pad item restore <ref>` (cli.Client.RestoreItem → the existing
  endpoint, which resolves the ref include-deleted server-side). Mirrors
  `pad item delete`'s structured JSON envelope: {ref, title, restored: true}.
- MCP: pad_item action=restore via passThrough(["item","restore"]). Restore
  is non-destructive, so it's safe to expose. The action auto-joins the
  schema's action enum (derived from the Actions map) and is documented in
  the tool description.

Conflict case (slug/invocation_slug reclaimed while archived) is already
handled by handleRestoreItem (409) and surfaced by the client's
handleResponse.

Tests: restore endpoint already covered (handlers_items_test.go); restore
added to the MCP catalog<->cmdhelp bijection + dispatch tests. Child of
BUG-1791 (TASK-1827 shipped in #733).
xarmian added a commit that referenced this pull request Jun 15, 2026
…#735)

With GET now returning soft-deleted items read-only (deleted_at populated,
shipped in #733), the detail route can show an archived item instead of a
hard 404. Adds the recovery UI:

- isArchived derived from item.deleted_at; folded into the existing canEdit
  derived so every edit affordance disables while archived.
- A read-only "Archived" banner at the top of the item view (date via the
  file's relativeTime helper) with a Restore button -> api.items.restore,
  then re-fetches the item so the banner clears and editing re-enables.
  Success/error via the existing toastStore; a 409 reclaimed-slug conflict
  surfaces verbatim.
- restoring in-flight flag; handleRestore is a standalone async function
  (not an effect) per CONVE-1688 / CONVE-606.

No API/client/server change — GET is already ungated (#733) and
api.items.restore already existed. Child of BUG-1791 (TASK-1827 in #733,
TASK-1828 in #734).
xarmian added a commit that referenced this pull request Jun 15, 2026
…736)

Follow-up to TASK-1829 (Codex review of #735). When the open item was
archived live — via the SSE item_archived handler or the sync-resume
deleted path — the detail route redirected back to the collection, so the
new in-place Archived banner only appeared on a fresh direct load. Now both
handlers re-fetch the item (GET returns soft-deleted items with deleted_at,
#733) and render the banner in place.

- SSE item_archived: re-fetch and show the banner instead of goto().
- Sync-resume deleted: re-fetch — an archived item (still resolvable, 200)
  shows the banner; a hard-deleted one (404) still redirects.
- Both keep the prior redirect when mid-edit (saveStatus==='saving' ||
  editingTitle): an in-flight save against an archived row would fail and a
  re-fetch would clobber the editor (the original Codex-round-2 reasoning).
- Race guards mirror the handlers' existing pattern (capture item id before
  await, bail if navigated away). The actor's own archive still navigates
  via handleDelete's goto; this only changes the someone-else-archived-it case.

svelte-check + web build green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant