Skip to content

feat(voice): the session-end task files voice-agent problems as pending questions; its result reaches the owner's DM, not a later call - #5429

Open
sonichi wants to merge 1 commit into
mainfrom
feat/voice-session-end-pending-questions
Open

sonichi wants to merge 1 commit into
mainfrom
feat/voice-session-end-pending-questions

Conversation

@sonichi

@sonichi sonichi commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Follow-up to #5334 (merged). Approved by Chi.

What changes

  1. Prompt (sessionEndTask, src/voice-session-end.ts): one added instruction, inserted before the [no-send] line. Nothing else in the prompt changes.
 		`Keep any durable fact from the call (a preference, a commitment, a follow-up) in memory. ` +
+		`If the call shows the voice agent handled something badly (a misunderstanding, a dropped request, something slow or wrong), ` +
+		`or raises a decision only the user can make, record it with python3 scripts/ask-owner.py "<question>" --context "<why / options>", not as a message; ` +
+		`an improvement with no decision attached is a short note in your reply. ` +
 		`If the call leaves nothing to do, answer with [no-send].`;

ask-owner.py already files the row in the owner's Pending questions database and notifies the DM.

  1. Routing (src/task-bridge.ts): a session-end result now reaches the owner's DM and is no longer spoken into a later call.

Routing finding (read from startResultWatcher, before this PR)

The session-end task is written with buildVoiceTaskHeader(..., null): source: voice, no origin, id task-<ts>, not in _pendingTasks.

  • No client attached (the usual case after the 60 s hang-up grace): _isVoiceTask → _forwardOfflineThenArchive → forwardOfflineVoiceResult with no origin. This publishes an untagged proactive-result-<id>-<ts>.txt, which the bridges deliver to the owner's DM. If no bridge claims it, the drain speaks it on reconnect. This already reaches the DM.
  • Client attached when the result lands (the user is on another call): the fallthrough calls onResult. The follow-up report is spoken into that unrelated call, then archived, and never reaches the DM.
  • Shutdown: the task is written during drainPostSession, and its result lands after the voice agent has exited. On the next start with no client, it takes the first path above.

Fix

  • A session-end task's id is task-voice-end-<ts> (SESSION_END_TASK_PREFIX). It matches local_task_protocol.TASK_ID_RE and OFFLINE_COPY_RE. The prefix survives a restart, which the in-memory maps do not.
  • One line in the result watcher: while a client is connected, a session-end result is held (continue). Once no client is attached, it takes the existing offline path to the owner's DM.
  • No new delivery code: the result is published through the existing forwardOfflineVoiceResult → proactive-result-* path, which the bridges and outbox deliver. Skip markers ([no-send]) are handled earlier in the loop and are unchanged.

Trade-off: if the user is still on a later call, the DM arrives when that call ends, not immediately. Installs with no bridge that claims untagged proactive files (voice-only, Telegram-only) keep today's fallback: the copy is spoken on the next reconnect, with the missed-result note.

Out of scope: in split-host relay mode, startRelayResultWatcher only consumes _pendingTasks results. That was already true before this PR and is unchanged.

Evidence

New assertions: the prompt clause and the id prefix in tests/voice-session-end.test.ts, plus a new file, tests/voice-session-end-delivery.test.ts. The new file is a real startResultWatcher run: a control work result is spoken while connected, the session-end result is held while connected, then forwarded as an untagged owner-DM copy once disconnected, and never spoken.

Parent 4e22807db src, new tests (git stash push src/; npx tsx --test --test-force-exit tests/voice-session-end.test.ts tests/voice-session-end-delivery.test.ts):

  ✖ held while a later call is live, then forwarded to the owner DM once no client is attached (7054.552041ms)
  ✖ says when the session ran and how it ended, asks the core to do its follow-ups, and carries its spoken lines (2.653792ms)
ℹ pass 11
ℹ fail 2
  AssertionError [ERR_ASSERTION]: the session-end result is not spoken into the live call
  AssertionError [ERR_ASSERTION]: The input did not match the regular expression /If the call shows the voice agent handled something badly .../

HEAD:

ℹ tests 13
ℹ suites 4
ℹ pass 13
ℹ fail 0

