fix: batch of 8 onboarding / MCP / harness / chat / auth bugs#39
Merged
fix: batch of 8 onboarding / MCP / harness / chat / auth bugs#39
Conversation
- #6, #7: onboarding Save Draft validates name + model, status-branched navigation - #8: MCP tiger_junction servers show Needs Verification before netid link - #9: chat header MCP badges refresh after OAuth reconnect - #14: harness detail page exposes a draft/started/stopped status Select - #17: interrupted assistant messages persist with interruptionReason across all error paths - #22: first-response spinner uses RoseCurveSpinner with rotating flavor text - #23: new /sign-up route, landing CTAs target it, Clerk cross-links work - dev-only: CORS allowlist covers localhost and 127.0.0.1 ports 3000-3020
Owner
Author
|
@claude review this |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Ports the refreshHealth callback plumbing from chat/index.tsx so the McpServerStatus OAuth reconnect button re-runs the health check after a reconnect — previously the status would stay stale until a harness swap. Also adds pytest coverage for the interrupted / interruptionReason branches in save_assistant_message that were added in this PR but previously untested.
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
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
Batched fixes for 8 reported bugs spanning onboarding, MCP status display, the harness editor, chat streaming UX, and the sign-in/sign-up flow. Plus a small dev-only CORS convenience. All eight fixes were investigated against the original reports, implemented, and type-checked; the new staging test suite (79 Convex + 166 web + 74 FastAPI = 319 tests) still passes cleanly on top of this branch.
Per-bug breakdown
Bug #6 —
Save Draftcreated harnesses with no modelCause:
handleSaveDraftsilently fell back to"gpt-4o"when no model was selected and didn't validatename.Fix:
apps/web/src/routes/onboarding.tsx— validatesname.trim()+ model before mutating; Save Draft is disabled with a tooltip until both are set. Silent fallback removed.Bug #7 — Onboarding
Start chattingsilently produced a draftCause:
onSuccessalways navigated to the same place regardless ofstatuspassed to the mutation.Fix: same file —
onSuccessbranches onvariables.status:"draft"→/harnesseswith a toast, otherwise →/chat.Bug #8 — tiger_junction MCP servers showed
connectedbefore netid was linkedCause:
/mcp_health/checkreturned ok if the MCP endpoint was reachable, ignoring Princeton netid requirement. UI had no variant for "reachable but needs user action".Fix (backend):
packages/fastapi/app/routes/mcp_health.py— fortiger_junctionservers, early-returnauth_requiredwhenever the resolvedUserContexthas noprinceton_netid. Applies to both/checkand/generate-prompts.Fix (frontend):
apps/web/src/components/mcp-server-status.tsx— newneeds_verificationstatus variant with aGraduationCapbadge, plus distinct badges perauthType(Public / Key / OAuth / Princeton).Known caveat: the
needs_verificationbadge only renders a tooltip — no click-through to the Princeton link flow (PrincetonConnectRowlives in harness settings). Filed as follow-up.Bug #9 — Reconnecting an OAuth MCP server left stale status badges in the chat header
Cause: health-check effect in
chat/index.tsxonly ran on mount; reconnect had no way to re-trigger it.Fix:
apps/web/src/routes/chat/index.tsx— factored health check intorunHealthCheck(servers)+refreshHealth()callbacks with a cancellation ref. ThreadedonRefreshHealththroughChatHeader→McpServerStatus; wiredonReconnected={() => { onRefreshHealth(); onRefreshCommands(); }}.Known caveat: only the status-popover reconnect path refreshes header state. Reconnecting from an inline
OAuthReconnectPromptinside a failed tool-call result still won't refresh badges. Left for a follow-up.Bug #14 — Harness status was read-only in the editor
Cause: the UI had only a static status badge even though the Convex schema supports
draft | started | stopped.Fix:
apps/web/src/routes/harnesses/\$harnessId.tsx— replaced the label with a<Select>(colored dot per state). Included inhasChangesandhandleSave. Adds a hint "Saving will promote this harness out of draft" when transitioning fromdraft.Known caveat: no client-side guard against promoting a malformed draft; server-side validation in
harnesses:updateshould reject invalid promotions (unverified here).Bug #17 — Interrupted assistant messages vanished on some failure paths
Cause: only the browser-side
saveInterruptedMessagemutation existed. Backend-side failures (HTTP errors, max-tool-iterations) produced nothing in Convex, so users saw the bubble disappear.Fix (schema):
packages/convex-backend/convex/schema.ts+convex/messages.ts— addedinterruptionReason: v.optional(v.string())to themessagestable;saveAssistantMessageinternalMutation acceptsinterrupted/interruptionReasonargs.Fix (backend):
packages/fastapi/app/routes/chat.py— new_save_interrupted()helper called from all three exception handlers (HTTPStatusError,HTTPError, generic) and the post-loop max-iterations path. It also records budget usage if captured.packages/fastapi/app/services/convex.py—save_assistant_message()gainedinterrupted+interruption_reasonkwargs.Known caveat: the frontend-path
messages:saveInterruptedMessagedoes not yet acceptinterruptionReason. The two paths will diverge if a UI feature displays the reason later.Bug #22 — First-response spinner felt broken; nothing happened until tokens arrived
Cause: streaming bubble mounted only once
streamingContenthad text, and the fallback was a static spinner.Fix (new component):
apps/web/src/components/pending-response-indicator.tsx—RoseCurveSpinner+ rotating flavor text (16 messages, 2.6s cadence, random starting index, honorsprefers-reduced-motion).Fix (callers):
apps/web/src/routes/chat/index.tsxandapps/web/src/routes/workspaces/index.tsx—showStreamingBubblegate widened to(isStreaming || …)so the bubble mounts immediately; inlineRoseCurveSpinner size={14}replaced with<PendingResponseIndicator />.Bug #23 — Landing-page
Get StartedCTAs routed to sign-inCause: no
/sign-uproute existed; all CTAs linked to/sign-in; Clerk's footer "Sign up" link fell back to its hosted page.Fix: new file
apps/web/src/routes/sign-up.tsxmirroringsign-in.tsxwith Clerk's<SignUp>andsignInUrl="/sign-in".apps/web/src/routes/index.tsx— 4Link to="/sign-in"→/sign-up.apps/web/src/routes/sign-in.tsx— addedsignUpUrl="/sign-up"on<SignIn>so the widget's footer link routes locally.apps/web/src/routeTree.gen.tsregenerated by the TanStack router plugin.Dev-only change — CORS allowlist
packages/fastapi/app/main.py— expanded local dev origins to coverlocalhostand127.0.0.1on ports 3000–3020, so running the web dev server on an alternate port (e.g. 3004) during bug verification no longer hits CORS preflight failures. Production and hosted origins are unchanged.Test plan
/landing → click any Get Started CTA → lands on/sign-upwith Clerk Create-account form./sign-up→ "Sign in" footer link → lands on/sign-in. Round-trip works./onboarding→ Save Draft disabled with tooltip until name + model are set. After saving, land on/harnesseswith a confirmation toast; harness is persisted asdraftwith the chosen model (no silentgpt-4o)./onboarding→ Start chatting with valid inputs → lands on/chat, harness isstarted./harnesses/:id→ status dropdown visible with draft/started/stopped options; changing draft → started shows the promotion hint; save persists./chatwith tiger_junction MCP server and no Princeton netid linked → MCP status popover shows the graduation-cap "Needs verification" badge, not Connected./chatwith OAuth MCP server → open MCP status popover → click Reconnect → after Clerk OAuth flow closes, header badges re-run the health check without a full reload./chatand/workspaces→ send a prompt → rose-curve spinner + rotating italic flavor text appears immediately; first tokens replace it. With macOS Reduce Motion on, text stays on one phrase.interrupted: trueand a populatedinterruptionReasonin Convex.apps/web,packages/convex-backend,packages/fastapi). Verified locally: 166 + 79 + 74 = 319 green.Known follow-ups (intentionally out of scope)
needs_verificationbadge should link into the Princeton connect flow (Bug Full application build: landing page, auth, chat with MCP tool use, harness management, and OAuth #8).OAuthReconnectPromptin tool-call results should also callonRefreshHealth(Bug Pin Clerk issuer to prevent JWKS endpoint spoofing #9).messages:saveInterruptedMessage(frontend mutation) should acceptinterruptionReasonfor parity with the backend path (Bug Feat/filesystems sandboxes #17).harnesses:updateshould reject draft → started promotions that don't satisfy full-harness invariants (Bug Add Claude Code GitHub Workflow #14).save_assistant_message,parseAuthRequiredError, etc.) but does not yet assert the newinterrupted/interruptionReasonbranches or theneeds_verificationUI state. Lines 78/80 ofapp/services/convex.pyshow up as uncovered in the FastAPI coverage report.