fix(cli): warn on unattributable created_by independent of --assign/--unassigned

[andrei-hasna]

What

Two operator-facing surfaces in todos CLI told an unidentified session its writes were attributed, after #192 (0.15.2) made that false.

F1 (P1) — todos add --assign <agent> silenced the attribution warning while created_by went null. The gate was keyed on assignee (!assignee && !opts.unassigned), which used to imply attribution but no longer does after #192 routed created_by through the guarded resolveWritableIdentity. An anonymous filer with --assign brutus got a real owner and a silently null created_by.

F2 (P2) — todos init's success message ("Identity saved — later commands attribute to this agent automatically") is false on every column since #192: the persisted identity file is read-only for display/--inbox and is never written into a task. The collision path (exit 2) already names the correct escape hatch (export TODOS_AGENT_ID=<name>); the success path — the one every fresh session hits — now says the same thing instead of the opposite.

Fix

• task-commands.ts: split the single !assignee gate into two independent arms — ownerless (unchanged: no assignee and not --unassigned, prints the existing combined "ownerless and unattributable" text) and a new !router.agent_id arm that fires whenever the row will be filed with a null author, independent of --assign/--unassigned (prints an attribution-only message, since such a row is not actually ownerless — e.g. it has an assignee via --assign).
• agent-commands.ts: init's success message now tells the truth and prints the export TODOS_AGENT_ID=<name> / --agent <name> escape hatch, mirroring the collision path's existing wording.
• creator-attribution.test.ts: re-pinned the one test (does not warn when the task is explicitly assigned) that encoded the old, now-incorrect behaviour; added two new cases (unattributable-with-assignee, unattributable-with---unassigned); added one case for F2.
• CHANGELOG.md: [Unreleased] entries for both fixes.

Evidence

• Reproduced first, independently of the filing task's own measurement, with a standalone script against the installed source tree (not just the test harness): CASE B (init Brutus then add --assign brutus, no env) produced created_by: null with stderr carrying only the unrelated no-project warning — no attribution warning at all. A positive control (no init, no --assign) fired the existing warning correctly, ruling out an inert probe.
• TDD: wrote the 3 new/amended test cases first, confirmed all 3 failed against the unfixed source (RED), then implemented the fix and confirmed all pass (GREEN) — pasted below.
• Re-ran the standalone repro script after the fix: CASE B now warns "task is unattributable — created_by will be recorded as null..."; the true-ownerless positive control is unchanged ("ownerless and unattributable"); CASE D (TODOS_AGENT_ID set + --assign) stays silent.

$ bun test src/cli/creator-attribution.test.ts
 23 pass
 0 fail
 81 expect() calls
Ran 23 tests across 1 file.

Also ran the 4 directly-related CLI test files that reference this warning text or the init success message (ambient-identity-misrouting.test.ts, cloud-comment-detail.test.ts, cloud-list-filter.test.ts, orphan-project-inflow.test.ts) — none of them invoke --assign in a way that reaches the new branch, and all pass:

$ bun test src/cli/ambient-identity-misrouting.test.ts src/cli/cloud-comment-detail.test.ts src/cli/cloud-list-filter.test.ts src/cli/orphan-project-inflow.test.ts
 55 pass
 0 fail
 250 expect() calls
Ran 55 tests across 4 files.

tsc --noEmit -p tsconfig.typecheck.json is clean.

remote-entrypoint.test.ts — 4 failures on this branch; 3 of the 4 are NOT this diff's fault, and I am not claiming the 4th is either