Regression check at HEAD:

  • npx tsx --test --test-force-exit tests/task-bridge-*.test.ts tests/voice-offline-notification-dated.test.ts tests/ag2space-voice-room.test.ts: tests 210, pass 210, fail 0
  • Every tests/*.py that reads task-bridge.ts (injection-guard-sweep, dm-only-strip, interaction-type-header, local-task-protocol, …): PASS.
  • git diff --cached | bash scripts/review-checks.sh: hardcoded-paths and root-artifacts are clean.

Live round trip: not run. This touches the voice result drain. If reviewers want it, a witness is: hang up, start a second call while the core works the session-end task, and confirm the report arrives in the DM after that call ends.

🤖 Generated with Claude Code

…g questions; its result reaches the owner's DM, not a later call

- sessionEndTask asks the core to record a voice-agent handling problem, or a
  decision only the user can make, with scripts/ask-owner.py rather than a raw
  message; an improvement with no decision is a short note in the reply.
- A session-end task's id is task-voice-end-<ts>. While a client is connected
  the result watcher holds its result instead of speaking it into the later
  call; with no client it takes the existing offline path to the owner's DM
  (proactive-result-*, delivered by the bridges / outbox).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not ready to approve yet: there is no approving review on this PR. Nothing I checked blocks it.

What I checked at 874a0e3:

  • Ran tsx --test tests/voice-session-end.test.ts tests/voice-session-end-delivery.test.ts: 13 pass, 0 fail. With src/task-bridge.ts and src/voice-session-end.ts put back to the parent 4e22807, the same tests give 11 pass, 2 fail (the hold test and the prompt clause). That matches the PR body.
  • The new delivery test runs the real startResultWatcher. It has a control (an ordinary voice result is still spoken). It does not grep source.
  • The hold, src/task-bridge.ts:1763, comes after the skip-marker and Signal branches. So [no-send] still archives silently. With no client, the result takes the existing _forwardOfflineThenArchive path (:1769-1771). No new delivery code.
  • The new id task-voice-end-<ts> matches TASK_ID_RE (src/local_task_protocol.py:237) and OFFLINE_COPY_RE (src/task-bridge.ts:576). I grepped src/skills/scripts for a task-\d id pattern. The only hit is src/task_envelope_census.py:32, which uses search, so it still finds the timestamp.
  • Hardcoded paths: none in added lines. The test sets SUTANDO_WORKSPACE to a temp dir. That is a test fixture, as in 25 other test files, not a live default.
  • CI is green. license/cla is success (1 context). There are no review threads.

Still missing:

  • An approval from someone who knows the voice area (voice is live-test-required).
  • A live round trip. This changes the voice result drain, and the PR says it was not run. Per the current policy that gap is named, not blocking. The witness in the PR body would do: hang up, start a second call while the core works the session-end task, and check the report reaches the DM after that call ends.

Not verified: the live round trip; that the prompt change gives good ask-owner.py rows in practice; the relay-mode watcher (out of scope, unchanged).

— Sutando (susans-MacBook-Pro)

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment at 874a0e37325633df6d5f7bdbae9d4338ddd2311d. The code looks correct. I'm not approving because this changes the voice result drain, a live delivery path, and only harness proof is attached (REVIEW.md lesson 15). One live round trip would make it approvable from my side.

Verified

  • tsx --test tests/voice-session-end.test.ts tests/voice-session-end-delivery.test.ts at head: tests 13, pass 13, fail 0.
  • With src/task-bridge.ts and src/voice-session-end.ts reverted to the parent, the same run shows 2 AssertionErrors (the hold and the prompt clause). So the new tests are able to fail.
  • I read the activated path in startResultWatcher (src/task-bridge.ts, ~1690-1800). The hold (:1763) sits after the voice-*, skip-marker and Signal branches, so [no-send]/[REPLIED] still archive silently while a call is live. With no client, the result reaches _forwardOfflineThenArchive via _isVoiceTask, which keys on the source: voice header, not the id shape, so the new prefix does not change that.
  • Other id parsers: local_task_protocol.TASK_ID_RE accepts task-voice-end-<ts>. task_envelope_census._ID_MS (task-(\d{13})) no longer matches the new ids, but it falls back to the timestamp: header that buildVoiceTaskHeader writes, so the census still dates them correctly.
  • Hardcoded-path scan: clean. CI is green.

What would make it ready

  • The witness the body already proposes: hang up, start a second call while the core works the session-end task, and confirm the report is not spoken and reaches the DM after the second call ends.

Nit (non-blocking)

  • A session-end result is held for as long as any client stays connected, with no upper bound. A user who keeps a long session open gets the report late. That is the stated trade-off, so I'm only noting it.

— sutando-qingyun-air

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not approvable yet: there is still no approving review. Head 874a0e37 is unchanged since my review on 2026-10-13, and nothing I checked blocks it.

Reviewed head 874a0e37325633df6d5f7bdbae9d4338ddd2311d.

Re-checked at this head

  • The diff is the same two src/ changes: the task-voice-end- id prefix in submitVoiceSessionEndTask, and the one-line hold in startResultWatcher (src/task-bridge.ts ~L1762) that sits after the skip-marker and Signal branches, so [no-send]/[REPLIED] still archive while a call is live.
  • tests/voice-session-end-delivery.test.ts drives the real startResultWatcher with a control (an ordinary voice result is still spoken) and checks the untagged owner-DM copy appears only after disconnect. It does not read source.
  • CI green, license/cla success (1 context), mergeable: MERGEABLE, no review threads.

Missing

  • An approval from someone who knows the voice area. Voice is live-test-required, and the PR body's "Approved by Chi" refers to #5334, not this PR.

Named, not blocking: the live round trip is still owed. The witness in the body fits: hang up, start a second call while the core works the session-end task, and confirm the report is not spoken into that call and reaches the DM once it ends.

— Sutando (susans-MacBook-Pro)

This branch has not been deployed

No deployments
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.

3 participants