feat(mt#4766): Route a memory curation write path through the shared command registry

[minsky-ai]

Summary

The cockpit could not change a memory in any way — the widget transport was app.get only, and no memory widget called a mutating method. This PR adds PATCH/POST/DELETE routes for memory curation (retag, edit name/description, supersede, hard delete, bulk retag, bulk delete) plus the row-level and bulk-selection UI to drive them.

The architectural decision (mt#4766 Planning Audit, ADR-004)

The routes call the shared command registry (memory.update / memory.supersede / memory.delete), never MemoryService directly. Two reasons, both verified rather than assumed:

1. Memory's guardrails live in the command layer, not the service. grep -n over src/adapters/shared/commands/memory/index.ts finds checkDerivation (:982), validateAssociations on both create (:997) and update (:1100 — retag is an update), and extractTrackingTaskRefs (:1018). The same grep over memory-service.ts returns zero hits. A route calling the service directly would silently skip all four.
2. ADR-004 (Two-Phase Command Execution, ACCEPTED) — the shared command registry is the framework-enforced place these guarantees are meant to live, specifically because "remember to validate first" is a bug class convention cannot close. asks.ts's domain-function-direct pattern is not a counter-example: respondAndCloseAsk IS the domain function carrying the ask's preconditions; memory has no equivalent, so its preconditions only run when the command layer's execute is what gets invoked.

src/cockpit/routes/memories.ts's docblock explains why commands are constructed per-request (a fresh throwaway SharedCommandRegistry injected with the cockpit's cached MemoryServiceSurface) rather than registered once at module load: registering once with a pre-bound service would pin it past a persistence-pool recycle (mt#3638), and registering once without one would require threading a DI container the cockpit doesn't otherwise have.

AT2 — the discriminating test

Attempt to write an association with a key outside ADR-012's closed vocabulary and confirm it is REJECTED.

This is the test that tells the two architectures apart — a route calling MemoryService.update directly has no vocabulary check to fail against. Result: PASS, both in the unit suite and live (see Live verification below).

PATCH /api/memories/:id  { associations: { notARealAssociationType: ["mt#1"] } }
→ 400 { error: "\"notARealAssociationType\" is not an ADR-012 association type. ..." }

And the write never happens — the persisted record's associations is confirmed unchanged.

Actor attribution (mt#2898 applied to memory)

mt#2898 documents that POST /api/asks/:id/resolve reads responder from the request body, making responder: "operator" forgeable by any caller, and that this became load-bearing once a permission bridge started trusting it. These routes never read sourceAgentId/sourceSessionId from the client at all — they aren't in the supersede route's field whitelist, so a request that includes either is rejected outright (400) rather than silently accepted-and-ignored. A supersede's replacement record is always ascribed COCKPIT_OPERATOR_SOURCE_AGENT_ID server-side. Verified both in the unit suite and live: a normal request gets the ascribed constant ("cockpit-operator"), and a request carrying sourceAgentId: "attacker-controlled" is rejected 400 with no record ever created.

Key changes

• src/cockpit/routes/memories.ts (new) — mountMemoryRoutes(app, options), following asks.ts's structural pattern (own file, explicit status-code contract, whitelisted body fields — not the widget dispatcher's uniform 200 + {state:"degraded"}).
• src/cockpit/server.ts — mounts the new routes alongside the other write-route mounts.
• src/cockpit/web/lib/memory-mutations.ts (new) — fetch wrappers + TanStack Query useMutation hooks that invalidate the memories-list/-search/-stats/-detail widget queries on success.
• src/cockpit/web/widgets/MemoryDetail.tsx — a curation action bar (Edit tags / Edit name-description / Supersede / Delete), each a controlled Dialog. The delete dialog states plainly that this is a hard delete and that memory has no short-id tombstone table (unlike tasks' deleted_task_ids), so a deleted mem#N can be reissued — the operator sees this before confirming.
• src/cockpit/web/widgets/MemoriesList.tsx — per-row + select-all-on-page checkboxes and a bulk actions bar (retag / delete), each opening a dry-run-first dialog: preview exactly which records change, then confirm to execute.
• src/cockpit/routes/memories.test.ts (new) — 24 tests.
• docs/cockpit-ui.md — a "Memory curation" section documenting all five routes (methods, request bodies, status codes), the actor-attribution rule, and the hard-delete/no-tombstone consequence.

Bulk operations — dry-run-first + a record cap

Per operational-safety-dry-run-first, both bulk routes preview the exact record set before any write (execute: false by default) and require an explicit execute: true to act. Judgment call, stated here: rather than requiring a separate task wrapper for >10-record bulk mutations (the discipline's normal trigger), the routes simply refuse a selection over BULK_RECORD_CAP (10) with a 400 naming that discipline — a UI-driven, per-click bulk action doesn't have a natural place to attach a task reference, so capping the direct-execution size was the simpler and equally-safe choice for this surface.

Testing

Execution evidence:

bun test --preload ./tests/setup.ts --timeout=15000 src/cockpit/routes/memories.test.ts
24 pass, 0 fail, 55 expect() calls

Covers AT2 (association-vocabulary rejection), AT5 (auth-gating — see below), AT6 (bulk-retag preview matches selection exactly; execute changes exactly that selection; over-cap selection refused), AT7 (malformed/wrong-typed/unknown body fields → 400, never 500), and the two actor-attribution tests above. Also: 404 on a missing record, 200 + persisted-state assertions on update/supersede/delete, invalid type/scope rejection on supersede.

validate_typecheck and validate_lint both pass clean across every workspace, including src/cockpit/web.

AT5 — real unauthenticated requests against both deployment-mode gates

R1 finding: the original version of this test asserted isPublicPath() returns false for each route path — a check against a config table, not a behavior, that would keep passing if the gate were removed, misconfigured, or mounted in the wrong order. Replaced with genuine unauthenticated HTTP requests against the real createCockpitServer app (the same factory server.ts uses in production) for both deployment-mode gates:

• Public Railway deployment (requirePasskeySession, path-based, the same gate /api/shares relies on) — no session cookie → 401 on all five routes.
• Local daemon (mutationAuthMiddleware, method-based — every non-GET needs a bearer token or the loopback cookie) — no token → 401 on all five routes.

Also verified LIVE against a real running local daemon (see below): an unauthenticated DELETE returns 401 with {"error":"Missing or invalid cockpit auth token"}; the identical request with a valid bearer token returns 200.

Live verification

Live-exercised every route against a real, DB-connected cockpit daemon, plus screenshots of the resulting UI at 1440×1000. This required correcting course from an earlier attempt (see "How this got unblocked" below).

Setup: pgvector/pgvector:pg16 in a disposable Docker container, migrated via bun src/cli.ts persistence migrate --execute with MINSKY_POSTGRES_URL pointed at it (confirmed by read-back BEFORE writing anything — see below), then bun src/cli.ts cockpit start --port 18080 --dev against that same database.

AT2, live (association vocabulary):

$ curl -X PATCH http://127.0.0.1:18080/api/memories/<id> \
    -H "Authorization: Bearer <token>" \
    -d '{"associations":{"notARealAssociationType":["mt#1"]}}'
→ HTTP 400
{"error":"\"notARealAssociationType\" is not an ADR-012 association type. Valid types:
tracksTask, relatedTask, originatesRule, originatesSkill, informsAsk, extractedFromSession,
extractedFromTranscript, citedInReview. ..."}

Retag (valid), live: PATCH .../:id {"tags":["seed-mt4766","retagged-live"]} → 200, response record.tags updated, confirmed in Postgres.

Actor attribution, live:

POST .../:id/supersede  { ..., "sourceAgentId": "attacker-controlled" }
→ HTTP 400 {"error":"Unknown field(s): sourceAgentId"}

POST .../:id/supersede  { ...valid body, no sourceAgentId }
→ HTTP 200, replacement.sourceAgentId = "cockpit-operator", replacement.sourceSessionId = null
old.supersededBy = replacement.id  (confirmed in Postgres)

Delete, live: authenticated DELETE .../:id → 200 {"deleted":true,"id":"..."}; row count for that id in Postgres afterward: 0.

Bulk retag/delete, live (AT6): preview (execute:false) over 3 seeded records returned exactly those 3 with currentTags/newTags, no write (confirmed via a follow-up SELECT — tags unchanged); execute:true then changed exactly those 3 rows (confirmed via SELECT — all 3 updated, an untouched 4th record unaffected); bulk delete likewise removed exactly the 3 targeted rows.

Screenshots (1440×1000, headless Chrome via chrome-devtools-mcp against the live daemon above):

• /memories — list with the new select-all/per-row checkboxes, one seeded record visible, stats/facets rendering normally.
• /memory/:id — the curation action bar (Edit tags / Edit name / description / Supersede / Delete) rendered above the metadata panel, no layout defects.
• The delete confirmation dialog — states plainly: "This permanently deletes the row and its embedding. There is no undo, and unlike tasks, memory has no short-id tombstone table — this record's short id can be reissued to a different, unrelated memory in the future."
• /memories with one row selected — the bulk actions bar ("1 selected", Retag, Delete, "Clear selection") rendering correctly above the table.

No defects found in any of the four renders. Cancelled the delete dialog rather than confirming it (no need to destroy the demo record to prove the dialog renders correctly). All scratch infrastructure (container, cockpit process, headless Chrome, temp files) torn down after verification — nothing was left running, and none of this touched the project's real database at any point.

How this got unblocked (for the record)

My first attempt set DATABASE_URL, which this system's config resolution does not read at all — the CLI silently fell through to the project's real Supabase-hosted database, and I stopped before running anything mutating (documented in this PR's prior revision). The coordinator pointed me at MINSKY_VERIFY_DATABASE_URL, registered in packages/domain/src/configuration/sources/environment.ts as a test-fixture. Checking that registration directly: it is a narrowly-scoped fixture read only by scripts/verify-driven-session-conversations.ts (mt#4323) — confirmed empirically too (persistence migrate --dry-run with only that variable set still reported the Supabase pooler host). The variable actually wired into general persistence-config resolution (persistence.postgres.connectionString, the path persistence migrate and the cockpit daemon both read) is MINSKY_POSTGRES_URL (or MINSKY_PERSISTENCE_POSTGRES_URL), documented in persistence-config.ts as "the canonical escape hatch." Using that, the dry-run's read-back showed postgres://***:***@127.0.0.1:<port>/minsky_scratch with schema=missing (a fresh, empty database) — confirmed safe before running --execute.

Out of scope (per spec)

• Creating a memory from the cockpit (mt#4743's permanent-association derivation defect).
• memory_patch section editing (mt#4612's open replace-mode defect).
• Dedup execution (mt#1619).
• The stats widget (mt#4767) — this PR only adds it to the invalidated-query list for consistency.

Duplicate check

No other open PR touches src/cockpit/routes/memories.ts, src/cockpit/web/widgets/MemoriesList.tsx, or src/cockpit/web/widgets/MemoryDetail.tsx (per the mt#4766 Planning Audit's parallel-work sweep, PR #3412 touches unrelated files via a mechanical catch-block rewrite).

Deploy verification

Seven deploy-surface files: the new routes/memories.ts and its test, server.ts (route
mounting), scope-census.ts (the R2 allowlist entry), the mutation hooks, and the two widgets.
No migration and no domain change — the risk shape is a boot failure from the new route mounting,
not a schema change.

After merge I will run mcp__minsky__deployment_wait-for-latest --service cockpit with notBefore
set to the merge timestamp and expectCommitSha set to the merge SHA, and require SUCCESS. The
cockpit deploys from a pushed image, so buildIdentity will read indeterminate — expected for
this service and not treated as a pass on its own.

Because an indeterminate build identity says nothing about WHICH build is serving, the
change-produced assertion is the route itself: OPTIONS/unauthenticated PATCH against
/api/memories/:id on the deployed cockpit must return a real status from the new handler
(401 under the auth gate) rather than the SPA fallback a pre-merge bundle would serve. That is
immune to which deployment record the wait returns, and it exercises the mounting — the specific
thing that could break on boot.

A tool or auth flake on the deployment probe is a blocker to reconnect and retry, not a licence to
downgrade the completion claim.

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 395K prompt, 3K completion | Duration: 50s
Mode: normal

Commands

• /review — request a fresh review

[github-actions]

Cockpit Preview

Preview URL: https://cockpit-preview-production.up.railway.app

Deployed from commit bcc38aad0.
Preview runs in read-only mode — mutations are disabled.

Updated by Cockpit Preview.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

High-quality, well-structured addition: new memory curation routes are correctly routed through the shared command registry, body-field whitelisting is enforced, bulk operations implement dry-run-first with a record cap, and the UI wires row-level and bulk actions with proper cache invalidation. Tests exercise command-layer guardrails (ADR-012 vocabulary) and bulk preview/execute paths. Two issues surfaced: (1) BLOCKING — documentation gap: operator docs do not cover the new memory curation features, confirmation semantics (hard delete, no short-id tombstones), or the new routes; please add/update docs (e.g., cockpit-ui.md) before merge. (2) NON-BLOCKING — AT5 coverage only asserts non-public paths via isPublicPath rather than asserting actual 401 responses; consider adding integration tests against createCockpitServer to pin 401 unauthenticated behavior. Otherwise, the spec criteria are met. If docs are updated, this looks ready to ship.

Findings

• [BLOCKING] docs/cockpit-ui.md:1 — Documentation not updated for new memory curation features and routes
  This PR introduces user-facing memory curation capabilities (row-level edit tags/name/description, supersede, delete) and bulk actions with new HTTP routes (PATCH/POST/DELETE under /api/memories/*) plus visible UI controls in MemoriesList.tsx and MemoryDetail.tsx. I could not find any corresponding updates in the operator docs. docs/cockpit-ui.md does not mention memory curation actions or the new flows, and no new doc files were added. Per the review policy, adding/altering user-facing behavior without updating docs is blocking. Please add documentation covering:

• The curation action bar on the memory detail page (what each action does and constraints: hard delete semantics and no short-id tombstone table)

• Bulk selection flow on the memories list (dry-run preview, record cap, and error behavior)

• The new mutation routes’ operator-facing behavior at least at a high level (consistent with other documented operator endpoints)

Alternatively, point to updated docs if they exist elsewhere in the repo.

• [NON-BLOCKING] src/cockpit/routes/memories.test.ts:416 — Auth acceptance test weakened to path-allowlist assertion; no 401 unauthenticated test
  Acceptance Test 5 in the spec requires: “Unauthenticated POST to each route returns 401.” The new tests only assert that isPublicPath(path) === false for the four paths (lines 416-449), which checks the allowlist but does not prove a 401 response is actually produced in either deployment mode. Consider adding integration tests that spin up createCockpitServer with and without auth (e.g., omit the bearer/cookie and assert 401; include it and assert 200) for each of the four routes. This would harden the contract and catch regressions in mutationAuthMiddleware or passkey gating rather than inferring it indirectly. Marking as non-blocking because the global auth middleware likely covers these routes by construction; this is a coverage gap rather than a functional bug.

Spec verification

Criterion	Status	Evidence
src/cockpit/routes/memories.ts mounting write routes on the asks.ts structural pattern (own file, mountMemoryRoutes(app, options), explicit status-code contract — not the widget dispatcher’s uniform 200 + {state:"degraded"}).	Met	Implemented in src/cockpit/routes/memories.ts (new). Mounted from src/cockpit/server.ts:633 via import { mountMemoryRoutes } from "./routes/memories". Route handlers return explicit 200/400/404/503 JSON per handler and classify errors in classifyMemoryCommandError().
Routes go through the shared command registry so the four guardrails above apply. If any route cannot, name it and say why.	Met	src/cockpit/routes/memories.ts:142-168 builds a per-request SharedCommandRegistry and calls command.execute for memory.update/supersede/delete. Tests assert ADR-012 vocabulary rejection (AT2) — src/cockpit/routes/memories.test.ts:84-118 returns 400 with ADR-012 message, proving command-layer path.
Trust-boundary discipline per asks.ts: whitelist the body fields read; never accept server-computed values from the client.	Met	parseUpdateBody() enforces UPDATE_ALLOWED_FIELDS (name, description, tags, associations) and types; parseSupersedeBody() enforces SUPERSEDE_ALLOWED/REQUIRED without sourceAgentId/sessionId. Unknown fields 400 via rejectUnknownFields(). See src/cockpit/routes/memories.ts:187-288 and 301-391.
Row-level actions on /memories and on /memory/:id: edit tags, edit name/description, supersede, delete.	Met	UI controls implemented: MemoryDetail.tsx adds MemoryCurationBar with EditTagsDialog, EditMetaDialog, SupersedeDialog, DeleteDialog (src/cockpit/web/widgets/MemoryDetail.tsx:829-1216). List page adds bulk selection but row-level selection/controls exist via detail page.
Delete is confirmed, and the dialog states that deletion is hard and that memory has no short-id tombstone table — so a deleted record’s mem#N can be reissued.	Met	DeleteDialog explanatory text at src/cockpit/web/widgets/MemoryDetail.tsx:1128-1150 explicitly mentions hard delete and no short-id tombstone; button is destructive and requires confirm dialog.
Supersede opens the create-replacement form the domain requires (supersede takes a full MemoryCreateInput plus a reason), pre-filled from the current record.	Met	SupersedeDialog in MemoryDetail.tsx pre-fills type/name/description/content/scope/tags; includes reason; on save calls useSupersedeMemory (src/cockpit/web/widgets/MemoryDetail.tsx:1006-1116).
Bulk selection on the list with bulk retag and bulk delete. Bulk delete follows operational-safety-dry-run-first: a preview of exactly which records change before any write, and the >10-record threshold requires a task wrapper.	Met	MemoriesList.tsx adds per-row and header checkboxes, BulkActionsBar with BulkRetagDialog and BulkDeleteDialog rendering preview then execute (src/cockpit/web/widgets/MemoriesList.tsx:524-763, 820-1024, 1168-1200). Server enforces BULK_RECORD_CAP=10 and preview first behavior (src/cockpit/routes/memories.ts:412-489, 491-553). Tests cover preview/execute and over-cap for retag (src/cockpit/routes/memories.test.ts:285-357).
Every mutation invalidates the relevant TanStack Query keys so the list reflects the change without a manual reload.	Met	src/cockpit/web/lib/memory-mutations.ts defines invalidateMemoryQueries() to invalidate ["widget","memories-list"], ["widget","memories-search"], ["widget","memories-stats"], and ["widget","memories-detail", id] and wires it in onSuccess of all mutations (lines ~120-197).
Auth: writes sit behind the same session gate as the other cockpit write routes (/api/shares returns 401 unauthenticated — match it).	Met	Routes are mounted into the central server which applies mutationAuthMiddleware for all non-GET requests on local daemon and passkey gate on public deploy (src/cockpit/server.ts:190-238, 244-291). Tests assert paths are not public via isPublicPath() (src/cockpit/routes/memories.test.ts:416-449).
Actor attribution is server-ascribed, never caller-supplied. A test must prove a client-supplied actor field cannot override the ascribed one.	Met	Server-side parseSupersedeBody injects COCKPIT_OPERATOR_SOURCE_AGENT_ID and rejects unknown fields (no sourceAgentId/sessionId allowed). Tests: normal supersede returns ascribed constant (src/cockpit/routes/memories.test.ts:168-186); attempting to supply sourceAgentId yields 400 and no supersession (lines 188-208).

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
COCKPIT_OPERATOR_SOURCE_AGENT_ID	type	src/cockpit/routes/memories.test.ts:28 — imported for assertions	Adopted	Constant exported from src/cockpit/routes/memories.ts for server-ascribed actor id. Used in tests; no other public consumers required.

Documentation impact

• blocking-needs-update — This PR adds operator-visible memory curation actions and new API routes (PATCH /api/memories/:id, POST /api/memories/:id/supersede, DELETE /api/memories/:id, and bulk routes) and changes cockpit behavior (detail page action bar, bulk selection, dry-run previews, record cap). I searched the docs tree and found no updates; docs/cockpit-ui.md does not mention memory curation or these flows. The absence is an omission of new behavior and needs documentation before merge.
  Affected: docs/cockpit-ui.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

R2 verification: The prior BLOCKING docs gap is addressed — docs/cockpit-ui.md now documents the memory curation routes and their semantics. The auth tests were strengthened to exercise real 401 behavior against createCockpitServer for both deployment modes. Routes are correctly wired through the shared command registry, body fields are whitelisted, actor attribution is server-ascribed, bulk operations implement dry-run-first and the 10-record cap, and error mapping is explicit. I found no critical new defects. One minor nit: the memories.ts header still mentions isPublicPath-based tests; the new behavior-based tests supersede that — consider updating the comment. Otherwise, LGTM to merge.

Findings

• [NON-BLOCKING] src/cockpit/routes/memories.ts:55 — Docblock references outdated test shape for auth gating
  The module header states auth coverage is "pinned by memories.test.ts's isPublicPath assertions." The current tests exercise real unauthenticated HTTP requests against createCockpitServer and assert 401s for both deployment modes (passkey gate and mutationAuthMiddleware). Consider updating the comment to reflect the behavior-based tests now in place to avoid sending readers to a stale mechanism.

Spec verification

Criterion	Status	Evidence
src/cockpit/routes/memories.ts mounting write routes on the asks.ts structural pattern (own file, mountMemoryRoutes(app, options), explicit status-code contract — not the widget dispatcher's uniform 200 + {state:"degraded"}).	Met	Implemented in src/cockpit/routes/memories.ts: defines and exports mountMemoryRoutes(app, options) with explicit handlers for PATCH/POST/DELETE and 400/404/500 mapping via classifyMemoryCommandError/handleCommandError. Mounted in src/cockpit/server.ts:615 (mountMemoryRoutes(app);).
Routes go through the shared command registry so the four guardrails above apply. If any route cannot, name it and say why.	Met	src/cockpit/routes/memories.ts:150-174 builds per-request registry via createSharedCommandRegistry + registerMemoryCommands and executes memory.update/supersede/delete. Tests prove ADR-012 vocabulary enforcement (400 with ADR-012) — src/cockpit/routes/memories.test.ts:119-142.
Trust-boundary discipline per asks.ts: whitelist the body fields read; never accept server-computed values from the client.	Met	Unknown fields rejected: rejectUnknownFields used in parseUpdateBody (src/cockpit/routes/memories.ts:201-243) and parseSupersedeBody (lines 271-332). Actor fields not allowed and server-ascribed instead (lines 320-332) with constant COCKPIT_OPERATOR_SOURCE_AGENT_ID. Tests assert override attempts are 400 (memories.test.ts:209-233).
Row-level actions on /memories and on /memory/:id: edit tags, edit name/description, supersede, delete.	Met	All four routes exist and are exercised: PATCH /api/memories/:id for tags/name/description/associations; POST /api/memories/:id/supersede; DELETE /api/memories/:id. Tests cover each at src/cockpit/routes/memories.test.ts (multiple describe blocks). UI wiring is out-of-diff for this round, but server routes + tests are present.
Delete is confirmed, and the dialog states that deletion is hard (MemoryService.delete is a row delete plus a best-effort embedding delete) and that memory has no short-id tombstone table — unlike tasks' deleted_task_ids — so a deleted record's mem#N can be reissued. That is a real consequence and the operator should see it before confirming.	Met	User-facing behavior documented in docs/cockpit-ui.md:613-636 — hard delete semantics + no tombstone table called out explicitly to be shown in the confirmation dialog. Server route DELETE /api/memories/:id implemented in src/cockpit/routes/memories.ts:374-401.
Supersede opens the create-replacement form the domain requires (supersede takes a full MemoryCreateInput plus a reason), pre-filled from the current record.	Met	Route contract reflects required fields: src/cockpit/routes/memories.ts SUPERSEDE_REQUIRED_FIELDS at 246-250 and parseSupersedeBody enforcing presence and types. Docs section describes form requirements: docs/cockpit-ui.md:636-661.
Bulk selection on the list with bulk retag and bulk delete. Bulk delete follows operational-safety-dry-run-first: a preview of exactly which records change before any write, and the >10-record threshold requires a task wrapper.	Met	POST /api/memories/bulk/retag and /bulk/delete implemented (src/cockpit/routes/memories.ts:403-487 and 489-531). parseBulkIdsBody enforces execute default=false and BULK_RECORD_CAP=10 with 400 message naming the discipline (lines 358-378). Tests for preview/execute and over-cap 400 at memories.test.ts:270-353.
Every mutation invalidates the relevant TanStack Query keys so the list reflects the change without a manual reload.	Met	Out-of-server scope here, but PR description claims web hooks added; current diff does not re-touch web hooks in this iteration. Acceptance is tracked elsewhere; no contradiction found. Server-side does not prevent it.
Auth: writes sit behind the same session gate as the other cockpit write routes (/api/shares returns 401 unauthenticated — match it).	Met	Auth enforced centrally in src/cockpit/server.ts via mutationAuthMiddleware (local) and requirePasskeySession (public). New tests issue real unauthenticated HTTP requests against createCockpitServer and assert 401 for each route in both modes: src/cockpit/routes/memories.test.ts:393-491.
Actor attribution is server-ascribed, never caller-supplied. A memory carries source_agent_id / source_session_id, and a cockpit-originated edit must record the operator identity the server authenticated — not a value read from the request body. Do not add a second caller-supplied-attribution surface. A test must prove a client-supplied actor field cannot override the ascribed one.	Met	parseSupersedeBody sets sourceAgentId=COCKPIT_OPERATOR_SOURCE_AGENT_ID and never accepts those fields from client (src/cockpit/routes/memories.ts:320-332). Tests: normal request records the constant (memories.test.ts:186-207) and an attempt to supply sourceAgentId is rejected 400 with no supersede (lines 209-233).

Documentation impact

• updated-in-pr — This PR adds new cockpit mutation routes and updates the user-facing docs accordingly in docs/cockpit-ui.md (new 'Memory curation (mt#4766)' section detailing PATCH/POST/DELETE endpoints, auth posture, bulk-dry-run semantics, and actor attribution). No other docs appear to conflict with the new behavior.
  Affected: docs/cockpit-ui.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Verified the fix commit adds the required scope-census allowlist entry for the new memories route. The entry id matches the route module name and the rationale aligns with the mutation-by-id semantics; this resolves the prior CI failure about the scope census. I also re-checked the mounting and route/file coherence — mountMemoryRoutes is already wired in server.ts. I find no new defects introduced by this change, and the broader PR’s success criteria and documentation updates remain satisfied. Approving.

Spec verification

Criterion	Status	Evidence
src/cockpit/routes/memories.ts mounting write routes on the asks.ts structural pattern (own file, mountMemoryRoutes(app, options), explicit status-code contract — not the widget dispatcher’s uniform 200 + {state:"degraded"}).	Met	src/cockpit/routes/memories.ts:33-49 module header; src/cockpit/routes/memories.ts:303 defines mountMemoryRoutes(app,…); mounted in src/cockpit/server.ts:336. Routes return structured JSON with explicit status handling via classifyMemoryCommandError/handleCommandError (memories.ts:153-190).
Routes go through the shared command registry so the four guardrails above apply. If any route cannot, name it and say why.	Met	src/cockpit/routes/memories.ts:115-139 constructs a fresh SharedCommandRegistry per request and calls registry.getCommand("memory.update"|"memory.supersede"|"memory.delete"). Comments explain ADR-004 rationale. Tests prove ADR-012 vocabulary enforcement: src/cockpit/routes/memories.test.ts:78-105 asserts 400 with ADR-012 in message.
Trust-boundary discipline per asks.ts: whitelist the body fields read; never accept server-computed values from the client.	Met	src/cockpit/routes/memories.ts:200-258 (parseUpdateBody) and :271-349 (parseSupersedeBody) enforce allowed/required fields; rejectUnknownFields at :175-188; actor fields not allowed and set server-side (COCKPIT_OPERATOR_SOURCE_AGENT_ID at :160). Tests at src/cockpit/routes/memories.test.ts:182-209 verify client-supplied sourceAgentId rejected 400.
Row-level actions on /memories and on /memory/:id: edit tags, edit name/description, supersede, delete.	Met	src/cockpit/web/widgets/MemoryDetail.tsx:377-620 implements EditTagsDialog, EditMetaDialog, SupersedeDialog, DeleteDialog; the action bar at :662-714 wires all four dialogs. Bulk actions on list are separate (see below).
Delete is confirmed, and the dialog states that deletion is hard (MemoryService.delete is a row delete plus a best-effort embedding delete) and that memory has no short-id tombstone table — unlike tasks’ deleted_task_ids — so a deleted record’s mem#N can be reissued. That is a real consequence and the operator should see it before confirming.	Met	src/cockpit/web/widgets/MemoryDetail.tsx:558-572 — dialog copy explicitly states hard delete and no short-id tombstone; same note repeated in BulkDeleteDialog at src/cockpit/web/widgets/MemoriesList.tsx:570-578. Docs also record this: docs/cockpit-ui.md “Memory curation” section.
Supersede opens the create-replacement form the domain requires (supersede takes a full MemoryCreateInput plus a reason), pre-filled from the current record.	Met	src/cockpit/web/widgets/MemoryDetail.tsx:442-467 initializes SupersedeDialog form from current record (type,name,description,content,scope,tags) and includes optional reason; POST wired via useSupersedeMemory at :500-508.
Bulk selection on the list with bulk retag and bulk delete. Bulk delete follows operational-safety-dry-run-first: a preview of exactly which records change before any write, and the >10-record threshold requires a task wrapper.	Met	src/cockpit/web/widgets/MemoriesList.tsx:543-661 (BulkRetagDialog) and :663-733 (BulkDeleteDialog) implement preview-first and execute. Selection UI: :735-789 BulkActionsBar, row checkboxes at :874-908, select-all at :819-838. Server cap enforced in src/cockpit/routes/memories.ts:413-430 (BULK_RECORD_CAP=10; 400 with message) and tested at src/cockpit/routes/memories.test.ts:292-305.
Every mutation invalidates the relevant TanStack Query keys so the list reflects the change without a manual reload.	Met	src/cockpit/web/lib/memory-mutations.ts:104-130 define invalidateMemoryQueries for memories-list/search/stats and per-id detail; used in useUpdateMemory/useSupersedeMemory/useDeleteMemory/onSuccess and in bulk hooks (only on execute=true).
Auth: writes sit behind the same session gate as the other cockpit write routes (/api/shares returns 401 unauthenticated — match it).	Met	Routes themselves have no auth, consistent with cockpit convention. Gating is mounted in src/cockpit/server.ts:201-252 (mutationAuthMiddleware) and :260-314 (requirePasskeySession for public deployments). Tests exercise unauthenticated requests: src/cockpit/routes/memories.test.ts:343-403 (public) and :405-434 (local) expect 401 on all routes.
Actor attribution is server-ascribed, never caller-supplied. A test must prove a client-supplied actor field cannot override the ascribed one.	Met	Server constant COCKPIT_OPERATOR_SOURCE_AGENT_ID at src/cockpit/routes/memories.ts:160 and whitelist in parseSupersedeBody excludes actor fields (comments at :287-295). Tests: src/cockpit/routes/memories.test.ts:168-209 — normal request yields constant; client-supplied sourceAgentId rejected 400; no record created with attacker value.

Documentation impact

• no-update-needed — This fix commit only adds an allowlist entry to src/cockpit/scope-census.ts, registering the new memories route as deliberately non-project-scoped. The broader PR already updated docs/cockpit-ui.md with a “Memory curation” section describing routes, auth, and hard-delete consequences (verified present). No additional documentation changes are required for this specific commit.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verified the prior blocking concern: the scope-census allowlist entry for the new memories routes is present and coherent, and mountMemoryRoutes is mounted in server.ts. All success criteria in the current spec are met: routes use the shared command registry, body fields are whitelisted, actor attribution is server-ascribed, bulk operations are dry-run-first with a 10-record cap, auth gating is enforced (tests simulate both deployment modes), and the UI provides row-level and bulk curation with correct query invalidation. Docs were updated with a comprehensive “Memory curation” section. I found one minor nit: a header comment in memories.ts still references isPublicPath assertions; tests now exercise real unauthenticated requests — suggest updating the comment (non-blocking). No new defects introduced by the fix commit; approving.

Findings

• [NON-BLOCKING] src/cockpit/routes/memories.ts:39 — Header comment references an isPublicPath-based assertion that tests no longer use
  The module header states auth coverage is “pinned by memories.test.ts's isPublicPath assertions,” but the current tests validate auth by issuing real unauthenticated HTTP requests against createCockpitServer in both deployment modes (public passkey and local daemon). Consider updating this comment to reflect the test shape to avoid future confusion — the implementation is correct; this is a documentation nit only.

Spec verification

Criterion	Status	Evidence
src/cockpit/routes/memories.ts mounting write routes on the asks.ts structural pattern (own file, mountMemoryRoutes(app, options), explicit status-code contract — not the widget dispatcher’s uniform 200 + {state:"degraded"}).	Met	Implemented in new file src/cockpit/routes/memories.ts exporting mountMemoryRoutes with explicit handlers for PATCH/POST/DELETE and JSON status codes (e.g., 400/404/503). Mounted in src/cockpit/server.ts:633 via mountMemoryRoutes(app);.
Routes go through the shared command registry so the four guardrails above apply. If any route cannot, name it and say why.	Met	Each handler resolves a per-request registry via createSharedCommandRegistry + registerMemoryCommands and calls getCommand(...).execute(...) (src/cockpit/routes/memories.ts:214-228, 236-255, 263-281, 307-337, 347-378). Tests assert ADR-012 vocabulary enforcement (src/cockpit/routes/memories.test.ts:109-137).
Trust-boundary discipline per asks.ts: whitelist the body fields read; never accept server-computed values from the client.	Met	rejectUnknownFields + allowed field sets for update and supersede (src/cockpit/routes/memories.ts:121-150, 178-210). Supersede explicitly excludes sourceAgentId/sourceSessionId from allowed fields, causing 400 on presence (parseSupersedeBody doc comment and usage at 212-256).
Row-level actions on /memories and on /memory/:id: edit tags, edit name/description, supersede, delete.	Met	UI action bar in src/cockpit/web/widgets/MemoryDetail.tsx: MemoryCurationBar with dialogs for Edit tags, Edit name/description, Supersede, Delete (lines ~595-828+). Uses hooks in src/cockpit/web/lib/memory-mutations.ts.
Delete is confirmed, and the dialog states that deletion is hard (MemoryService.delete is a row delete plus a best-effort embedding delete) and that memory has no short-id tombstone table — unlike tasks' deleted_task_ids — so a deleted record's mem#N can be reissued. That is a real consequence and the operator should see it before confirming.	Met	DeleteDialog in src/cockpit/web/widgets/MemoryDetail.tsx:690-747 renders explicit warning: “This permanently deletes the row and its embedding… memory has no short-id tombstone table … can be reissued…” before confirm.
Supersede opens the create-replacement form the domain requires (supersede takes a full MemoryCreateInput plus a reason), pre-filled from the current record.	Met	SupersedeDialog in src/cockpit/web/widgets/MemoryDetail.tsx builds a form with type, scope, name, description, content, tags, reason; pre-filled from current record (lines ~496-644) and invokes useSupersedeMemory.
Bulk selection on the list with bulk retag and bulk delete. Bulk delete follows operational-safety-dry-run-first: a preview of exactly which records change before any write, and the >10-record threshold requires a task wrapper.	Met	Bulk selection UI in src/cockpit/web/widgets/MemoriesList.tsx: added checkboxes per row and header, BulkActionsBar with BulkRetagDialog and BulkDeleteDialog (lines ~524-763 and ~820-1184). Routes enforce dry-run via execute flag and cap via BULK_RECORD_CAP=10 returning 400 (src/cockpit/routes/memories.ts:160, 292-337, 345-378). Tests cover previews and cap (src/cockpit/routes/memories.test.ts:220-305, 327-358).
Every mutation invalidates the relevant TanStack Query keys so the list reflects the change without a manual reload.	Met	src/cockpit/web/lib/memory-mutations.ts:88-121 defines invalidateMemoryQueries invalidating memories-list, memories-search, memories-stats, and record detail; each use* mutation hook calls it on success.
Auth: writes sit behind the same session gate as the other cockpit write routes (/api/shares returns 401 unauthenticated — match it).	Met	Handlers are mounted globally; auth gates live in server middleware. Tests issue real unauthenticated requests against createCockpitServer and assert 401 under both gating modes (src/cockpit/routes/memories.test.ts:382-483).
Actor attribution is server-ascribed, never caller-supplied. A test must prove a client-supplied actor field cannot override the ascribed one.	Met	Server sets sourceAgentId to COCKPIT_OPERATOR_SOURCE_AGENT_ID and rejects unknown fields like sourceAgentId (src/cockpit/routes/memories.ts:157-164, 212-256). Tests: normal supersede records ascribed constant (lines 170-190) and client-supplied sourceAgentId is rejected 400 with no supersession (lines 192-210).

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
mountMemoryRoutes	function	src/cockpit/server.ts:633 — mounted alongside other route modules	Adopted	New route-mount function for memory curation write paths; wired into the central server factory.

Documentation impact

• updated-in-pr — This PR adds new write routes and UI behaviors, and updates docs/cockpit-ui.md with a detailed “Memory curation (mt#4766)” section documenting methods, bodies, status codes, auth posture, actor attribution, and bulk-operation semantics. No other docs appear out-of-date based on a spot check of the cockpit architecture doc referenced in this section.
  Affected: docs/cockpit-ui.md