fix(remote-crew): allow same major.minor peers and surface create failures

[iamwhatever]

Problem / Motivation

Creating a session on a connected crew (New chat menu → New chat on crew → pick a crew) failed silently: no session appeared in the local sidebar list, none was created on the remote crew, and no error was shown — the click read as a no-op.

Two independent causes:

1. The remote-execution version gate (ensure_version_parity) required the peer's kiro_crew.__version__ to match the local build byte-for-byte, so any build skew refused the create before the peer was ever asked to open a session.
2. The frontend createRemoteChatMutation had onSuccess but no onError, so react-query swallowed the resulting 502 and nothing surfaced.

Why it matters

The create opens the peer session before the local slot and, by design, leaves nothing behind on failure. So a version skew — the common case right after an update lands on one end but not the other — made "New chat on crew" look broken with no diagnosis, and users had no way to tell it was a version issue rather than a bug.

What changed (motivation → approach → change)

• Root cause 1 — the gate was too strict. ensure_version_parity now compares the major.minor series, not the full string. 0.6.0 and 0.6.3 interoperate; 0.6.x vs 0.7.x is still refused — a feature release moves the unversioned frame vocabulary the two ends exchange, and on a 0.x line that axis is the minor (the major stays 0). Added _version_series() to parse the leading semver. When either side is not semver-shaped (a packaging build id replaces __version__ at build time), compatibility cannot be proven by series, so it falls back to strict full-string equality. The refusal message now says "same major.minor version".
• Root cause 2 — the failure was swallowed. createRemoteChatMutation gains an onError that surfaces the backend reason inline in the submenu (new remoteCrewError state + a truthiness-gated error line). The reason is read off the rejected value's .message: RTK's .unwrap() rejects with a SerializedError plain object, not an Error, so an instanceof Error guard would have dropped the message. Crew rows use onSelect + preventDefault so a failed create keeps the menu open long enough to read the error, while a successful create moves focus to the composer (which closes the menu).

Tests

• test/test_remote_crew_execution.py:
  • test_a_patch_skew_in_the_same_series_passes — same major.minor, different patch is allowed.
  • test_a_different_minor_is_refused — a minor bump is refused and the message names major.minor.
  • test_a_non_semver_build_id_falls_back_to_strict_equality — a build id compares by full-string equality.
  • Existing equal-version / unknown-version / unreachable / redaction cases still hold (189 pass).
• Frontend: catalogParity (77) and the ChatSidebar suites pass; tsc -b and eslint clean. No new user-facing string was added — the failure text comes from the backend, already localized by apiFailure.

Manual verification

N/A — unit coverage is sufficient for the version gate. The frontend change is a conditional error line whose steady state is unchanged; reproducing the live surface needs a connected peer at a mismatched major.minor, which a unit test cannot stage.

Screenshots / video

Why no screenshot: the only rendered change is a conditionally-shown error line in the "New chat on crew" submenu that appears solely when a remote create fails, and it requires a live version-mismatched or unreachable peer to trigger. The steady-state (no-error) sidebar is pixel-identical.

Related Issues

no linked issue: field-reported behaviour after #7693 landed; no tracking issue was filed.

Pattern harvest

Rule candidate: review-prompt
Pattern: a react-query useMutation whose mutationFn dispatches a thunk .unwrap() but declares no onError silently swallows the rejection — a user-visible action that "leaves nothing behind" on failure must surface the reason. Corollary for the extraction: .unwrap() rejects with a SerializedError plain object, so err instanceof Error is false; read .message off the object.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of fe6decb8d58b122f3c1adb54fa0e4d423156fc92 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design review complete. The stable-channel problem is real and the fix is sound; one compatibility-contract gap is worth surfacing.

Design-Verdict: CONCERNS

Series parity is right for stable releases but silently waves through prerelease skew, exactly where the frame vocabulary moves fastest.

Watch

• parse_version deliberately strips prerelease/build suffixes ("0.4.0rc3, as stamped into every insider wheel"), so two insider/RC builds of the same upcoming release — e.g. 0.7.0-insider.1 vs 0.7.0-insider.30, weeks of feature work apart — now pass parity. That is the one channel where the frame vocabulary changes within a series, and the docstring's own harm ("a session that mostly works, which is worse than one that plainly refuses") returns there. The "non-semver → strict equality" fallback does not catch these because they parse cleanly.
• The loosened gate turns "a patch release never moves the frame vocabulary" into a pure release-discipline invariant: the wire contract is still unversioned, so nothing in code or CI fails a hot-fix that adds a frame. The gate remains a proxy; this PR raises the cost of one undisciplined backport. A frame-vocabulary/protocol version in the handshake is the durable root-cause fix — follow-up, not this PR.

Suggestions

• When either version string carries a prerelease/dev suffix, fall back to strict full-string equality (same shape as the existing non-semver fallback) — one-line change that closes the insider gap within this PR's scope.

[DESIGN-REVIEWED] fe6decb

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of fe6decb8d58b122f3c1adb54fa0e4d423156fc92 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/dashboard/remote_relay.py:151 -- parse_version(value) accepts numeric-prefix non-semver IDs instead of using strict equality -> Fix: validate the complete version syntax before comparing series.

[GPT-REVIEWED] fe6decb

False positive or not applicable? A repository writer can comment:
/ai-review override gpt fe6decb8d58b122f3c1adb54fa0e4d423156fc92: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of fe6decb8d58b122f3c1adb54fa0e4d423156fc92 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff checks out against the frontend patterns — ErrorNotice is the sanctioned shared primitive with role="alert", the pending state is covered by the existing creatingSlot spinner and disabled rows, and the failure copy comes from the backend with concrete versions plus a named action. Two placement/reachability edges are worth naming; neither is material for typical crew counts.

UX-Verdict: PASS

Silent no-op becomes an inline, announced, actionable error at the point of click — the fix lands where the user's attention already is.

Suggestions

• Move errRow outside the mobile max-h-[240px] overflow-y-auto div (and below the desktop DropdownMenuSubContent scroll area, or scrollIntoView it): with 8+ crews the error renders below the fold, recreating the invisible failure this PR exists to fix.
• The askAgent button inside Radix menu content sits outside the menu's roving focus, so a keyboard user who triggered the failure with Enter can hear the error (role="alert") but cannot reach "Ask agent" — either accept it as pointer-only or move focus to it on error.

[UX-REVIEWED] fe6decb

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed fe6decb8d58b122f3c1adb54fa0e4d423156fc92 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] fe6decb

Verdict parsed from the review's SHA-scoped output markers for commit fe6decb8d58b122f3c1adb54fa0e4d423156fc92.

False positive or not applicable? A repository writer can comment:
/ai-review override fable fe6decb8d58b122f3c1adb54fa0e4d423156fc92: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of fe6decb8d58b122f3c1adb54fa0e4d423156fc92 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. Here is the review.

First-Principles-Verdict: CONCERNS

Both fixes sit at cause level and reuse existing mechanisms — but the swallowed-rejection cause has five counted, unfixed siblings in the same file.

What this change ships

Intent: make "New chat on crew" stop failing silently on routine version skew — a FIX (two causes).

1. Crews one patch release apart can now start sessions — justified, cause-level.
2. Non-semver build ids still require an exact match — justified, derived.
3. An oversized peer version string now refuses instead of HTTP-500ing — undeclared (only in a code comment), justified.
4. Refusal text now says "same major.minor version" — justified.
5. A failed crew create shows the backend's reason inline in the submenu — justified, the fix.
6. The menu stays open after a failed crew create — justified, declared.
7. The New-chat menu is now controlled and closes explicitly on success — undeclared, required by item 6.
8. Closing the menu clears a stale error — undeclared, minor, justified.

Watch

• The PR's own harvest names the general cause — "a useMutation whose mutationFn dispatches a thunk .unwrap() but declares no onError silently swallows the rejection" — yet fixes one instance. Grepped dispatch(createSlot mutations in ChatSidebar.tsx: 5 siblings ship onSuccess: focusComposer with no onError (createAutopilot, createCrew, createChat, createPlainChat, createEphemeralChat; createChatInFolder surfaces only to console). Accepted-and-deferred: each needs its own surface, but a local-gateway failure on those reads as the same silent no-op.
• Description says "Added _version_series()" and that focusing the composer "closes the menu"; the diff instead reuses parse_version and closes the menu explicitly (setNewChatMenuOpen(false)). Both shipped shapes are better than described — stale prose, not a code change.

[FIRST-PRINCIPLES-REVIEWED] fe6decb

[iamwhatever]

• An oversized numeric version crashes remote operations — fixed in db9ee68 (span=16688a917388)

_version_series did int(m.group(1)) on the peer-reported version, and CPython caps int(str) at 4300 digits — a peer returning thousands of leading digits raised ValueError outside the RemoteTurnError handler and 500ed the create. Fixed by dropping the hand-rolled parser and reusing the shared kiro_crew.apps.version.parse_version inside try/except ValueError: parse_version(local)[:2] != parse_version(value)[:2], and any ValueError (oversized digits OR a non-semver build id) falls back to strict full-string equality. This also resolves the duplicate-parser observation. New test test_an_oversized_numeric_version_is_refused_not_crashed drives a 5000-digit version and asserts a RemoteTurnError, not a crash.

An oversized numeric version crashes remote operations

[iamwhatever]

• Remote-create errors bypass the required shared error surface — fixed in db9ee68 (span=c3bc99e7b8e9)

The hand-written <div className="text-danger">{remoteCrewError}</div> violated the blocking errors-use-error-notice AUTOSDE rule (website/AUTOSDE.yaml). Replaced with the shared component: <ErrorNotice message={remoteCrewError} variant="inline" askAgent testId="new-chat-on-crew-error" /> (askAgent on — the create leaves nothing behind). Also fixed the related regressions in the same change: the crew rows onSelect preventDefault had removed the auto-close, so a successful create left the menu open (duplicate-session risk) — the New-chat menu is now controlled (open/onOpenChange) and closes in onSuccess, and onOpenChange clears a stale remoteCrewError on close.

Remote-create errors bypass the required shared error surface

[iamwhatever]

• Hand-written text-danger div re-implements the error surface — fixed in db9ee68 (span=f7cc1ed5117a)

Per website/AGENTS.md ("Never a hand-written <div className=\"text-danger\">{err}</div>") and the blocking errors-use-error-notice rule, the banned div is replaced by <ErrorNotice message={remoteCrewError} variant="inline" askAgent testId="new-chat-on-crew-error" />, which carries the structured hand-off instead of re-implementing the visual. The two paired advisory observations are also addressed: the menu now closes on a successful create (controlled open), and remoteCrewError is cleared on menu close. The _version_series duplicate-parser point is resolved separately by reusing parse_version. The 5 sibling createSlot().unwrap() mutations without onError are a pre-existing, out-of-scope gap left to a follow-up (this PR fixes the crew-create path only).

Hand-written text-danger div re-implements the error surface

[bolichen97]

LGTM.