fix(tasks): key create on (sourceRef, title), so a second ask cannot adopt the first ask's row (TASK-063)

[lilyshen0722]

The defect, reproduced twice in production

sourceRef alone was the unique key, so a create sharing a ref with an existing task returned that task's row: the caller's title was dropped, and if the row was settled it was reopened to pending.

• 2026-08-25 — TASK-052 (@pod-architect): one message raised two asks with two owners; the second create adopted the first ask's row and reopened a task that merged feat(threading): add human thread entry points #1169 had legitimately completed.
• 2026-09-25T08:09Z — TASK-163 (@ux-lead): a create with a done row's ref returned alreadyExists:false, reopened:true and put that row back to pending, under a title nobody chose. Re-completed 28 s later. fix(tasks): a sourceRef reopen preserves the completed run's notes and reports a differing title #1748 had made the reopen report the mismatch — it still reopened, so (a) was still live.

What changed

A source is provenance, not identity: one message or one PR can raise several asks with several owners. The identity of a create is the (sourceRef, title) pair.

• models/Task.ts — unique partial index on (podId, sourceRef, title). Same ref + same title is still an idempotent retry; a different title becomes its own row.
• routes/tasksApi.ts — the pre-check and the E11000 re-read match the pair, so a mismatched title can no longer reach the reopen branch at all. That branch's "submitted title was NOT applied" reporting is superseded rather than disabled: the title now gets its own row, and fix(tasks): a sourceRef reopen preserves the completed run's notes and reports a differing title #1748's notes-preservation is untouched.
• Response contract — a reopen answers alreadyExists: true, reopened: true. It used to answer false, and alreadyExists is the field a caller branches on to decide whether its own create landed, next to a row it never created. That contradiction is half of the report.
• Deploy-order guard — a database still carrying the ref-only index cannot perform the create the caller asked for, so that case answers 503 with code: 'task_source_ref_index_migration_pending' naming the migration, not a generic 500.
• backend/scripts/migrate-task-source-ref-identity.ts (npm run migrate:task-source-ref-identity, --dry supported) — drops podId_1_sourceRef_1_partial, creates the pair. Required, not optional: autoIndex creates declared indexes but never drops one it does not know about, and the legacy index is the stricter of the two.
• Discoverability — commonly_create_task's description names the key, so a caller learns it from the tool list rather than from a contradictory response. The three places documenting the ref-only contract (frontend/src/content/guides.json, docs-site/concepts/task-board.mdx) are corrected in the same change.
• AX audit entry 69 — an idempotency key the tool description never names is read as ordinary metadata.

Deploy order

1. Merge and deploy. autoIndex adds the pair index; the stricter legacy index is still present, so behaviour is unchanged except that a mismatched-title create answers 503 with the migration's name.
2. cd backend && MONGO_URI=… npm run migrate:task-source-ref-identity — prints the before/after index state and droppedLegacy.

Same-pair retries are unaffected at both steps; there is no data repair (the pair is a strict relaxation, so no existing document can violate it).

Evidence

• npx jest (Node 22): 429 suites / 4006 tests green, 24 pre-existing skips. Task suites together: 17/17.

• Migration run end-to-end on a real mongod, not only through the unit test: seeded the legacy index and a completed row, ran the npm script → legacyIndexPresent=true pairIndexPresentBefore=false pairIndexPresentAfter=true droppedLegacy=true; then inserted a second ask under the same ref → 2 rows, and the original is still done with its notes intact. The dry run and the repeat run both report no change.

• npm run lint:ts → 0 errors; npm run tsc:check → clean.

• Mutations, each asserted to apply exactly once, each restored byte-identical with a green baseline after:

  mutation	red
  pre-check keys on sourceRef alone again	2 tests
  index declared ref-only again	5 tests (model declaration + real index enforcement)
  reopen answers alreadyExists:false again	1 test
  deploy-order guard disabled	1 test

  The last mutation first produced "suite failed to run" — a build break, which is red for the wrong reason and would have read as a held guard. The valid form flips the condition instead of deleting the binding.

• frontend/scripts/generate-seo-pages.test.mjs green after the guides copy edit.

Two things worth a second opinion

1. This changes a published contract, deliberately: the key is composite rather than sourceRef alone. The alternative — keep the ref-only key and refuse a mismatched create with a conflict — fixes the harm but leaves sourceRef as identity, which is the thing that is wrong, and makes a legitimate second ask unfilable. If the product answer is instead "one row per source, ever", take the conflict variant and this PR is the wrong shape.
2. Docs copy now says "(sourceRef, title)" in guides.json and docs-site/concepts/task-board.mdx; if the wording matters to anyone downstream, it is three strings in one commit.

Not verified: the migration has not been run against the dev cluster — I have no DB access, so I cannot say whether that instance is still on the legacy index. If it is, the 503 names the migration rather than failing silently. The full frontend suite was not run: no frontend source changed, only guides.json.

Reviewer-measured evidence (@sprint-review, 2026-09-25)

• The unique key is now long. title sits inside it, and 6 live rows carry (ref,title) keys past the classic 1024-byte index limit — TASK-124 is 3275 B. Mongo dropped that limit in 4.2 and this repo is on 7/8, but it was run rather than assumed: a 3300-char title indexes fine, two asks under one ref give two rows, and an exact duplicate pair still throws E11000.
• The migration builds the pair index over six pre-seeded 3200-byte-key rows, drops the legacy index, and all six rows survive.
• Mutation campaign, independently re-run on this branch: pre-check keyed on the ref alone → 2 red; reopen answering alreadyExists:false → 1; the legacy 503 guard disabled → 1; index declared ref-only → 3.

Reopen semantics, stated (ux-lead's read of the shipped description)

A same-pair retry against a done row reopens it to pending and takes its assignee from the request — existing.assignee = assignee || undefined (tasksApi.ts:289), so a call that omits assignee leaves the reopened task unassigned. That is now in the tool description, docs-site/concepts/task-board.mdx and guides.json; four assertions pin the description itself (commonly-mcp/__tests__/tools.test.mjs), because after AX entry 69 the description is the contract.

The omitted-assignee reopen, pinned (sprint-review's gap, 73815)

The suite covered the reopen with assignee PRESENT and nothing covered it OMITTED — the half the clause exists to warn about. Now the 11th case in tasks.source-ref-idempotency.test.js: a done row assigned to ux-lead, asserted as a positive control, reopened on the same (sourceRef, title) with no assignee. It checks both readers, because they disagree on the sentinel and agree on the fact — the route assigns undefined, Mongoose $unsets the path, and hydration returns the String path's null default, so the response carries undefined and a fresh findById carries null. Mutation: existing.assignee = assignee || existing.assignee reds exactly this test.

[lilyshen0722]

The Tests red at b0477697 was a cli-suite race, not this diff — re-run is green

Attribution, first, because it decides whether this PR is pressable: it touches zero cli/ files (14 files: backend/routes/tasksApi.ts, backend/models/Task.ts, the migration + its suites, commonly-mcp/src/tools.js, frontend/src/content/guides.json, docs-site, the AX audit).

That job runs two bundles, and the split is the evidence:

bundle	result at b0477697
backend (npx jest)	green — 454 passed, 6 skipped; 4215 passed / 4239
cli	1 failed, 48 passed; 885 passed / 888

The single failure was cli/__tests__/adapters.pi.credential-channel.test.mjs:

SyntaxError: Unexpected end of JSON input
   83 |     const seen = JSON.parse(await waitForFile(out));

Root cause: the poll treats "the file exists" as "the child finished writing"

waitForFile (:36) resolves as soon as readFileSync succeeds — it only retries on ENOENT:

try { return readFileSync(path, 'utf8'); }
catch { if (Date.now() > deadline) throw ...; await sleep(25); }

The child writes with writeFileSync(out, JSON.stringify({...})), which is open-with-truncate, then write. readFileSync does not care that the open was in another process: a reader that lands in that window gets a zero-byte string, successfully. So the file becoming readable is not the file becoming complete, and JSON.parse('') throws. On a quiet machine the two syscalls are effectively adjacent; under CI load they separate.

Either half fixes it:

• child side — write atomically: writeFileSync(tmp); renameSync(tmp, out);
• test side — retry while the content is empty or unparseable, not only on ENOENT

The first is the better one: it removes the window instead of narrowing it, and it keeps waitForFile's contract ("the file appeared" ⇒ the probe is done).

Introduced with #1780 / #1801 / #1812 (2026-09-19/20) — not new today, and this is its first firing I can find in the recent Tests runs, so it may be rare rather than routine. Flagging it rather than opening a PR for it: it is cli/'s surface, and a 2-line fix is in hand for whoever owns that file.

State now

gh run rerun 36244929824 --failed → attempt 2: success. mergeStateStatus is CLEAN, all checks green, 2 behind main (13a98535); the stale-base guard passed live at 13:22:13Z on this head, well under its 40 ceiling.

[lilyshen0722]

DIRTY on main and resolved — head b0477697 → 30ac7349 (merge commit, no rebase). The conflict is a duplicate audit-entry number, and the resolution renumbers this branch rather than main.

What happened. a0a18011 (#1944, docs(ax): entry 69, a name resolves in its own scope) landed at 06:34Z. Pre-#1944 main's last agent-experience-audit.md entry was 68, and both sides took 69:

• main / docs(ax): entry 69, a name resolves in its own scope #1944 — 69. A name resolves in its own scope… (the isPodMember two-definitions entry)
• this branch — 69. An idempotency key that the tool description never names… (TASK-063)

Both append at the file's tail, so git reported a UU conflict on that one path and nothing else: git merge-tree --write-tree --name-only origin/main b0477697 → conflicted paths = docs/development/agent-experience-audit.md, exit 1. Effective diff files are unchanged (14) either way.

Resolution. Main's entry keeps 69; this branch's entry becomes 70. Main merged first and a number that is live on main wins — renumbering the merged copy would repoint every existing citation at different text. Ordered so the file reads 68 → 69 → 70.

The renumber had to reach a second file, which is the part a mechanical edit misses. commonly-mcp/__tests__/tools.test.mjs:235 carried // AX entry 69: sourceRef was an idempotency key the description never names… above the assertion on the tool description. Left alone it would have pointed a future reader at the isPodMember entry. It is now AX entry 70. That reference is what makes this entry resolvable from the code it describes, so it is corrected in the same commit rather than noted.

Verified on the merged tree (Node 22.23.1):

• tasks.source-ref-idempotency + migrate-task-source-ref-identity + Task.sourceRefIndex — 3 suites / 16 tests passed
• commonly-mcp — 3 suites / 73 tests passed, including create_task documents the idempotency key, not just the fields it dedups on
• no conflict markers remain in the audit file; agent-experience-audit.md is the only conflicting path and the only file this merge changed beyond main's own content

State: head 30ac7349, behind 0 — the stale-base guard re-ran live and passed (Stale-base merge guard ✔ on this sha). CI is in flight. prUrl stays null and the row stays open until merge.

Gate: the code PASS recorded on the TASK-063 row is at an earlier head and does not carry here. @sprint-review — re-gate requested at 30ac7349; the delta against the last gate is main's 24 commits plus this resolution, and the only branch-authored change inside it is the 69 → 70 comment.

Follow-up, not folded in. This is the second instance of the same class: on 2026-09-25 main gained a rule 34 (#1877) while an open PR added its own rule 34 in review-checklist.md, and #1885's scripts/verify-numbered-rules.js was written for exactly that — its header cites it. The AX audit has the same property (numbered entries cited by number from code comments) and no guard. Git only catches it because both sides append at the tail; a keep-both-69 resolution of this very conflict would have shipped silently, which is the failure the guard exists to prevent. Extending the guard to agent-experience-audit.md is a real follow-up — deliberately not folded into #1885 here, since that PR is press-ready and the addition would void a stale gate for a new scope.

[lilyshen0722]

Re-gate in: the PASS carries to 30ac7349 — and sprint-review re-ran it rather than accepting a carry on the description, on the grounds that two merges is exactly where a carry claim stops being free. No gate is outstanding on this PR.

The instrument is the part worth keeping. They compared this PR's own diff at both merge bases, not the delta between the two heads. Every file byte-identical except the two the renumber touched.

That is the right comparison and my ask got the question wrong. I wrote "no stylesheet or route code changed" — imprecise, and the imprecision is the kind that propagates: tasksApi.ts did gain a cloudflareIpRateLimitKeyGenerator refactor and authController.ts gained TASK-133b's reserved-identity block between c0b7abba and the head. Both arrived from main through the merge-in, not from this branch. A head-to-head delta cannot distinguish those, and "did any file change" is not the question a carry turns on — "did this PR's own diff move" is. Corrected here so the claim does not outlive the ask.

The renumber is confirmed correct, independently of my own reading: main's 69 is a different entry (connector-ops, 09-27), this branch's is 70, and commonly-mcp/__tests__/tools.test.mjs:235 cites 70 — the branch's own entry — so the citation resolves to the text it means.

Suites at the head: backend 16/16 across the three task suites; commonly-mcp 73/73. CI fully green. Press-ready.

Separately, a pre-existing defect in the file this PR renumbered — filed as its own row, not folded in here. Scanning the audit's headings on main: a duplicate entry 52 (two different entries, both dated 2026-08-30 — pod-architect's "The prescribed fix erases the evidence for the warning that prescribed it" and sprint-review's "A producer and a consumer, each green, disagreeing about one key's depth"), plus gaps at 39–40 and 44–46, max 69.

Reproduced rather than quoted, and the reproduction changes the severity: I extracted headings only (^## N.), and then asked which of those numbers is actually cited. None of 52, 39, 40, 44, 45 or 46 is cited anywhere in the repo — the only hits for 40 outside the audit were a MAX_LIMIT = 40. So the harm is latent, not active. It is still worth a guard: the file is cited by number from code (11 distinct numbers, all resolving), which is precisely the argument that earned review-checklist.md its numbering guard in #1885, and that checker takes --file.

[lilyshen0722]

Head rewritten: 30ac7349 → ff4fd428 — author-only. lily-shen flagged that c0b7abba is authored sprint-impl <agent@commonly.me>, and a squash merge emits Co-authored-by trailers for the PR's commits, which would put that identity on main.

I checked the mechanism before acting rather than taking it on trust: git log --format=%b origin/main | grep -o 'Co-authored-by:.*' returns 799 trailers across main, several of them agent identities (Cody <cody@commonly.me>, Nova <nova@commonly.me>, Ops (Commonly Agent) <ops-agent@users.noreply.github.com>). So the risk is real and the trailer is how it lands.

The rewrite changed no content. c0b7abba re-authored to Lily Shen <115414357+lilyshen0722@users.noreply.github.com> (author and committer), then the two merge commits replayed onto it:

git diff --stat 30ac7349 ff4fd428     # empty
git diff --shortstat origin/main...ff4fd428
 14 files changed, 484 insertions(+), 44 deletions(-)
diff <(git diff --name-only origin/main...30ac7349) \
     <(git diff --name-only origin/main...ff4fd428)   # IDENTICAL
git rev-list --format='%an <%ae>' a0a18011..HEAD | grep -v 'Lily Shen'   # (none)

GitHub now resolves all six commits to lilyshen0722. Trees are byte-identical, so the re-gate is a tree-equality check plus the suite, not a re-read of the change.

This supersedes the PASS at 30ac7349 (a PASS binds to a sha). Re-gate requested from @sprint-review on ff4fd428.

[lilyshen0722]

Re-gate: PASS at ff4fd428 — recorded here because it arrived by private consult and gh pr view 1876 --json reviews is empty, so the presser cannot see it on the PR. Quoting the gate verbatim; the head is unchanged by this comment.

Tree-identical: git diff --stat 30ac7349 ff4fd428 is empty, and both resolve to tree 5635fc95172d. So my earlier gate of the content carries in full.

Authorship, which is the actual deliverable: all six commits in a0a18011..ff4fd428 are Lily Shen <115414357+lilyshen0722@…> as author and committer. I checked both fields — the trailer comes from the author, but a stray committer is the kind of thing that shows up later in a different tool's view.

The thing your writeup didn't mention, and it's the one that would have defeated the exercise: no commit message in the range already contains a Co-authored-by trailer. Re-authoring the commits fixes the identity the squash derives, but a trailer sitting in a message body would have been copied through verbatim regardless — so the property you want holds for both routes, not just the one you fixed.

Shortstat matches at both shas: 14 files, +484/-44. Suites at the new head: backend 16/16, commonly-mcp 73/73.

The gap they closed, which is worth keeping. My re-authoring write-up named one route by which an identity reaches main — the author field a squash derives its Co-authored-by from. There is a second: a trailer already written into a commit message is copied through verbatim no matter who the author is. I verified the first and never looked at the second, so the property was true and under-proved. For the next identity remediation: check both the author/committer fields AND every message body in the range — git log --format='%b' <base>..<head> | grep -c 'Co-authored-by' should be 0.

State at ff4fd428: MERGEABLE, BLOCKED solely on Test & Coverage (pending); every other check green, including CodeQL and E2E Tests. Behind main by 11.