Skip to content

Commit staged changes with a typed message when Ryco can't write one - #860

Merged
sak0a merged 1 commit into
mainfrom
qa-fix/commit-staged-message
Oct 29, 2026
Merged

sak0a merged 1 commit into
mainfrom
qa-fix/commit-staged-message

Conversation

@sak0a

@sak0a sak0a commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

What and why

The Linux QA pass (bug 3) found that Commit staged (#769) could not commit at all when commit-message generation failed, and showed the raw generation output:

  • With the default text-generation model's provider not signed in (Codex), the panel printed Codex's whole stderr in red: model and sandbox lines, the full commit-message prompt, the staged patch, and a dozen 401 retries.
  • With a provider whose output isn't JSON: "Cursor Agent returned invalid structured output", and no way forward.
  • The server already accepted a commitMessage for a staged-only commit, but the button never sent one.

Now:

  • A message field in the Staged view. Blank still asks the node to generate one; a typed message is sent as commitMessage with the same stagedOnly guard. It clears after a successful commit.
  • A failed generation keeps the review. Generation runs before anything is written, so the controller (client-runtime localChanges) recognises a TextGenerationError as "changed nothing": it keeps the reviewed index and shows "No commit: the commit message could not be generated (…). Type a message and commit again." Other failures (index moved, uncertain delivery) still clear the review as before.
  • The prompt and patch never reach the UI. cliFailureDetail (server, TextGenerationUtils.ts) builds the detail of a failed Codex or Claude CLI run: it drops every output line that is also a line of the prompt, strips ANSI and log timestamps, keeps the last two error lines and bounds the result at 400 characters. This applies to every surface that shows a text-generation failure (the git action toasts too), not just this button. The other providers' failures never echoed raw output.

For the QA case the panel now reads: "No commit: the commit message could not be generated (Codex CLI command failed: ERROR: Reconnecting... 5/5 ERROR: unexpected status 401 Unauthorized: Missing bearer or basic authentication in header, …). Type a message and commit again."

Overlaps

Validation

  • New: TextGenerationUtils.test.ts (prompt lines dropped, error lines kept, fallbacks, bound), a controller test (review kept, then a typed commit), and a DiffPanel.staging browser test (alert, no commitMessage on the first try, typed message sent, field cleared).
  • bun run check, bun run check:changed (typecheck, related tests, read-from-disk tests incl. serverBundleBoundaries): pass.
  • Web bundle budget: boot 516.3 / 520 KiB, RootAppShell 411.8 / 420 KiB (111 files), chat route 756.8 / 760 KiB (155 files). Unchanged.

Verified on the Linux QA box

Integration build, real ryco serve, text generation set to the fake Cursor (returns non-JSON): Commit staged showed "No commit: the commit message could not be generated (Cursor Agent returned invalid structured output). Type a message and commit again." with the staged review still there. Typing "Tweak the greeting" and committing created b50ee4b Tweak the greeting with exactly the staged hunk of src/app.ts; the unstaged edits to README.md and src/app.ts stayed in the working tree. A follow-up commit drops the doubled period the box run showed ("output.).").

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 28, 2026
sak0a added a commit that referenced this pull request Oct 29, 2026
…860)

Rebased for the merge train as one commit on top of #858: this PR's commits and its review fixes ("Show the error message of a failed Claude text-generation run instead of its JSON envelope").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sak0a
sak0a force-pushed the qa-fix/commit-staged-message branch from 45cc03e to ad9ce1d Compare October 29, 2026 03:22
@sak0a
sak0a changed the base branch from main to qa-fix/acp-session-updates October 29, 2026 03:22
sak0a added a commit that referenced this pull request Oct 29, 2026
Rebased for the merge train as one commit on top of #860: this PR's commits and its review fixes ("Re-probe Claude after a signed-out answer so Refresh sees a new login").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:XXL and removed size:L labels Oct 29, 2026
@sak0a
sak0a force-pushed the qa-fix/acp-session-updates branch from 3f451db to ee3170f Compare October 29, 2026 04:17
sak0a added a commit that referenced this pull request Oct 29, 2026
…860)

Rebased for the merge train as one commit on top of #858: this PR's commits and its review fixes ("Show the error message of a failed Claude text-generation run instead of its JSON envelope").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sak0a
sak0a force-pushed the qa-fix/commit-staged-message branch from ad9ce1d to 17ccc51 Compare October 29, 2026 04:18
sak0a added a commit that referenced this pull request Oct 29, 2026
Rebased for the merge train as one commit on top of #860: this PR's commits and its review fixes ("Re-probe Claude after a signed-out answer so Refresh sees a new login").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:L and removed size:XXL labels Oct 29, 2026
@sak0a
sak0a force-pushed the qa-fix/acp-session-updates branch from ee3170f to 8ac7f84 Compare October 29, 2026 07:40
…860)

Rebased for the merge train as one commit on top of #858: this PR's commits and its review fixes ("Show the error message of a failed Claude text-generation run instead of its JSON envelope").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sak0a
sak0a force-pushed the qa-fix/commit-staged-message branch from 17ccc51 to 3001316 Compare October 29, 2026 07:41
@sak0a
sak0a changed the base branch from qa-fix/acp-session-updates to main October 29, 2026 07:41
@sak0a
sak0a merged commit b589d03 into main Oct 29, 2026
2 checks passed
sak0a added a commit that referenced this pull request Oct 29, 2026
Rebased for the merge train as one commit on top of #860: this PR's commits and its review fixes ("Re-probe Claude after a signed-out answer so Refresh sees a new login").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
sak0a added a commit that referenced this pull request Oct 29, 2026
Rebased for the merge train as one commit on top of #860: this PR's commits and its review fixes ("Re-probe Claude after a signed-out answer so Refresh sees a new login").

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sak0a

sak0a commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Merge train 3 reviewed this PR before merging. The squash includes these fixes from the review:

  • Show the error message of a failed Claude text-generation run instead of its JSON envelope. Claude runs with --output-format json, so a failed run with empty stderr reported its whole JSON result envelope, cut to 400 characters, with the message buried inside. A failed run now reads the envelope's error result first (e.g. "Not logged in · Please run /login") and passes it through the same bounded failure detail; any other output falls back to the detail as before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant