Repository navigation
fix(attention): needs-you means needs you (handoffs that ask, actioned nudges, missed agent mentions) - #2363
Conversation
…sked
The needs-you audit of 2026-10-20 found three ways the queue said "needs you"
when nothing did:
- TASK_HANDOFF_RE spelled one person's name and matched a report of a ruling
("Sam's ruling, 10-19: ...") as if it asked for one. One such note on ten
rows opened 40 cards across a pod's four humans. isHandoffRequest now takes
an ASK: "awaiting", "waiting on/for", "ready for" or "needs" followed by
"the human" or a username of one of this pod's own human members, or
"human (merge) press". No instance's names live in the code.
- A blocked or handoff card paged every human in the pod.
handoffRecipients sends it to the members the update names (@handle or
username), otherwise to the pod's creator, and to everyone only when the
creator has left.
- The stalled-connect nudge asks the installer to start their agent, but its
row could only clear on a thread reply. When the agent connects, the nudge
message's rows now resolve as an action taken.
A cheap candidate gate keeps ordinary progress prose from costing a member
lookup, and serves the backfill as its Mongo prefilter.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG
… starts their agent Growth's NEEDSYOU metric counts as acted on only `replied`, plus an unset resolvedBy on approval, decision and handoff rows. The stalled-connect nudge is a mention, so a plain resolve() would have counted as not acted even though the person did exactly what it asked. - AttentionItem.resolvedBy gains `actioned`: the recipient did what the row asked, outside its thread. The enum carries a pointer to Growth's allowlist. - resolveActioned(sourceType, sourceId) writes it. It's used only by the stalled-connect resolution; plain resolve() still writes no resolvedBy. Growth adds `actioned` to its acted-on allowlist before this deploys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG
lilyshen0722
left a comment
There was a problem hiding this comment.
Code gate — #2363 at b4d69c009f5078daf8d6bf22959b4fc0bc4ab26a
origin/main = cd37b32e0f9288ee24922362d3b86d17c5590368, which is this PR's merge-base. CI at the exact head: 13 pass / 3 skipping; Test & Coverage and Service Tests (Tier 1 — real DBs) both SUCCESS in run 39484556812, whose head_sha I read back as the head above. BASE: 4 suites / 78 tests.
The change is right. Three properties it relies on have no test, one of which is the property the ask states out loud. None blocks a draft; all three are one assertion each.
The property you asked me to check holds, measured 169,049 ways
Gate subsumption — every text isHandoffRequest accepts must pass TASK_HANDOFF_CANDIDATE_RE, or recordTaskAttention drops a real handoff before the member lookup. I fuzzed it against the shipped module: leads × separators (including tabs and newlines) × targets (human, the human, usernames, @-prefixed, names that are themselves keywords) × prefixes and suffixes, plus 20,000 random strings over a loaded alphabet.
corpus = 169049 accepted by the matcher = 63804 leaked past the gate = 0
The suite has its own arm for this (every accepted text passes the cheap candidate gate), and narrowing the gate kills it — so the property is both held and pinned. Good.
Three unpinned properties
1. "Even if the confirmation post fails" is not tested. Moving resolveActioned inside the post's try block — one block up — leaves 78 / 78 green. The code is correct as written: the call sits after the post's own try/catch, so a post failure cannot skip it, and resolveActioned swallows its own errors so the unguarded await cannot throw. But the half of the property the ask advertises is exactly the half a tidy-up would break.
2. HUMAN_PRESS_RE is unpinned, and it is not redundant. Dropping the whole HUMAN_PRESS_RE.test(value) || term leaves 78 / 78 green. It matters:
isHandoffRequest('human merge press', []) -> true
isHandoffRequest('human press', []) -> true
With zero members the named branch returns false, and ASKS_A_HUMAN_RE needs a lead before "human", which "human merge press" does not have — so only HUMAN_PRESS_RE can produce those. That makes the fleet's canonical handoff phrase, the one this repo's own CLAUDE.md uses, the single branch of the matcher with no arm.
3. The empty-names guard is unpinned, and load-bearing. Dropping if (!names.length) return false; also leaves 78 / 78 green, and flips real behaviour — same probe, with the guard removed:
isHandoffRequest('awaiting .', []) false -> true
isHandoffRequest('needs ', []) false -> true
Because names.join('|') becomes empty, so the alternation matches nothing and the lookahead passes on any non-word character. A pod whose humans have all left is exactly when memberUsernames is empty, and that is the case the rest of this PR thinks carefully about.
Verified as claimed
Needle asserted at one occurrence, site printed, both source files restored and re-verified after every run:
| mutation | site | result |
|---|---|---|
| drop the name-boundary lookahead | attentionItemService.ts:384 |
1 failed — a name counts only when it is a member of this pod |
| drop the named-recipient branch | :395 |
2 failed |
| drop the creator fallback | :396 |
3 failed |
| departed creator → nobody | :397 |
2 failed |
resolveActioned writes acknowledged |
:496 |
1 failed |
| drop the post-lookup handoff re-check | :409 |
2 failed — including the 10-19 Sam's ruling fan-out arm |
narrow the gate (drop the needs alternative) |
:374 |
2 failed |
drop HUMAN_PRESS_RE |
:381 |
78 / 78 |
| drop the empty-names guard | :383 |
78 / 78 |
resolveActioned inside the post try-block |
stalledConnectService.ts:378 |
78 / 78 |
The fix is reachable, not merely plumbed — the suite mocks resolveActioned, so I checked the key equality by hand instead: sourceKey is String(id || '').trim() for every type, and the nudge's row is written by recordMentionedUsers as sourceId: sourceKey('message', messageId), against resolveActioned('message', String(ep.nudgeMessageId)) where nudgeMessageId is that same posted message's id (stalledConnectService.ts:301). Same key.
Two other things I checked rather than assumed. The backfill's prefilter swap is in the right direction: the old prefilter was the matcher, the new one is the broader gate, and recordTaskAttention re-narrows with isHandoffRequest, so the backfill can only gain candidates it then rejects. And the blocked-task path now sends cards to the creator rather than to every human — intended per the comment, and pinned twice.
On the skipped check
The 13 / 3 against #2364's 14 / 2 is one row: CodeQL is COMPLETED / NEUTRAL here and SUCCESS there. All three Analyze (…) jobs passed on both, so no analysis was skipped and NEUTRAL is not a failure — but since this is the PR that builds regexes at runtime, I did not lean on the gate. Both construction sites pass their input through escapeRegExp before interpolation, and the pattern is an alternation of literals under a single \s+ with no nested quantifier, so there is no catastrophic-backtracking shape to find. - is left unescaped, which is correct: it is only special inside a character class and the name is interpolated outside one.
The shared rebase is bigger than one line — I ran it
The note says "whichever merges second takes a one-line rebase that keeps both." Measured with git merge-tree and then by performing the merge: three conflicting files and four resolutions. Detail is in my #2364 verdict so it is written once; the short version is that one of the four is a rename collision and one of them does not parse if you resolve it the obvious way. They are compatible — with all four resolved, the production files typecheck clean and the combined suites are 86 / 86.
Written by sprint-review, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf
GATE: PASS
sprint-review on #2363 (PASS at b4d69c0): dropping either left the suite green. "human merge press" with no members is accepted only by HUMAN_PRESS_RE, and without the empty-names guard 'awaiting .' with no members became an ask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG
…d, not "needs you" A thread reply is the only reply (Sam, 2026-09-06), so an agent's mention that nobody answered in its thread stayed in the needs-you queue for good. Seats post status and gate notes that name a person; the 2026-10-20 audit found 640 such rows older than a week (535 on one operator account, 104 on Sam's, whose queue held 119, the oldest from 09-11). - AttentionItem.resolvedBy gains `missed`: resolved, never counted as acted on. The row stays in the store and its message stays in the pod. - sweepMissedAgentMentions resolves an open mention as `missed` once its source is AGENT_MENTION_MISSED_DAYS (7) old and its author is an agent, re-checking `status: open` at write time so a reply or acknowledgement that lands during the pass keeps its own resolution. A person's mention, a decision, an approval and a handoff never expire this way, and a row with no recorded actor is left alone. - schedulerService runs it hourly and logs every pass. Reply semantics are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG
lilyshen0722
left a comment
There was a problem hiding this comment.
Re-gate — #2363 at b24fe55aab84080679121139ff58ee6160ff9360
Baseline: my PASS at b4d69c00, plus my PASS on #2364 at 3bc4f0a0 — which is where #2364 was closed, so nothing moved on it after the gate. origin/main = cd37b32e0f9288ee24922362d3b86d17c5590368, still the merge-base. CI at the exact head: 14 pass / 2 skipping; Test & Coverage and Service Tests (Tier 1 — real DBs) SUCCESS in run 39488735971, the three Analyze jobs in 39488728114, both head_sha read back as the head above. CodeQL is SUCCESS here, where it was NEUTRAL at b4d69c00 — so the 13/3-vs-14/2 difference I explained last time is gone on its own.
No defects. And my item 1 was wrong — the fault was in my mutation, not in your tests.
Correcting myself first
You are right, and the property was pinned the whole time. My mutation added a resolveActioned call inside the try and left the original call in place outside it, so the mutant had two call sites and the property could not break. I reported that green as absent coverage; it was a non-result.
Done as a true move — delete the outside call, insert it as the last statement of the try, one call site total:
calls to resolveActioned now: 1
✕ resolves the nudge row even when the confirmation post fails: the person still did it
Tests: 1 failed, 18 passed, 19 total
That arm is unchanged since b4d69c00 (stalledConnectService.test.js is not in the diff), so it was already holding the property when I said it wasn't. And our placements were the same position, not different ones: after extractMessageId, last inside the try. The only difference was that mine didn't remove the original.
Items 2 and 3 are closed, and the arms say what they hold
| mutation | site | result |
|---|---|---|
drop HUMAN_PRESS_RE |
attentionItemService.ts:425 |
1 failed — "human (merge) press" is an ask on its own, with no lead word and no members (only HUMAN_PRESS_RE accepts it) |
| drop the empty-names guard | :427 |
1 failed — with no human members left, a bare lead word is not an ask (the empty-names guard) |
The third case in the second arm, isHandoffRequest('awaiting .', ['', ' ']), covers a path I had not probed: names that survive to the array but not past trim() + filter(Boolean). Good addition.
The fold-in is faithful, checked by line rather than by eye
I took every line #2364 added against main and asked which are absent at this head. Exactly five, and they are precisely the four resolutions:
backend/models/AttentionItem.ts 3 (the comment line, the TS union, the Mongoose enum)
backend/services/attentionItemService.ts 2 (export default, module.exports)
Everything else landed verbatim — sweepMissedAgentMentions' whole body, AGENT_MENTION_MISSED_DAYS, the 17 * * * * job and its registration in schedulerService.ts, and both test files. #2364's second commit (the Growth-allowlist comment) was indeed already on this branch; it is at AttentionItem.ts:38.
The resolved forms are right, not merely different:
resolvedBy?: 'replied' | 'acknowledged' | 'actioned' | 'missed';
resolvedBy: { type: String, enum: ['replied', 'acknowledged', 'actioned', 'missed'] },
Both export lines carry both sides' names, with AGENT_MENTION_MISSED_DAYS in module.exports only; all six of resolveActioned, sweepMissedAgentMentions, AGENT_MENTION_MISSED_DAYS, TASK_HANDOFF_CANDIDATE_RE, isHandoffRequest, handoffRecipients are both defined and exported; and TASK_HANDOFF_RE occurs 0 times, so nothing exports a missing identifier. That was the resolution I called ambiguous, and it went the right way.
Twelve mutations, every one discriminates
The two new arms above, plus #2364's ten re-run here to confirm the fold-in did not weaken them. Needle asserted at one occurrence, site printed, source restored and re-verified after every run. BASE 5 suites / 88 tests.
| mutation | result |
|---|---|
drop kind: 'mention' from the missed find |
1 failed |
drop status: 'open' from the missed write filter |
1 failed |
| treat every actor as an agent | 1 failed |
| invert the bot filter | 1 failed |
drop the createdAt fallback branch |
1 failed |
| window 7 → 8 days | 2 failed |
apply default true → false |
1 failed |
resolvedBy: 'missed' → 'acknowledged' |
1 failed |
resolveActioned writes 'acknowledged' |
1 failed |
| drop the post-lookup handoff re-check | 2 failed |
tsc reports 50 errors on the tree, all in cleanup-test-data.ts and its neighbours and none in any changed file.
Two notes on numbers
I could not reproduce 85. jest attentionItem gives 4 suites / 69 tests; adding stalledConnectService gives 5 suites / 88. Nothing wrong with the work — a count is only comparable with the selection beside it, so here are mine with their patterns.
And one on myself: a mid-run measurement I took while my own battery was rewriting attentionItemService.ts came back 1 failed, 68 passed. That was my mutation, not a real failure — re-measured on the restored tree it is 69 / 69. I nearly reported it, which is the same shape as the item-1 error above: both came from not checking what my own instrument had done to the tree.
Written by sprint-review, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf
GATE: PASS
Why
A read-only audit of the needs-you queue (7 days to 10-20) found that most of it asked nothing of the person it paged. This PR fixes the three rules that produced cards with no real ask. Agent mentions are left alone: the separate 7-day "missed" rule handles those.
TASK_HANDOFF_REcontained one person's name and matched "Sam's ruling, 10-19: …", which reports a ruling that was already made. A single note opening with that phrase, written on ten board rows, created 40 cards across a pod's four humans: all 40 of that week's handoffs, none acted on. (Those 40 rows have since been removed by_id, after a backup.)blockedbecame a card for all of them.commonly agent run …. Its row cleared only on a thread reply, so even when they did start the agent, the row stayed open.What
isHandoffRequest(text, memberUsernames). A handoff has to ask someone:No name is hard-coded. A report of a ruling asks nothing.
handoffRecipients(members, text, createdBy)decides who gets a blocked or handoff card:TASK_HANDOFF_CANDIDATE_REis a cheap gate, so ordinary progress text costs no member lookup. It's also the backfill's Mongo prefilter, and every text the matcher accepts passes it.stalledConnectService.resolveConnected. When the agent connects,resolve('message', nudgeMessageId)closes the installer's row as an action taken. That happens whether or not the confirmation post goes out.Tests
attentionItemService.test.jsandstalledConnectService.test.js(68 tests):Mutations, each failing at least one arm:
All 19 related suites pass, 322 tests. Lint shows 0 errors and
tscshows no type errors in the changed files.For Growth: a new acted-on value
This PR adds
resolvedBy: 'actioned', meaning the recipient did what the row asked, outside its thread. It's written only byresolveActioned, which only the stalled-connect resolution calls. It closes the installer's nudge row withstatus: 'resolved'andresolvedBy: 'actioned'once their agent first connects. Growth's NEEDSYOU metric countsactionedas acted on (added to its allowlist 10-20). Plainresolve()still writes noresolvedBy, so approval, decision and task resolution are unchanged.Since that change, tests cover the service write (exact
$set), the nudge usingresolveActionedboth when the confirmation post succeeds and when it fails, the schema acceptingactionedand refusing unknown values, andresolve()writing noresolvedBy. 75 tests pass. Three more mutations each fail a test: not writingactioned, the nudge using plainresolve(), and the enum withoutactioned.Merge order with #2364: both add a value to the same
resolvedByenum line, so whichever merges second takes a one-line rebase that keeps both values.Also in this PR: agent mentions nobody answers in 7 days become
missed(was #2364)#2364 is folded in here. Both PRs changed the same
resolvedByenum, its comment, the service's export lines and the same test file, and sprint-review measured the merge between them at four resolutions, one of which doesn't parse if resolved naively. One PR and one gate avoid doing that rebase in the middle of deploy 2. #2364's commit is cherry-picked as is (b24fe55a), with those four conflicts resolved. A separate change it carried, a comment pointing at Growth's allowlist, is already on this branch.What
AttentionItem.resolvedBygainsmissed. It means resolved, but never counted as acted on. The row stays in the store, and its message stays in the pod.sweepMissedAgentMentions({ now, apply })resolves an openmentionasmissedwhen both hold:AGENT_MENTION_MISSED_DAYS(7) days old:sourceCreatedAt, falling back tocreatedAt;actorUserIdresolves to anisBotuser.The write re-checks
status: 'open', so a reply or acknowledgement that lands during a pass keeps its own resolution.Never expired: a person's mention, a decision, an approval or a handoff. A row with no recorded actor is left alone too.
schedulerServiceruns the sweep hourly at :17 and logs every pass.When this sweep goes live, Sam's queue drops from 119 to about 15, by rule, with nothing deleted.
Tests
attentionItemService.test.js:missedand a person's doesn't;createdAtfallback;models/AttentionItem.resolvedBy.test.js: the real schema acceptsreplied,acknowledgedandmissed, and refuses anything else.Mutations. Each fails 1 test:
missedacknowledgedinsteadstatus: 'open'guard at write34 related suites pass (445 tests). Lint shows 0 errors and
tscis clean on the changed files.For Growth:
missedis correctly NOT acted on.actioned, above, IS acted on. Growth's allowlist already reflects both.Merge
Draft. It ships in Wednesday's second deploy (evening, after deploy 1 has soaked).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG