Skip to content

fix: harden session recovery, remote lifecycles, and Git review - #131

Open
S-huNsuke wants to merge 2 commits into
chenbstack:mainfrom
S-huNsuke:codex/fix-remaining-audit-issues
Open

S-huNsuke wants to merge 2 commits into
chenbstack:mainfrom
S-huNsuke:codex/fix-remaining-audit-issues

Conversation

@S-huNsuke

@S-huNsuke S-huNsuke commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

变更内容 / Summary

Unreadable saved state, archiving active terminals, and stopping external control could discard session data or leave previously accepted commands authorized. Review also hid staged files before the first commit and rendered Git failures as empty results. This PR addresses those regressions and bounds the related remote-resource and refresh lifecycles.

  • Preserve unreadable state before a fresh save; retry preservation after I/O failures. Confirm busy workspace archives, revalidate after the modal, capture final metadata, and force/drain scrollback before archive or exit.
  • Revoke ControlBridge tokens and accepted connections on stop, and recheck the accepting generation immediately before main-thread actions.
  • Bound all WebSocket queue entries by total bytes and item count while allowing large image responses. Limit file I/O per client and globally, retaining unfinished tickets across disconnect/restart. Cap HTTP connections, enforce a fixed deadline, and cancel accepted peers on stop.
  • Support unborn HEAD with a repository-format empty tree. Propagate Git errors to localized Review states. Bound untracked-file reads and validate the actual nonblocking, no-follow descriptor against path-replacement races.
  • Share in-flight quota refreshes per agent/home, cancel obsolete work, and reject late results. Pin Sparkle 2.6.4 consistently in XcodeGen, the Xcode project, and the tracked lockfile; enforce locked resolution in CI/release.

Includes English source strings and Simplified Chinese translations, targeted regression coverage, and CI wiring. No persistence schema migration or architectural restructuring.

验证方式 / Testing

  • CI passed on ff3e349: run 39224831139. The full Xcode build and all 431 XCTest tests passed (0 failures), along with the browser and native-logic script steps.

  • The initial CI failure on 47c9bfc exposed a Grok default-argument function-reference mismatch. Fixed by invoking the reader inside a no-argument async closure. Reader declarations in the isolated quota tests now come directly from production source, preserving labels, parameter types and defaults; corrected fixtures reproduced the same compiler error before the fix and passed all 11 quota scenarios afterward.

  • 55 Node test entries/subtests passed, 0 failed, 0 skipped using the combined command below. This includes the 42 existing browser tests plus compiled production-source harnesses; it is not a full app XCTest count.

node --test scripts/test-web-remote-*.mjs scripts/test-git-review.mjs \
  scripts/test-git-untracked.mjs scripts/test-persistence-recovery.mjs \
  scripts/test-workspace-lifecycle.mjs scripts/test-control-lifecycle.mjs \
  scripts/test-usage-refresh.mjs scripts/test-http-lifecycle.mjs \
  scripts/test-web-file-admission.mjs
node scripts/verify-dependency-lock.mjs
git diff --check
  • Real temporary Git repositories (including SHA-256), filesystem permissions/path replacement, Unix sockets, and loopback Network.framework connections. Store/UI, Ghostty, quota readers, and file-I/O fixtures are isolated where necessary; no real user state or credentials are used.
  • Internal coverage includes 39 Git checks, 30 bounded-read assertions, 24 recovery scenarios, 11 workspace lifecycle scenarios, 11 quota scenarios, 6 HTTP scenarios, 15 outbound-queue assertions, and 7 file-admission scenarios. Control tests include stop/restart and requests paused before main-thread execution.
  • Swift frontend parsing, project plist/YAML syntax, and new localization checks passed. Real SwiftPM resolution and forced locked resolution verified Sparkle revision 0ef1ee0220239b3776f433314515fd849025673f.
  • Full app build/XCTest and real Ghostty/AppKit UI verification were not run locally: only Command Line Tools is installed and GhosttyKit is unavailable. Native HTTP integration tests and the script regressions are included in CI for an Xcode runner.

截图或录屏 / Screenshots or recordings

No screenshots captured; full app UI validation was unavailable locally. Visible changes are the busy-archive confirmation and Review error messages.

关联议题 / Related issues

Follow-up to the already merged #130; covers the remaining audit findings.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant