Skip to content

fix(runtime-host): surface session copy failure cause - #4044

Open
fxl112233 wants to merge 1 commit into
apache:mainfrom
fxl112233:fix/session-revision-error-cause
Open

fix(runtime-host): surface session copy failure cause#4044
fxl112233 wants to merge 1 commit into
apache:mainfrom
fxl112233:fix/session-revision-error-cause

Conversation

@fxl112233

Copy link
Copy Markdown

Summary

  • Preserve the existing persistence_failed, rollback, and unknown-outcome semantics for conversation-copy commit failures.
  • Return the caught error code and message to clients after secret redaction and a conservative 1024-byte bound.
  • Write the sanitized stack to Runtime Host diagnostics and cover SQLite codes, redaction, UTF-8 truncation, and duplicate-code handling.

Fixes #4006

Verification

  • npm --workspace @maka/runtime-host run typecheck
  • npm --workspace @maka/runtime-host run build
  • node --test packages/runtime-host/dist/__tests__/session-revision-diagnostics.test.js (3 passed)
  • Session Revision diagnostic, graph-reference, and protocol tests (21 passed)
  • npm run format:check
  • npm run lint
  • npm run check:asf-headers
  • Full Runtime Host test:dist was attempted on Windows, but did not complete: unrelated IPC/filesystem tests reported EBUSY, EPERM, and read_eof, while two execution-composition workers stopped making progress. The affected suites above passed.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the Runtime Host diagnostic formatting, coordinator wiring, regression tests, and drafted this PR at @fxl112233's direction. @fxl112233 is the human contributor of record.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Preserve rollback semantics while returning a bounded, redacted summary of the commit error and logging the full sanitized diagnostic.

Generated-by: OpenAI Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026
@Astro-Han

Copy link
Copy Markdown
Contributor

This is a synthesis of the independent blind review by @Sol-404ARE at exact head ebc73d85e2587ef71adb183dc7b3be54309f3020 (base ff226aff2f3082e54300dea0ee9e410898ae2c0f, 3 files +144/-2). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

  • Read gh pr diff 4044 (fix: surface session-copy failure causes, 3 files) and confirmed the new sanitized host diagnostic with 8 KiB bound and the bounded client cause (1024-byte) routed into the existing rollback classifier at session-revision-coordinator.ts:569-578 and the preserved state-machine at :762-795.
  • Checked exact-head CI: label run 33107705615 SUCCESS, test run 33107706323 action_required (not green), Windows recovery run 33107706020 action_required, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — GO (0 P0–P3)

  • No Standards finding. Commit trailer and PR metadata hygiene pass; git diff --check PASS; changed-file Biome 3/3 PASS; worktree clean.

Spec — GO (0 P0–P3)

  • The coordinator at 569-578 now surfaces a sanitized host diagnostic (redacted, 8 KiB bound) and a bounded client cause (secret-redacted, 1024 UTF-8-byte bound) into the original rollback classifier. 762-795 strictly preserves committed→success, discard success→persistence_failed, and unconfirmable terminal→commit_outcome_unknown + drain. The underlying SQLITE_BUSY/SQLITE_FULL code should remain as message detail and not escape as an undeclared operation code — current handling keeps it as detail and matches the declared-code 1024-char decoder at protocol/operations.ts:439-453.
  • New helpers correctly apply redactSecrets + byte-bound truncation for both audience projections, and the tests cover SQLite code/message, secret redaction, multi-byte truncation, and duplicate-code suppression. Not running a coordinator failure-injection E2E is an optional hardening gap, not a regression — the call site passes helper output directly to the unmodified classifier. Entropy is neutral-to-reduced: a new pure cohesive module concentrates the two projections and reuses existing primitives.

What was proven green: label SUCCESS, local git diff --check and Biome pass, Windows inventory and epoch guard pass (via prior), worktree clean, head unchanged.

What I did not judge: a full coordinator failure-injection E2E with injected SQLITE_FULL and duplicate-operation-code replay was not executed beyond unit coverage — verification was by code inspection and the passing unit suite noted above.

Gate: Standards 0 and Spec 0 — code is GO, but the hosted CI gate is pending (test and windows_recovery are action_required awaiting workflow approval, not SUCCESS), so head ebc73d85 is not yet merge-ready despite the green code verdict. Seal: notes/pr-4044-provisional.md.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(runtime-host): surface root cause for session revision commit failure instead of generic message

2 participants