Skip to content

fix(attention): needs-you means needs you (handoffs that ask, actioned nudges, missed agent mentions) - #2363

Merged
lilyshen0722 merged 4 commits into
mainfrom
fix/needs-you-requests-only
Oct 21, 2026
Merged

lilyshen0722 merged 4 commits into
mainfrom
fix/needs-you-requests-only

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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.

  1. Handoffs matched reports, by a hard-coded name. TASK_HANDOFF_RE contained 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.)
  2. A blocked card paged every human in the pod. In a shared work pod, every status flip to blocked became a card for all of them.
  3. The stalled-connect nudge could never be acted on. It asks the installer to run 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:

    • "awaiting", "waiting on/for", "ready for" or "needs", followed by "the human" or by a username of one of this pod's human members;
    • or "human (merge) press".

    No name is hard-coded. A report of a ruling asks nothing.

  • handoffRecipients(members, text, createdBy) decides who gets a blocked or handoff card:

    • the members the update names (by @handle or username);
    • otherwise the pod's creator;
    • everyone, only if the creator has left the pod.
  • TASK_HANDOFF_CANDIDATE_RE is 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.js and stalledConnectService.test.js (68 tests):

  • the real 10-19 note text opens nothing, and it reaches the matcher itself, not just the gate;
  • an ask naming a member goes only to that member;
  • "the human" goes to the creator;
  • a name that isn't a pod member opens nothing;
  • progress text does no member lookup;
  • the pure helpers, including the creator-has-left fallback;
  • the nudge row resolves on connect, including when the confirmation post fails, and doesn't resolve while the agent has never connected.

Mutations, each failing at least one arm:

mutation result
count "'s ruling/call/decision" as an ask 3 failed
page every member 4 failed
accept any name, not just members 2 failed
drop the candidate gate 1 failed
drop the nudge resolve 2 failed

All 19 related suites pass, 322 tests. Lint shows 0 errors and tsc shows 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 by resolveActioned, which only the stalled-connect resolution calls. It closes the installer's nudge row with status: 'resolved' and resolvedBy: 'actioned' once their agent first connects. Growth's NEEDSYOU metric counts actioned as acted on (added to its allowlist 10-20). Plain resolve() still writes no resolvedBy, so approval, decision and task resolution are unchanged.

Since that change, tests cover the service write (exact $set), the nudge using resolveActioned both when the confirmation post succeeds and when it fails, the schema accepting actioned and refusing unknown values, and resolve() writing no resolvedBy. 75 tests pass. Three more mutations each fail a test: not writing actioned, the nudge using plain resolve(), and the enum without actioned.

Merge order with #2364: both add a value to the same resolvedBy enum 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 resolvedBy enum, 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.resolvedBy gains missed. 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 open mention as missed when both hold:

    • its source is AGENT_MENTION_MISSED_DAYS (7) days old: sourceCreatedAt, falling back to createdAt;
    • its author is an agent: actorUserId resolves to an isBot user.

    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.

  • schedulerService runs 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:
    • an agent's week-old mention becomes missed and a person's doesn't;
    • the exact query: open, has an actor, older than the 7-day cutoff by source time with the createdAt fallback;
    • nothing old enough means no actor lookup and no write;
    • a dry run writes nothing;
    • the window is 7.
  • models/AttentionItem.resolvedBy.test.js: the real schema accepts replied, acknowledged and missed, and refuses anything else.

Mutations. Each fails 1 test:

mutation result
enum without missed 1 failed
no bot filter 1 failed
stamp acknowledged instead 1 failed
a 1-day window 1 failed
drop the status: 'open' guard at write 1 failed

34 related suites pass (445 tests). Lint shows 0 errors and tsc is clean on the changed files.

For Growth: missed is 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

lilyshen0722 and others added 2 commits October 20, 2026 12:21
…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 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

lilyshen0722 and others added 2 commits October 20, 2026 13:01
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 lilyshen0722 changed the title fix(attention): a handoff card is an ask, and it goes to the person asked fix(attention): needs-you means needs you (handoffs that ask, actioned nudges, missed agent mentions) Oct 20, 2026

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@lilyshen0722
lilyshen0722 marked this pull request as ready for review October 21, 2026 17:56
@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 1fb351a Oct 21, 2026
27 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant