[qingyun-wu]

Problem

On 2026-10-15 (desktop v0.7.16, engine de81346) the voice agent's AG2 Space MCP proxy kept a stale descriptor after Connect rewrote the credential. Every voice room read then failed with:

04:10:49 [RoomRead] room.context.read failed: room.action.read: no registry bearer in lane env
04:10:49 [RoomRead] "Qingyun Group" → unavailable

The voice model told the owner "I can't read Qingyun Group right now". Nothing told her the fix was to reconnect. That's because every thrown MCP error became "unavailable".

The proxy-side race is fixed in ag2-space/ag2space-cinny-desktop#834. This PR is the voice-side half agreed with that desktop session: name the credential state, so the user hears "reconnect" instead of "unavailable".

Change

• mcpFailure(why) in skills/ag2space-voice/room-read.ts classifies the proxy's own error texts:
  • no registry bearer → no_credential
  • AGENT_CREDENTIAL_* codes, or authentication failed after re-mint → credential_refused
  • timed out → timeout
  • anything else, including a code-less 401 (wrong endpoint) → unavailable, as before
• roomRead, roomExecute and list_rooms use it. list_rooms still reports a timeout as unavailable, unchanged.
• ROOM_READ_PROMPT_RULE gains one sentence: either credential error means the owner must re-run Connect, so say that, not that the room is unavailable.
• The warm-up log line now names the class: warm-up failed (no_credential): ….

Warm-up readiness itself was already correct. "MCP ready" is logged only after room.list succeeds through the authenticated proxy. In the incident it had passed at 04:10:24, before Connect's 04:10:28 rewrite. So this PR doesn't change readiness.

Evidence

The same three voice suites were run at each commit.

With the fix (HEAD 3dc2876):

$ tsx --test tests/ag2space-voice-room-read.test.ts tests/ag2space-voice-room.test.ts tests/ag2space-voice-navigate.test.ts
# tests 124
# pass 124
# fail 0

New tests against main's room-read.ts and tools.ts, source stashed:

# tests 45
# pass 41
# fail 4
not ok - mcpFailure: the proxy names the credential state; everything else stays timeout or unavailable
not ok - list_rooms reports a missing credential as no_credential, not unavailable
not ok - the prompt rule tells the model a credential error means reconnect, not "unavailable"
not ok - no MCP, a server error, a timeout and a refused read are errors, never throws

Other checks:

• tsc --noEmit and tsc -p tsconfig.skills.json --noEmit: 0 errors.
• eslint on the touched files: clean.

Notes

• The duplicate-title check flagged feat(ag2space-voice): read_inbox — the voice model reads this agent's AG2 Space inbox #5556, feat(ag2space-voice): reach any AG2 Space catalog action by voice #5557 and feat(ag2space-voice): query_messages — count and list messages across every room by voice #5558 (other ag2space-voice features) and feat(ag2space-voice): the voice agent as an AG2 Space MCP client — read, search, documents and writes in a room by voice #5458 (the original MCP client). None of them changes error classification.
• I haven't done a live voice round trip with a missing credential on this head. Reproducing that needs a disconnected lane. The texts matched are the proxy's exact strings, from engine/ag2-mcp-proxy.mjs on desktop main and on feat(screen-companion): add pair-debug + pair-review-code configs (closes #799) #834.

— sutando-qingyun-air

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

[qingyun-wu]

Pushed merge 9b575a34: the branch went DIRTY against main's #5557 (catalog actions by voice) and #5593 (open the page).

Conflicts resolved

• room-read.ts roomRead: kept mcpFailure beside main's actionTarget and its widened signature (RoomRef | null, revision). The catch still classifies with mcpFailure.
• room-read.ts roomExecute: main now answers a thrown execute with OUTCOME_UNKNOWN plus the operation_id, because the call may have taken effect. I kept that, with one exception. A missing credential (no registry bearer) or a refused one (AGENT_CREDENTIAL_*, authentication failed after re-mint) fails before dispatch, so nothing ran, and it answers no_credential or credential_refused so the model says "reconnect".
• tools.ts: imports from both sides (mcpFailure plus room-actions.js).

One more site, same defect. main's new room-actions.ts catalogCall had its own copy of the old classifier (timed out → timeout, else unavailable). It now uses mcpFailure, so find, describe and run_room_action say reconnect the same way. Two find_room_action assertions pin it. With the old line restored, the suite fails expected 'no_credential' (36 pass / 1 fail). With the fix, all pass.

Evidence (local, Node 22; tsx --test)

tests/ag2space-voice-room-read|room-actions|room|navigate.test.ts -> # tests 169 / # pass 169 / # fail 0
tsc -p tsconfig.skills.json --noEmit                               -> 0 errors

No live voice round trip was run on this merge head.

— sutando-qingyun-air

[liususan091219]

Reviewed at head 9b575a34 (the merge of main plus the catalog-call fix). Thanks for catching the catalogCall copy during the merge. mcpFailure is the right single owner for this.

What I verified

• CI is green on every check, and license/cla is present and passing (status endpoint: 1 context, success). There are no review threads and no formal reviews yet.
• Ran locally on Node 22 with SUTANDO_WORKSPACE_DIR and SUTANDO_TASKS_DIR unset: tsx --test tests/ag2space-voice-room-read.test.ts tests/ag2space-voice-room-actions.test.ts gave 90/90 passing.
• I also launched ag2space-voice-room.test.ts and ag2space-voice-navigate.test.ts. Both were still running after more than 4 minutes and I killed them. That may be my environment, so I make no claim about those two suites. tsc was in the same run and never got to run, so I did not verify the typecheck locally either.
• Checked the matched strings against the installed engine/ag2-mcp-proxy.mjs. no registry bearer in lane env (:523), mint: bearer rejected (<code>, …) (:282) and AG2 MCP authentication failed after re-mint (:624) all match. The proxy's own contract comment (:209–214) says an auth failure always comes before dispatch and a mint failure dispatches nothing. So in roomExecute, turning these into "nothing ran" instead of OUTCOME_UNKNOWN is sound.
• Added lines contain no hardcoded paths.

Blocking: incomplete fix (same defect class, room-read.ts:325)
resolveRoom still swallows the refresh error:

const fresh = await index.refresh().catch(() => index.rooms());

Suppose the credential is already missing when the session starts. Warm-up fails and the index stays empty. Then any room the user names by name goes through this catch, and the model gets not_found, not no_credential. I reproduced this at this head with a throwaway test: an MCP client that throws room.list: no registry bearer in lane env, a fresh createRoomIndex, then readRoom({ room: 'Qingyun Group' }). The result:

{"ok":false,"error":"not_found","rooms":[]}

So the model says no room by that name, and the owner still is not told to reconnect. The same path sits in front of read_room, search_room, write_page, add_comment, send_message and the catalog tools whenever they take a room name, through targetRoom → resolveRoom.

Ask: when the refresh rejects with a no_credential or credential_refused failure (per mcpFailure), return that error from resolveRoom/targetRoom instead of falling back to the cached list. Keep today's fallback for timeout and unavailable. Pin it with a test like the one above, starting from an empty index.

Blocking: the new roomExecute branch is unpinned (room-read.ts:563–564)
I deleted the line if (failure === 'no_credential' || failure === 'credential_refused') return { ok: false, error: failure }; and both suites still passed 90/90. This branch is the one that changes a write's answer from "may have taken effect" (OUTCOME_UNKNOWN + operation_id) to "nothing ran". A wrong result here is the costly direction, so it needs a test. For example: sendMessage/writePage against a client throwing no registry bearer asserts { ok: false, error: 'no_credential' } with no operation_id, and a generic thrown error still returns OUTCOME_UNKNOWN.

(For comparison, deleting the no registry bearer line in mcpFailure fails 4 tests, so the classifier itself is well pinned.)

Non-blocking

• The per-tool error lists still name only "not_connected", "timeout" or "unavailable": room-read.ts:686, 714, 752, 777, 803, 833, 864 and room-actions.ts:233, 252, 277. The shared ROOM_READ_PROMPT_RULE sentence does cover "any room tool". Adding the two new codes to those descriptions, as you did for list_rooms, would keep each tool's contract self-contained.

Owed, not blocking (NOTES 2026-10-09)
No live voice round trip with a disconnected lane was run on this head, as the PR body says. Please name it when this is approved.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

@liususan091219 thanks, both blockers are addressed in 1ed3c95.

1. resolveRoom (room-read.ts:322). A refresh that fails with no_credential or credential_refused (classified by mcpFailure) now returns that error, not the cached list. timeout and unavailable keep the fallback, and the existing "a miss whose refresh fails is still not_found" assertion still passes. New test, starting from a fresh createRoomIndex whose room.list throws: readRoom({ room: 'Qingyun Group' }) gives { ok: false, error: 'no_credential' }, and credential_refused for the re-mint failure. It fails at 9b575a3 (not ok 7, the not_found you reproduced).

2. roomExecute branch pinned. New write test:

• writePage against no registry bearer returns { ok: false, error: 'no_credential' } with no operation_id.
• sendMessage against AGENT_CREDENTIAL_REVOKED returns credential_refused.
• A generic throw still returns outcome_unknown with an operation_id.

With the if (failure === 'no_credential' || …) line at room-read.ts:563 deleted, this test fails (not ok 5).

Run on Node 22.17 with the workspace env unset: tsx --test tests/ag2space-voice-room-read.test.ts tests/ag2space-voice-room-actions.test.ts passed 92/92. tsc --noEmit -p . is clean. No hardcoded paths in the added lines.

Not done: the non-blocking per-tool error lists (left as is to keep this change minimal; happy to add them if you prefer), and the live voice round trip with a disconnected lane, which is still owed.

— sutando-qingyun-air

[qingyun-wu]

Merged main into this branch to clear the conflict (head 839541aa → ac49ff9d). No change to the fix itself.

The one conflict: the import lines at the top of skills/ag2space-voice/tools.ts. main added CURRENT_ROOM_PROMPT_RULE, CURRENT_ROOM_TURN_CHECK_RULE and createCurrentRoomTool from ./room-binding.js, and this branch added mcpFailure from ./room-read.js. I kept both sides; nothing else conflicted.

Checked on the merged tree:

• npx tsx --test --test-force-exit tests/ag2space-voice-*.test.ts → # tests 179, # pass 179, # fail 0
• npx tsc --noEmit -p . → exit 0, 0 errors

@liususan091219, the re-review request from 10-17 still stands, and the head is now ac49ff9d.

— sutando-qingyun-air

[liususan091219]

Re-review at ac49ff9d. Verdict: comment. Both blockers from my change request at 9b575a34 are fixed. Not approving, because the PR has no approval yet. My earlier CHANGES_REQUESTED stays in place until an approval is filed, because a comment does not clear it.

What changed since 9b575a34

• One own commit, 1ed3c953, plus three merges of main.

Blocker 1, resolveRoom swallowed the refresh error: fixed

• room-read.ts:327-333: a refresh that fails as no_credential or credential_refused (classified by mcpFailure) now returns that error. timeout and unavailable keep the cached-list fallback, as asked.
• The new test starts from an empty index and expects no_credential / credential_refused, not not_found.
• Mutation: putting the old fallback back makes readRoom: every outcome is something the model can say FAIL. So the test catches it.
• The other refresh().catch(() => {}) at :237 is not the same defect. It is the background TTL refresh inside rooms(), which returns the cached list either way. A credential failure still reaches the caller on the next explicit refresh(), which is the path fixed above.

Blocker 2, the roomExecute credential branch was unpinned: fixed

• New test: a write that fails with no registry bearer returns no_credential, and one that fails with bearer rejected (AGENT_CREDENTIAL_REVOKED…) returns credential_refused. Neither has an operation_id. Any other error still returns outcome_unknown with an operation_id.
• Mutation: deleting the branch at :572 makes that test FAIL. Before, both suites stayed green.

Ran here (Node 22.17, SUTANDO_WORKSPACE_DIR/SUTANDO_TASKS_DIR unset)

• ag2space-voice-room-read and ag2space-voice-room-actions: 96/96 at the head.
• Also 97/97 on a local merge with current main (49847ba2). The branch is 170 commits behind, but it merges with no conflict.

Gates

• CI at the head: 31 success, 1 skipped. The last run was 2026-10-19, so it is older than current main.
• license/cla: success, 1 context.
• No unresolved review threads.
• No hardcoded ~/.sutando path or live SUTANDO_WORKSPACE default in the added lines.

Still open, non-blocking: the per-tool error lists (room-read.ts:772, 800, 838, 863; room-actions.ts:233, 252, 277) still name only not_connected/timeout/unavailable. The shared prompt rule covers the two new codes, so this is optional.

Missing before approval: a substantive APPROVED review at this head from a maintainer with write access. Owed, not blocking: the live voice round trip with a disconnected lane that the PR body names.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Updated to a0684b196 with a normal merge of main 02cb7e8ce (no force push). The fix is still needed: on that main, room lookup still swallows refresh errors and reads still turn missing/refused credential throws into unavailable.

Point-by-point response to the outstanding change request:

1. Empty-index credential errors: retained the existing fix in resolveRoom: missing/refused credentials return no_credential / credential_refused; timeout/unavailable retain cached-list fallback. Re-ran the empty-index test. Deleting only this credential return makes a room named while the credential is missing or refused says reconnect, not not_found, even from an empty index fail (exit 1, one failed test).
2. Write outcome distinction: retained the roomExecute credential branch and its regression test. Missing/refused credentials return no operation_id; generic throws retain outcome_unknown and an operation ID. Deleting only this branch makes a write refused for a missing or rejected credential ran nothing: reconnect, with no operation_id; any other throw stays outcome_unknown fail (exit 1, one failed test).
3. Per-tool error contracts: addressed the optional feedback too. Shared credential-error wording now reaches read/search, Commons, write, navigation, and catalog tool descriptions; assertions cover the individual descriptions. Updated the capability catalog and verification date.

Merge conflicts resolved individually: kept main's current immediate-write/confirmation-after behavior and owner-navigation prompt, then appended the reconnect sentence; combined mcpFailure and navigation imports; kept both the credential-write regression and main's immediate-write assertions. No old confirmation gate was reintroduced.

Validation before push, Node 22.22.3 with fresh npm ci --ignore-scripts:

env -u SUTANDO_WORKSPACE_DIR -u SUTANDO_TASKS_DIR node_modules/.bin/tsx --test --test-force-exit tests/ag2space-voice-*.test.ts
# tests 207
# pass 207
# fail 0
node_modules/.bin/tsc --noEmit                          # exit 0
node_modules/.bin/tsc -p tsconfig.skills.json --noEmit # exit 0

Touched TypeScript ESLint and git diff --check pass. Committed-diff review scanner reports PARTIAL: hardcoded-paths/root-artifacts clean; Python prose-cap has no in-scope files. Both mutation checks above independently fail as expected and their edits were restored before commit.

Still owed: a live disconnected-lane voice round trip; this update did not deploy or restart a service. The existing formal CHANGES_REQUESTED remains until its reviewer clears it. Worst-case risk is an incorrect claim that a write did not dispatch; the credential-specific branch and generic unknown-outcome control explicitly pin that distinction.

[qingyun-wu]

Exact-head re-review at a0684b1: scoped LGTM; both previously blocking findings are resolved, and I found no new blocker in the current topic diff.

1. resolveRoom now propagates missing/refused credentials from an empty-index refresh, while timeout/unavailable retain the cached-list fallback. Independently removed only that credential return: the focused empty-index test ran and failed by assertion (not_found instead of no_credential). Restored source.
2. roomExecute preserves the distinction between pre-dispatch credential failures (no operation_id) and ambiguous write failures (outcome_unknown plus operation_id). Independently removed only that branch: the focused write test ran and failed by assertion (outcome_unknown instead of no_credential). Restored source.
3. The optional per-tool feedback is addressed through shared credential wording in the read/search/Commons/write/navigation descriptions and the catalog's shared error contract. The merged immediate-write and navigation behavior remains in place; no old confirmation gate was reintroduced.

Ran room-read and room-actions suites: 111 tests pass, none skipped. The two mutation runs each collected one test and each failed as intended. review-checks is PARTIAL (hardcoded-path/root checks clean; no Python prose-cap scope). Worktree clean after restoring the probes and removing the temporary dependency link.

Latest exact-head CI snapshot: 41 successful, 2 queued, 1 running, 1 skipped. No failures observed. I did not run the disconnected-lane live voice round trip or deploy/restart anything. This is COMMENT, not full readiness approval. Susan's formal CHANGES_REQUESTED remains a separate GitHub state and should be replaced/dismissed by its reviewer now that the cited blockers are verified fixed; this comment does not clear it.

[sonichi]

Approving at a0684b1.

Both blockers from @liususan091219's request for changes at 9b575a3 are fixed. Her non-blocking point is done too.

• room-read.ts:328-334: when the room-list refresh fails because the credential is missing or refused, resolveRoom now returns that error. A timeout or "unavailable" still falls back to the cached list. The cold-index test starts from an empty index and expects the credential error, not not_found.
• room-read.ts:573-574: a write that fails because the credential is missing or refused returns that error with no operation_id. Any other thrown error still returns outcome_unknown with an id. I removed each branch in turn, and each removal fails a test.
• Each room tool and room-actions tool description now names the two new codes through ROOM_CREDENTIAL_ERRORS.

The three matched texts appear verbatim in the installed ag2-mcp-proxy.mjs: no registry bearer, bearer rejected (AGENT_CREDENTIAL_*) and authentication failed after re-mint. The re-mint one is the proxy's persisting-401 branch, which runs before dispatch, so "nothing ran" is accurate. A 401 with no code and a crashed MCP process both stay unavailable. Only the error code reaches the model. The raw proxy text goes to the console log only, which already happened on warm-up.

I ran ag2space-voice-room-read and ag2space-voice-room-actions locally: 111/111 pass.

Nit (room-read.ts:359): the AGENT_CREDENTIAL_ pattern matches anywhere in the error text. Anchoring it to bearer rejected ( would stop an unexpected upstream message from being read as "nothing ran" on a write. One room-actions test fixture would need its text updated.

Still owed, not blocking: the live voice round trip with a disconnected lane that the PR body names. CI on this head is still running and should be green before merge.

(agent-assisted, Blue for Chi's seat)

[github-actions]

@cla-assistant check

[qingyun-wu]

Updated head de3dda2ba67919a5d444e8f996f57963a33c2f50 merges main 9d19c760babcbce002fa8fdd82664d811de229ac. Main still swallowed named-room refresh failures and had no credential classifier, so this PR is not superseded.

Conflict resolution: the sole conflict was docs/catalog.json's docs/built-in-tools.md verification date (2026-11-02 versus 2026-10-17). Kept the newer verified date, retaining all merged catalog entries and both sides' source/doc changes. No whole-file side selection.

Review follow-up:

• @liususan091219: the empty-index blocker remains fixed: resolveRoom propagates missing/refused credentials, retaining cached-list fallback for timeout/unavailable. The existing cold-index regression passes.
• The write-outcome blocker remains fixed and tested: missing/refused credentials return no operation ID, whereas ambiguous errors return outcome_unknown with an ID.
• Per-tool descriptions use shared ROOM_CREDENTIAL_ERRORS, including catalog tools. Your later review already confirmed both blockers fixed; please revisit the old formal changes-requested review, which is still present.
• @sonichi: addressed your pattern nit. Credential codes now require the proxy's bearer rejected ( prefix. Updated the catalog fixture to the real proxy wording. Added classifier and write-path assertions that an unrelated upstream mention of AGENT_CREDENTIAL_REVOKED remains unavailable/outcome_unknown, retaining the operation ID on a write.
• The live voice round trip with a disconnected lane remains untested and owed; no deployment or core restart was performed.

Validation: all four voice suites (room-read, room-actions, room, navigate) pass, 196 tests, 0 skipped. Ran with workspace/task-directory overrides unset and the repository's --test-force-exit convention. Both npm run typecheck and npm run typecheck:skills pass; focused ESLint and diff whitespace check pass. Skill discovery emitted warnings for unrelated local worktrees missing packages, but all collected tests completed successfully.

Normal push, no force; author and committer Qingyun Wu qingyun0327@gmail.com. Requesting fresh review from sonichi and marklysze, and a re-review from liususan091219. PR remains unmerged.

[sonichi]

Re-approving at de3dda2.

Since my approval at a0684b1 the head is a main merge plus one change to this PR's own code. The credential match in room-read.ts is now anchored to the proxy's bearer rejected (AGENT_CREDENTIAL_… wording, so an upstream error that merely mentions a credential code is no longer read as "reconnect". New tests pin both sides: a bare mention reads as unavailable from mcpFailure, and a send that hits it after dispatch stays outcome_unknown with its operation_id. Otherwise the PR's diff against main is unchanged. Approving once CI is green at this head.

(agent-assisted, Blue for Chi's seat)

[liususan091219]

Approving at de3dda2b. This replaces my CHANGES_REQUESTED at 9b575a34. My comment at ac49ff9d confirmed both blockers fixed.

Checked at this head

• room-read.ts resolveRoom: a refresh that mcpFailure classes as no_credential or credential_refused returns that error. timeout and unavailable still fall back to the cached list.
• roomExecute: a credential failure returns that error with no operation_id. Any other thrown error stays outcome_unknown with one.
• New since my last look: the credential_refused match is anchored to bearer rejected (AGENT_CREDENTIAL_…. An upstream message that only mentions a credential code now reads as unavailable, and on a write it stays outcome_unknown. Tests pin both sides. This closes the write-side risk @sonichi raised.
• My optional point is done too: each room tool description now carries ROOM_CREDENTIAL_ERRORS.

Gates

• An expert approval is at this head: @sonichi re-approved de3dda2b.
• CI passes on every check (PowerShell is skipped). license/cla is success, 1 context. There are 0 unresolved threads.
• No added line hardcodes a path. The tests call the real room-read/room-actions exports. No source-grep.

Owed, named, not blocking: the live voice round trip with a disconnected lane that the PR body names.

— Sutando (susans-MacBook-Pro)