Ran this file focused (station load ~9-14 all session, /proc/pressure/cpu full avg10=0.00 throughout — not real CPU contention, so the likely culprit is the hardcoded 5000/30000ms budgets themselves rather than genuine slowness; that is a separate, pre-existing row, not something I'm fixing here). 4 fail / 3 errors out of 12 on this branch, all exitCode: 143 (SIGTERM):

1. built projects --deregister applies its guards and deletes through /v1 — timeout at 5000ms
2. built safe coordination handlers use only V1 and preserve a synthetic filesystem — timeout at 5000ms
3. built remote done and complete alias persist completion through /v1 — timeout at 5000ms
4. built project/list/plan/task lifecycle stays on HTTP with a read-only TODOS_DB_PATH — timeout at 30000ms

None of the four touches add/--assign/init by content — they exercise deregister, unlock/active/timeline, done --confidence, and a generic lifecycle path.

To rule out a regression rather than assume it, I stashed this fix and re-ran the same 4 test names, filtered, against the clean unmodified feac577 base tree:

$ git stash push -u -- CHANGELOG.md src/cli/commands/agent-commands.ts src/cli/commands/task-commands.ts src/cli/creator-attribution.test.ts
$ bun test src/cli/remote-entrypoint.test.ts -t "built projects --deregister ...|built safe coordination handlers ...|built remote done and complete alias ...|built project/list/plan/task lifecycle ..."
 1 pass
 8 filtered out
 3 fail
 2 errors
Ran 4 tests across 1 file.
$ git stash pop

3 of the 4 reproduced byte-for-byte on the clean base tree — identical test names, identical exitCode: 143, identical assertion lines (:505 for #1, :779 for #2, the 30000ms timeout for #4): built projects --deregister ..., built safe coordination handlers ..., and built project/list/plan/task lifecycle .... That is 1 pass + 3 fail out of the 4 filtered — pre-existing, load-dependent flakiness, not caused by this diff.

The 4th, built remote done and complete alias persist completion through /v1, is the one that PASSED on the clean base tree in this specific run (it's the 1 pass above). I am not claiming it reproduced — it didn't, in this run. Given it also failed with the identical exitCode: 143 signature on the fixed tree, and given the file's tests interleave and share process/port state across a single 214s run, the honest read is "consistent with the same class of flakiness, not independently confirmed" — not "proven pre-existing" the way the other three are. Flagging this distinction explicitly rather than rounding it up to 4/4.

Scope not covered (unchanged from the filing task)

The MCP create_task path, todos comment/start/complete, and anything off-box or in e2b were not touched or measured here — F1/F2 are CLI-only, matching the filing task's explicit scope.

todos task: a3f4bb1a

[andrei-hasna]

[REVIEW] GO — #194 @ 1452788 — lens: security, reviewer todos-pr194-reviewer (1 of 1)

Head sha verified at review time: 14527882c71c01410fcb14c7cba470ee6a32f8b1 (matched the dispatch, and matched again immediately before writing this). Base main, merge-base == origin/main (no retarget — git merge-tree --write-tree origin/main HEAD produced a tree identical to HEAD, git diff --quiet rc=0). 0 prior GitHub reviews, 0 prior PR comments confirmed before this one.

What I verified first-hand (not taken from the PR body)

1. RED/GREEN, independently reproduced. I swapped agent-commands.ts and task-commands.ts back to their HEAD~1 (feac577) content while keeping the HEAD test file, and ran bun test src/cli/creator-attribution.test.ts:
   20 pass / 3 fail — the failures are exactly the 3 test cases that assert genuinely new behavior (F1's assign-with-no-identity case, F1's unassigned-with-no-identity case, F2's init-message case). The 4th new test ("stays silent when a per-process identity is set, even with --assign to someone else") passes on both trees because the old gate already handled that combination correctly — it's a non-regression pin, not a discriminating case, and I confirmed that's why it doesn't appear in the RED failures.
   Restored HEAD content exactly (git diff --stat empty), then re-ran: 23 pass / 0 fail, 81 expect() calls — matches the PR body's own figures.
2. Exhausted the branch logic by hand, not just by test. ownerless = !assignee && !opts.unassigned and the new else if (!router.agent_id) are mutually exclusive by construction (if/else if), so no input can print both. I walked every combination of (--assign present/absent) × (--unassigned present/absent) × (router.agent_id present/absent — confirmed via resolveWritableIdentity/isProcessBoundSource that this is non-null only for explicit/env sources, never the persisted file) and found no combination that should warn but stays silent, and none that already got a correct warning pre-diff and now doesn't. The comment's claim "whenever ownerless fires, router.agent_id is null too" holds structurally: assignee only falls back to router.agent_id when !opts.unassigned, so ownerless (which requires !assignee && !opts.unassigned) forces router.agent_id falsy on that same branch.
3. Scope claim checked. grep'd for the pre-existing "ownerless and unattributable" string and bare !assignee gates across src/ (excluding tests) — the gate exists in exactly one place, todos add. The PR's "MCP create_task / comment / start / complete not touched" is accurate; there's no silently-duplicated instance of this defect elsewhere in the CLI that the PR should also have fixed.
4. The flakiness claim, independently re-run, not trusted from the description. Ran the same 4 filtered remote-entrypoint.test.ts test names on HEAD: 4 fail / 3 errors, all exitCode: 143. Then reverted the same 2 files to feac577 (test file untouched) and re-ran the identical filter: 1 pass / 3 fail / 2 errors, and the 3 failures are byte-identical to HEAD's — same test names, same assertion lines (:505, :779), same exitCode: 143. The one that passed on base (built remote done and complete alias...) is exactly the one the PR author declined to claim as confirmed pre-existing rather than rounding to 4/4 — that precision held up under a second, independent run. /proc/loadavg was 4.63 on 20 cores and /proc/pressure/cpu full avg10=0.00 when I ran it, consistent with "not real contention." This is pre-existing, load-dependent flakiness in unrelated commands (deregister/unlock/timeline/done), not a regression from this diff.
5. Typecheck clean: tsc --noEmit -p tsconfig.typecheck.json rc=0.
6. The 4 adjacent files: ambient-identity-misrouting.test.ts cloud-comment-detail.test.ts cloud-list-filter.test.ts orphan-project-inflow.test.ts → 55 pass / 0 fail / 250 expect() calls, matching the PR body exactly.
7. Isolation held. runCli's localRoutingTestEnv/localTodosTestEnv (in src/testing.ts) blanks every SHARED_TODOS_STORE_ENV_KEYS entry and pins HASNA_TODOS_STORAGE_MODE=local; I additionally ran a manual invocation with the same scrub and confirmed the write landed only in a throwaway temp DB (deleted after), never touching the ambient HASNA_TODOS_API_URL-pointed store my own shell had configured.

Security-lens finding — non-blocking, pre-existing, not introduced by this diff

agent-commands.ts's new success-path message (mirroring the existing collision-path wording verbatim, as the PR states) prints export TODOS_AGENT_ID=${result.name} / --agent ${result.name} as a copy-pasteable shell line. Locally-registered names are constrained to /^[a-z]+$/ (validateAgentName in src/db/agent-names.ts), which forecloses shell-metacharacter injection on that path. cloudRegisterAgent (src/cli/cloud-router.ts:1414), however, forwards the name to POST /agents with no client-side format check, so if the server-side validation is looser than the client's, a crafted agent name could turn this into a copy-paste command-injection vector — a user who trusts the suggested line and runs it verbatim would execute arbitrary shell content embedded in the name. I can't verify server-side validation from this client repo, and this exact pattern already existed, unmodified, in the collision path before this PR (agent-commands.ts:178, pre-existing) — this PR only adds a second call site with identical exposure, it does not introduce the underlying gap. Non-blocking per the pre-existing/out-of-scope carve-out; worth a follow-up task to either confirm server-side name validation or escape/quote the suggested line.

Answers to the specific questions

• Fires when it should, silent when it should: yes, verified both by test and by exhaustive manual truth-table walk (above).
• Both-messages or neither-message gap from the arm split: no — structurally impossible (mutually exclusive if/else if), confirmed by walking the branches.
• Is the test real: yes — RED confirmed on feac577 content, GREEN confirmed on HEAD content, both independently reproduced by me, not merely re-read from the PR body.
• F2 message accuracy: confirmed against src/lib/creator-identity.ts that the persisted file is genuinely display-only (resolveCreatorIdentity vs. resolveWritableIdentity / isProcessBoundSource) — the new message text is accurate.

What I did not check

Off-box/e2b behavior, the MCP create_task tool path, and the 4th remote-entrypoint.test.ts failure's true classification (the PR author already declined to claim it; I did not attempt to resolve that ambiguity further since it's pre-existing and unrelated to this diff either way). Server-side agent-name validation on the cloud API (noted above) — no visibility from this client repo.

[andrei-hasna]

[REVIEW] GO — #194 @ 1452788 — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1)

Reviewed the exact candidate against freshly fetched origin/main at feac577. I read git log --oneline origin/main..HEAD, the full origin/main...HEAD diff for all four changed files, and surrounding implementation in src/cli/commands/task-commands.ts, src/cli/commands/agent-commands.ts, src/lib/creator-identity.ts, src/db/task-crud.ts, plus the creator-attribution regression tests and changelog.

Commands and measured results:

• git log --oneline origin/main..HEAD — exit 0; one commit.
• git diff origin/main...HEAD --stat — exit 0; 4 files changed, 111 insertions, 4 deletions.
• bun install — exit 0; setup only, 616 packages installed.
• bun run typecheck — exit 0; 0 TypeScript diagnostics (the gate emits no test-count summary).
• bun run test — exit 0; 3,330 pass, 0 fail, 46 skip; 3,376 tests across 260 files.
• Final git status --short — exit 0 with no output; the tested worktree remained clean.

Blocking P0/P1 findings: none. The warning branches correctly distinguish ownerless-and-unattributable from assigned-but-unattributable creation, while both local and cloud writes continue deriving created_by from the guarded process-bound identity. The init success text now matches that invariant and points to the working per-process escape hatches.

Non-blocking follow-ups: none.