fix(web): catch agent-status API failure during terminal reconnect - #2
Merged
Merged
Conversation
After a server restart the reconnect timer calls ensureTerminalConnected which immediately hits the agent status API. That API call was outside the try-catch block, so a network error (server still booting) threw an uncaught exception, silently exiting the function without rescheduling — leaving the terminal stuck at "Session disconnected, reconnecting...". Fix: wrap the agent status fetch in its own try-catch. On network failure, schedule a backoff retry using the same logic as scheduleReconnect so the loop continues until the server is reachable again. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
selfcontained
added a commit
that referenced
this pull request
Apr 23, 2026
…dispatch_get_feedback (CRU-133) Live dogfood against the dev stack surfaced a real bug in the CRU-133 text: the persona prompt and the launch-response guidance both tell agents to link round-2 follow-up findings to the originals using `responds_to_feedback_id`, but the MCP contract had no such field. - `dispatch_feedback` zod schema had no `respondsToFeedbackId` input, so reviewers couldn't actually set the linkage — the instruction was inert. - `FeedbackItem` (the type surfaced by `dispatch_get_feedback`) omitted the field, so parents couldn't read it either. The DB column `responds_to_feedback_id` existed and the SELECTs already aliased it to `respondsToFeedbackId`, but neither type exported that name. - Prompt and launch-response used snake_case (`responds_to_feedback_id`) while the rest of the MCP surface is camelCase (`feedbackId`, `personaAgentId`, `mediaRef`). End-to-end fix: - Add `respondsToFeedbackId?: number` to `FeedbackInput` (both `manager.ts` and `mcp/server.ts`). - Add `respondsToFeedbackId: number | null` to `FeedbackItem`. - Add the zod parameter to `dispatch_feedback` and thread it through to `AgentManager.submitFeedback`, which now persists it via the existing DB column. - Update persona prompt and launch-response copy to camelCase `respondsToFeedbackId`, and expand the persona-side explanation so a reviewer understands *why* the link matters (parent can map round-2 follow-ups back to round-1 concerns). - New regression test: `submitFeedback` persists `respondsToFeedbackId` on a round-2 finding. Existing tests updated for the camelCase form. Found via live-dogfood review against the dev stack running this branch (feedback items #1 and #2 from agt_35cac0176460). Co-Authored-By: gpt-5.4 review agent <noreply@anthropic.com>
selfcontained
added a commit
that referenced
this pull request
Apr 23, 2026
…se, tool descriptions (#386) * feat(server): inject recheck round-trip guidance into persona prompt, launch response, and tool descriptions (CRU-133) Wires the opt-in review round-trip through the three text carriers that agents actually see: - Persona prompt: append a round-trip guidance block to assemblePersonaPrompt when the review is launched with allowRecheck. - dispatch_launch_persona response: when allowRecheck is true, return a block that tells the parent how to drive the loop (get/resolve feedback, submit resolution, poll for round 2). - Tool descriptions: rewrite dispatch_launch_persona, dispatch_resolve_feedback, dispatch_submit_resolution, and dispatch_await_recheck so each carries the constraints the server enforces and tells the agent what to do next. - Rejection messages: update complete_review and submit_resolution errors so a fresh agent can self-correct on the next turn. No backend behavior changes beyond text — state machine and poll logic remain as landed in CRU-131/CRU-132. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): tighten recheck guidance text per review feedback (CRU-133) Address two items from backend-security-review: - dispatch_launch_persona response (allowRecheck=true): replace "poll dispatch_get_feedback until the reviewer's round-2 verdict is complete" — dispatch_get_feedback only returns agent_feedback rows, not persona_review.verdict/status, so a round-2 approve-with-no-new- items would look indistinguishable from "still running." New text describes what the API actually surfaces: watch for items linked via responds_to_feedback_id, and treat "nothing new after a reasonable window" as an approval signal. - dispatch_resolve_feedback description: the "the reviewer will see it in a recheck pass" clause was unconditional, but the tool is also used by parents whose reviews were launched with allowRecheck=false (the default), where no recheck happens. Qualify the clause. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(server): cover launch-response branching and rejection recovery hints (CRU-133) Close the test-coverage gaps identified in the CRU-133 self-review: - Extract the dispatch_launch_persona response-text assembly into a pure `buildLaunchPersonaResponseText` exported from shared/mcp/server.ts. The handler now delegates to it, so the allowRecheck branching is unit-testable without spinning up the MCP server. New tests in test/mcp-launch-persona-response.test.ts cover: base-only when allowRecheck is false, full driver-loop block when true, required tool references (dispatch_get_feedback, dispatch_resolve_feedback, dispatch_submit_resolution, responds_to_feedback_id), and the "no verdict signal" regression guard for the fix in ee45e96. - Strengthen the open-items and ignored-missing-reason rejection tests in both agent-manager.test.ts and resolution-capture-integration.test.ts to assert the new recovery hint text ("Call dispatch_resolve_feedback", status names, and the reason-explanation phrase). The existing regexes only proved IDs were named. - Lock the ticket-spec copy for the third-completion rejection in agent-manager.test.ts — toBe the exact string, not just a substring, so the rejection message itself (which is guidance) can't drift. No production behavior change beyond the pure-function refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): wire respondsToFeedbackId through dispatch_feedback and dispatch_get_feedback (CRU-133) Live dogfood against the dev stack surfaced a real bug in the CRU-133 text: the persona prompt and the launch-response guidance both tell agents to link round-2 follow-up findings to the originals using `responds_to_feedback_id`, but the MCP contract had no such field. - `dispatch_feedback` zod schema had no `respondsToFeedbackId` input, so reviewers couldn't actually set the linkage — the instruction was inert. - `FeedbackItem` (the type surfaced by `dispatch_get_feedback`) omitted the field, so parents couldn't read it either. The DB column `responds_to_feedback_id` existed and the SELECTs already aliased it to `respondsToFeedbackId`, but neither type exported that name. - Prompt and launch-response used snake_case (`responds_to_feedback_id`) while the rest of the MCP surface is camelCase (`feedbackId`, `personaAgentId`, `mediaRef`). End-to-end fix: - Add `respondsToFeedbackId?: number` to `FeedbackInput` (both `manager.ts` and `mcp/server.ts`). - Add `respondsToFeedbackId: number | null` to `FeedbackItem`. - Add the zod parameter to `dispatch_feedback` and thread it through to `AgentManager.submitFeedback`, which now persists it via the existing DB column. - Update persona prompt and launch-response copy to camelCase `respondsToFeedbackId`, and expand the persona-side explanation so a reviewer understands *why* the link matters (parent can map round-2 follow-ups back to round-1 concerns). - New regression test: `submitFeedback` persists `respondsToFeedbackId` on a round-2 finding. Existing tests updated for the camelCase form. Found via live-dogfood review against the dev stack running this branch (feedback items #1 and #2 from agt_35cac0176460). Co-Authored-By: gpt-5.4 review agent <noreply@anthropic.com> * feat(server): status-watch polling and survival-kit guidance for the recheck round-trip (CRU-133) Live dogfood against the dev stack exposed five concrete gaps in the recheck round-trip that the text-only CRU-133 guidance couldn't close on its own. Fixing all five here so the round-trip can actually run end-to-end without manual intervention. 1. **Parent-side status-watch primitive** — new `dispatch_await_review` MCP tool (parent-only, AGENT_TOOLS) and `AgentManager.awaitReview`. Mirrors `dispatch_await_recheck` for the opposite side of the round-trip. Returns 'pending' with `pollAgainInSeconds` while the reviewer works, 'feedback_ready' on round-1 close for an allowRecheck review, 'complete' when the final round is done (or when a single-pass review closes), or 'cancelled'. Uses the same `pollCadenceSeconds` helper as the reviewer side for consistency, but doesn't auto-cancel (that's owned by the reviewer flow). 2. **Launch-response guidance rewrite** — `buildLaunchPersonaResponseText` now produces a survival kit mirroring the reviewer block: explicit "do not emit a terminal dispatch_event yet," explicit `ScheduleWakeup` mechanism, numbered steps pointing at `dispatch_await_review` instead of polling `dispatch_get_feedback` (which was the original bug — it fires on the first item that lands and misleads agents into thinking the review is done). Also adds an explicit step 3: commit fixes before `dispatch_submit_resolution`, otherwise the reviewer's round-2 diff is empty. 3. **`dispatch_event` added to PERSONA_TOOLS** — standard guidance told personas to emit dispatch events but the tool wasn't exposed, so the first call always errored. Fixed. 4. **Consolidated completion paths** — removed `status: "complete"` from `review_status`'s enum. Completion now goes exclusively through `dispatch_complete_review`. Standard guidance updated to match. The dogfood reviewer called both and the second was rejected with a state-machine error that fresh agents can't self-correct from. 5. **Reviewer round-2 obligation sharpened** — `RECHECK_ROUND_TRIP_GUIDANCE` is now explicit that round 2 has a mandatory close via `dispatch_complete_review`, and that the terminal `dispatch_event` must wait until then. The dogfood reviewer woke from its recheck but exited without submitting a round-2 verdict, leaving the review permanently in `awaiting_recheck`. 6. **`dispatch_submit_resolution` description** — added the commit-before-submit warning so it's visible from the tool surface, not just the launch-response text. Tests: - New `awaitReview` manager describe block covering all five status transitions (pending/reviewing, feedback_ready, complete single-pass, pending/awaiting_recheck, complete round-2, cancelled, and the parent-mismatch / missing-review error paths). - `mcp-launch-persona-response.test.ts` rewritten for the new text: survival-kit sleep mechanism, status-watch pattern, commit-before- submit clause, and regression guards against the old "poll feedback items" pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): make dispatch_await_review multi-reviewer-aware and drop Claude-specific sleep hints (CRU-133) Two fixes from user feedback on the first survival-kit pass: 1. **`dispatch_await_review` now supports multi-reviewer parents.** `personaAgentId` is optional; omit it and the server returns whichever of the caller's launched reviewers most needs attention (priority: feedback_ready > cancelled > complete > pending; ties broken by most recently updated). Every non-`no_reviews` response now includes the `review` it refers to so the parent knows which one changed — previously the pending branch returned bare `reviewStatus`/`roundNumber` with no review ID, which was useless when several reviews were in flight. Adds a new `no_reviews` terminal status for parents that haven't launched anything. 2. **Remove Claude-Code-specific sleep hints.** The guidance text (launch response, reviewer persona prompt, dispatch_await_recheck / dispatch_await_review descriptions) previously named `ScheduleWakeup` as the sleep mechanism. Dispatch runs claude / codex / opencode — that's only correct for the Claude path. Now the text says "wait N seconds using whatever sleep mechanism your agent runtime provides" and each runtime can figure out its own primitive. The `pollAgainInSeconds` contract is unchanged; only the prose around it is now runtime-neutral. Tests: existing awaitReview tests updated for the new response shape (review now present on pending/cancelled); new coverage for the null-personaAgentId branches (no_reviews on empty, feedback_ready preference over in-progress, pending when all are reviewing). New assertion in mcp-launch-persona-response.test.ts that the text does NOT contain "ScheduleWakeup" and does describe the sleep mechanism in agent-runtime-neutral terms. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): address backend-security-review round 1 findings (CRU-133) - dispatch_await_review: handle feedbackCount=0 on a round-1 allowRecheck approve so the parent is told to submit_resolution/cancel_recheck instead of looping on an empty dispatch_get_feedback. - awaitReview multi-review path: fold the feedback COUNT into the primary SELECT via a correlated subquery and let classifyReviewForAwait accept a prefetched count, eliminating the N+1 COUNT queries. - submitFeedback: validate respondsToFeedbackId within the existing transaction — reject 400 when unknown, cross-review, or pointing at a round-2 item so the linkage invariant is enforced server-side. * fix(server): review_status must not clobber awaiting_recheck during round 2 (CRU-133) End-to-end dogfood on the dev stack surfaced a state-machine bug: the reviewer agent pinged review_status('reviewing', 'round 2 begin') per the standard progress-ping guidance while it was actually in round 2. updatePersonaReviewStatus unconditionally SET status = 'reviewing', which downgraded the review out of 'awaiting_recheck' back to 'reviewing'. The next dispatch_complete_review call then landed in the `status === 'reviewing'` branch of completePersonaReview, which sets nextRoundNumber = 1 — so round_number stuck at 1 even though the reviewer had just finished round 2. Downstream, dispatch_await_review saw complete + round_number=1 + allowRecheck=true and would have returned feedback_ready a second time, sending the parent into a loop over already-fixed findings. Two complementary fixes: 1. updatePersonaReviewStatus now leaves the status field untouched when the review is in a non-working state (complete, cancelled, or awaiting_recheck). The progress-ping channel only updates the message. This matches the intent: a reviewer pinging "still working" during round 2 shouldn't look identical to the round-1 start. 2. completePersonaReview is defensively tolerant of a 'reviewing' status that sits on top of an existing round-1 resolution. It infers the next round number from the most recent submitted resolution rather than trusting the status label alone. If any future path manages to put the review back into 'reviewing' after a resolution, the completion still lands as round 2. Tests: regression covering the review_status clobber (status stays awaiting_recheck, subsequent complete lands as round 2) and the defense-in-depth path (forcibly setting status='reviewing' after a resolution still yields round_number=2 on the next complete). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(server): move review_status status-resolution logic out of SQL (CRU-133) PR #386 review comment called out the CASE WHEN in updatePersonaReviewStatus: logic in SQL is hard to unit-test and can't produce contextual errors. Fair — pulled it into TypeScript. - New exported pure helper `resolveProgressPingStatus(requested)` validates the progress-ping status at the tool boundary. Unit-tested in its own file (no DB). Throws a 400 AgentError with the valid-set listed for any bad value. - `updatePersonaReviewStatus` now opens a transaction, SELECTs the current status FOR UPDATE, then branches in code: - `complete` / `cancelled`: rejected with a specific 409 — "Cannot ping review_status on a <status> review" — agents get a clear signal that pings on terminal reviews aren't allowed, instead of a silent no-op. - `awaiting_recheck`: preserves the status label (reviewer is still doing work; round-2 round-trip must stay intact) and just refreshes the message. - any other status: transitions to `reviewing` as before. Tests: two new integration tests that exercise the terminal-state 409 paths (complete and cancelled), plus a dedicated unit-test file for the pure helper. The existing round-2 regression test still passes — the awaiting_recheck preservation moved from SQL to code without changing behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
selfcontained
added a commit
that referenced
this pull request
Jul 8, 2026
…s, seed data - Restyle feedback items as bordered cards with visual separation (#5) - Clip long file paths from the left using direction:rtl so filenames stay visible (#1) - Add framer-motion AnimatePresence expand/collapse on feedback item detail (#3) - Dim file links not found in current diff with tooltip fallback (#2) - Fix file navigation to only scroll when target file exists in diff (#2) - Add review seed data with resolved/pending items and thread messages (#4) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
0724d4dwhere the terminal gets stuck at "Session disconnected, reconnecting..." after a deploy and never auto-recovers0724d4dchanged timer-driven reconnects to always call the agent status API — but thatawait api(...)call was outside the try-catch block. A network error (server still booting mid-deploy) threw an uncaught exception, silently exitingensureTerminalConnectedwithout rescheduling a retry1200ms × attemptlogic asscheduleReconnect) so the loop continues until the server is back upTest plan
npm run finalize:webpasses🤖 Generated with Claude Code