[lilyshen0722]

Follow-up 4 from QA walk 3 (fresh stranger, production 47ba879b): TASK-002 "Give your agent its first task" stayed pending after the hosted agent answered four asks. TASK-001 has a kernel event that completes it (a runtime token's first use, or the native provision, #2252). TASK-002 had none.

What changes

• agentAnswerService.humansAnsweredBy (new). This is feat(attention): an agent's answer to the person who asked opens no "needs you" row #2258's "who is this agent answering" rule, moved out of the attention writer so a second reader shares it instead of copying it. An agent's post answers:

  • the authors of the messages it holds a live claim on in the pod;
  • the author of a message it explicitly replies to.

  attentionItemService now calls it, with behaviour unchanged: feat(attention): an agent's answer to the person who asked opens no "needs you" row #2258's arms pass as they were.

• starterTaskService.completeFirstTaskOnAnswer. When an agent's post lands in a pod whose "Give your agent its first task" card is still open, and that post answers the workspace owner, the card is done ("Completed automatically — answered your first request").

  • Scout is excluded: it's the guide, not "your agent".
  • The open-card read comes first, so a pod with no open card (nearly every pod) costs one indexed Task.exists.
  • It never throws.

• AgentMessageService.postMessage offers each landed post to it, fire-and-forget, so a board write never delays or fails a post. A refused post offers nothing.

• The two starter cards now share one completion writer.

Why not the claim's completed release

A released claim's completed outcome also covers skips (the CLI's no-prompt and claim-held) and silent mention runs, so it doesn't mean the agent answered. A post landing while the agent holds the person's message is the answer itself, and it's the same signal attention uses.

Tests

• starterTaskService:
  • the owner answered → card done (filtered by sourceRef, open states only, task_updated emitted);
  • someone else, or nobody, answered → untouched;
  • Scout never counts;
  • no open card → stops at one read;
  • a failing read never throws.
• agentMessageService.threadWrites: a landed post is offered with its reply edge; a refused post isn't.
• attentionItemService: feat(attention): an agent's answer to the person who asked opens no "needs you" row #2258's arms all pass unchanged on the shared helper.
• Mutations: the owner check, the Scout exclusion, the open-card short-circuit, the post hook, and the helper's claims half and reply half each fail an arm.
• Locally on Node 22: starter task, attention, claims, all 11 agentMessageService suites, the hosted route, runtime auth, the activity queue and the PG message model: 352 tests, all green. tsc and eslint are clean.

The fresh-account re-walk after deploy is the end-to-end check: TASK-002 goes done after the first answered ask.

🤖 Generated with Claude Code

https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya

[lilyshen0722]

Written by sprint-review, a Commonly agent — code gate at 77e76904f. pod thread

Base 265e84fb6 (#2258). One finding, about an asymmetry the extraction creates.

The extraction is behaviour-preserving, and I proved it by re-running my own gate

"Attention's behaviour is unchanged" is the claim that matters, because a refactor that quietly loosened a guard would ship green. So I re-ran the mutations I used to gate #2258, against the extracted code:

my #2258 mutation, re-run here	result
escalation-marker short-circuit removed	same arm — "an answer carrying an escalation marker still opens their row"
isBot guard removed	same arm — "a human mention is unchanged"

Both still red, on the same arms, which is what makes the refactor trustworthy rather than merely green. The shared humansAnsweredBy is line-for-line the old body — same claim lookup, same /^\d{1,10}$/ filter, same Promise.all, same adds-after-every-await, same catch-leaves-empty — and the caller-specific checks (marker, isBot) correctly stayed in attentionItemService, matching the docstring's "callers decide what the answer means".

New arms also discriminate:

mutation	result
Scout exclusion removed	1 — "Scout answering is the guide, not "your agent""
owner check → any answered human	1 — "a post that answers someone else, or nobody, leaves it open"

BASE 3 suites / 56 passed; RESTORED 56/56.

The hook is correctly placed: right after _postToTarget stores the message, in the main post path only — the heartbeat-fallback and error-to-DM exits return earlier, and those are not answers. The claim is still live at that point, so liveClaimsHeldBy finds it. completeFirstTaskOnAnswer wraps its whole body in try/catch, so the void call cannot produce an unhandled rejection, and the open-card Task.exists really does come before the answer check, so a finished card costs one cheap query.

Finding: the marker exclusion did not come along to the second caller

ESCALATION_MARKERS appears zero times in both agentAnswerService and starterTaskService — only attentionItemService applies it. So for a post like [BLOCKED] @owner I need the staging key, holding a live claim on the owner's message:

• attention correctly still opens the needs-you row (it is an ask);
• onboarding completes "Give your agent its first task", and writes "Completed automatically — <agent> answered your first request".

The card therefore says the agent answered the request at the moment the agent said it is blocked on it. For attention the marker means "this is still an ask"; for the checklist it means "the first task is not done yet", so if anything the onboarding caller wants that check more than attention does. The two readings point the same way and the code applies it to only one.

Low severity — a checklist item completing early, and only when a first answer carries a marker. Not blocking, and not something I would fix by moving the check into the shared rule: attention's and onboarding's reasons differ, so the honest fix is the same caller-side test in completeFirstTaskOnAnswer.

One of my own probes was ill-posed

I tried a "fails open" mutation by inserting a throw after the sources.forEach adds, to see whether a partial set could survive the catch. It left 56/56 green, and that is not a surviving mutant — it is a probe of an unreachable state. There is no code after the forEach in the real function, so the docstring's "nothing is added before the last await" holds structurally and my mutant tested a path that cannot exist. Reporting it so the green line is not mistaken for a coverage gap.

Minor: 'scout' is a bare string literal in the exclusion rather than a shared constant; defensible for one builtin, worth a constant if a second agent ever needs excluding.

CI: 7 pass, 5 pending, nothing failing.

GATE: PASS

[lilyshen0722]

Written by sprint-review, a Commonly agent — re-gate at ee012fac1. pod thread

This supersedes my PASS at 77e76904f (review 5542972145). The finding is fixed, and fixed in the shape that keeps the two callers honest.

"Nothing else moved" checks out: the merge base is unchanged (265e84fb6 at both heads), and one commit touches four files, two of them tests.

The fix is caller-side, and better placed than I suggested

The marker test sits in completeFirstTaskOnAnswer, not in the shared agentAnswerService — which matters, because attention and onboarding read the same marker for opposite reasons ("still an ask" vs "not done yet"), and a shared rule would have had to pick one. It reuses the ESCALATION_MARKERS export rather than re-declaring the pattern.

It also sits before the open-card read, where I had only said "the same caller-side test". So a [BLOCKED] post now costs zero queries rather than one indexed read. Small, and the right way round.

The question I would have missed: is it the same string?

A marker check on one string and a marker check on a different string is two different guards wearing one name — which is exactly how I missed both security findings on #2249. So I traced it rather than assuming:

• attention's check reads message.content — the stored message (attentionItemService:126);
• the stored content is sanitizedContent (_postToTarget({ …, content: sanitizedContent }));
• onboarding now receives content: sanitizedContent from the same scope.

Both paths test the identical string. A marker that survives sanitisation reaches both checks, and one stripped by sanitisation reaches neither. No divergence to drift.

Measurement

BASE 3 suites / 57 passed; RESTORED 57/57. Failures by name:

mutation	result
marker check dropped (my finding reintroduced)	1 — "an answer that says it is blocked does not complete the card"
caller stops threading content (check goes inert via the '' default)	1 — "offers each landed post to the first-task check, with the reply edge"
Scout exclusion removed (re-run)	1 — same arm as at the previous head
owner check → any answered human (re-run)	1 — same arm as at the previous head

The second one is the one I most wanted to see. content = '' is a defaulted parameter, so a caller that quietly stopped passing it would make the new guard inert while every starterTask arm stayed green — the "declared but not delivered" shape. It is pinned: the threading has its own arm on the caller's side, and the re-runs confirm the two earlier guards did not regress while this was added.

My needle for that mutation matched 6 sites on the first attempt (content: sanitizedContent appears at every postMessage exit), so the assert blocked the write and the green total printed under that heading was an unmutated run. Re-scoped to agentLabel: displayName || agentName, content: sanitizedContent, it is unique and reds the arm above.

CI at this head: green apart from the usual pending Test & Coverage.

GATE: PASS

[lilyshen0722]

Written by sprint-review, a Commonly agent — re-gate at ca36f71c1. pod thread

This supersedes my PASS at ee012fac1 (review 5543010879). One line, merge base unchanged (265e84fb6 at both heads), nothing else moved.

The remediation is the right shape — a constant format string with the value as an argument — and it is non-behavioural. Measured rather than assumed:

new form renders: "[starter-task] first-task auto-complete failed for pod pod-1: boom"
old form rendered: "[starter-task] first-task auto-complete failed for pod pod-1: boom"
equivalent: true

%s consumes podId and the error message appends, so the log line a reader sees is byte-identical. 2 suites / 20 passed.

What this gate does not certify: CodeQL has not re-run

Analyze (javascript-typescript) is pending at this head, and the code-scanning query for this PR still returns one open alert — #1823 [high] js/tainted-format-string @ backend/services/starterTaskService.ts:118 — which is the previous analysis talking: the fix is now at :119, with your new comment at :118. So nothing has yet confirmed the alert cleared, and my PASS is on the change being the correct remediation for that rule, not on the scanner agreeing. Worth a glance at the check before pressing.

The twin: same construction, and correctly not flagged

completeConnectAgentStarterTask still builds its warn the same way, at :73:

console.warn(
  `[starter-task] connect-agent auto-complete failed for pod ${podId}:`,
  (err as Error).message,
);

I checked whether CodeQL had simply missed it rather than assuming either way. It has not: that podId comes from the podIds array, and both callers pass identifiers the agent is already authorised for — installationPodIds and req.agentAuthorizedPodIds in agentRuntimeAuth — i.e. DB-derived, not request-supplied. The first-task one is tainted because its podId arrives from the post request through agentMessageService. So the scanner's selectivity is right and this is not a defect.

It is still the same latent construction, one line from the same fix, and its safety rests on a provenance fact rather than on the call's shape. Worth normalising while the file is open; not worth holding for.

A note on my own method here: my first pass grepped console\.[a-z]+\(\`` for template-literal format strings and found **none** — a false negative, because that call puts the backtick on the next line. What caught it was counting the console.warn` sites (two) and reading both, rather than trusting the pattern. A regex over source lines cannot see a construct that spans them.

GATE: PASS