🐛 Resolve the in-process singleton when avenor_ask omits supervisor_id

[sdougbrown]

Why

avenor_ask failed on every call that omitted supervisor_id. The tool schema never exposed that parameter, so askTool always passed supervisorId: undefined into getSupervisorClient, which fed it to validateSupervisorSocketPath. path.dirname(undefined) threw The "path" argument must be of type string. Received undefined — the agent saw a tool error regardless of what arguments it supplied.

Changes

• getSupervisorClient now accepts supervisorId: string | undefined. An omitted id resolves the in-process singleton through Supervisor.get() — the same fallback spawn.ts and result.ts already use — instead of validating an undefined socket path. This also covers broker-reply and cancel, which pass optional ids through the same helper.
• avenor_ask exposes the optional supervisor_id parameter, matching avenor_reply, so the tool can also target an external supervisor.
• Add a regression test that calls the real exported askTool with no supervisorId against a patched Supervisor.get singleton, and asserts the singleton client is not closed in the finally block.

Validation

bun test → all packages 0 fail (ask.test.ts: 6 pass, including the new regression test). Verified the regression test fails against the pre-fix source (git stash of get-supervisor-client.ts) and passes with the fix. bun run build in packages/core → tsdown clean.

Notes

When no supervisor runs in-process and supervisor_id is omitted, Supervisor.get() starts a lazy singleton rather than erroring, consistent with result.ts. A caller that needs a different supervisor still passes an explicit socket path.

Review triage: two parallel reviewers (general + tests) ran on the diff. All Must/Should findings were dropped after verification — the events.ts "incomplete type annotation" claim is refuted by the code (the inline annotation at events.ts:126-127 already declares supervisorId?: string), and the remaining findings flagged pre-existing code outside the diff or patterns that match existing repo conventions (as any on supervisor mocks, afterAll restore, shared description strings). No open findings.

[sdougbrown]

👾 AI Agent

Added two commits fixing the follow-up failures (root cause found and verified end-to-end against the real binary):

Root cause: pi's agent_end payload serialises the assistant message's stop reason as camelCase stopReason with OpenAI-style values ("stop" for a successful turn). The Go translator (internal/runtime/pi/events.go, translateAgentEnd) read only the snake_case alias, so every session.end from a pi run carried no stop_reason. WaitForSession then returned the generic exit code 1, and runChild classified every successful pi turn as a retryable failure:

• Runs terminated right after the agent answered, with a FAILED / STOP_REASON=exit_1 sentinel — the watch-dialog follow-up "immediately ends the subagent's run".
• While the runtime idled awaiting prompts, the session attempt stayed owned, so avenor_follow_up's spawn-resume was rejected with session ID "..." is already owned by another active provider attempt.
• No DONE sentinel meant the sentinel-based resume path found nothing.

Fix (d4f05fe): read stopReason first (keeping the snake_case alias) and normalise pi's raw values onto avenor's canonical stop reasons (stop→end_turn, length→max_tokens, aborted→cancelled, end_of_turn→end_turn). Verified against a locally built binary: runs now complete with a DONE / STOP_REASON=end_turn sentinel, and avenor_follow_up spawn-resume succeeds (session id preserved).

Tests (14e5603): new tests pin the real camelCase payload shape and the mapping table (including unmapped-value passthrough, in subtests). The e2e follow-up test's races are fixed: sentinel waits are gated on AVENOR_REAL_INTEGRATION=1 (the fake fixture never writes sentinels) and both follow-ups wait for run completion, which the session-ownership guard requires.

Validation: go test ./... clean; bun run test all packages 0 fail; e2e bun test src/supervisor.test.ts with the fixed binary: 20 pass, 0 fail (needs the /tmp/original-repo fixture dir to exist — pre-existing env dependency, noted in Validation).