Skip to content

Run headless Orca with bundled Bun and preserve SQLite durability - #22635

Open
OrcaWin wants to merge 26 commits into
OrcaWin/np-async-profile-persistencefrom
OrcaWin/np-bun-profile-persistence
Open

OrcaWin wants to merge 26 commits into
OrcaWin/np-async-profile-persistencefrom
OrcaWin/np-bun-profile-persistence

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 65 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7389 $\color{#cf222e}{\Huge{\mathbf{−}}}$​196 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7193
Prod 100 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5349 $\color{#cf222e}{\Huge{\mathbf{−}}}$​586 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4763

ELI5

Headless Orca brings its own verified Bun runtime, so users can keep their installed Node version. Existing node orcad.js service commands hand off to bundled Bun before opening a profile. The desktop stays on Electron, and the SQLite/background-writer guarantees from #22612 remain in place.

Stack: #22612 → this PR → ordinary JSON-backend retirement. This extracts the runtime foundation from #16741; connecting managed remote deployment remains separate.

What Changed

  • Pin and hash-verify Bun 1.4.2. Artifacts include the server, persistence workers, terminal daemon/gate, watcher and required data. Resolve symlinked launchers correctly; incomplete runtime installations return configuration exit code 78 before opening a profile.
  • Extend the existing SQLite adapter for Bun, retaining foreign keys, transaction boundaries and integer safety. Large write metadata returns exact bigint instead of throwing after a commit; explicit undefined bindings are rejected consistently. Read-only WAL handling leaves directory-permission decisions to SQLite.
  • Run disposable native/SQLite readiness checks before profile admission, and budget preflight separately from server readiness. Existing Node 18 launchers are qualified by actual bundle-load and handoff checks in CI.
  • Connect launcher lifetime to the runtime through IPC. Losing the launcher starts bounded graceful shutdown, including disconnect before startup finishes. Detaching also escapes the Windows child job that otherwise kills Bun before it can drain. Windows stop requests disconnect IPC so requests made before handler installation remain observable. A separate POSIX session avoids terminal-group signal duplication; repeated stop signals coalesce. POSIX SIGHUP is explicitly ignored for headless operation; SIGINT/SIGTERM remain the stop contract. Keep the 15-second watchdog referenced so an unfinished stop cannot silently exit successfully. Startup configuration failures retain exit 78 even if launcher loss begins shutdown first.
  • Preserve Windows terminals when Ctrl-C interrupts a native foreground command: the job gate ignores console interrupts while supervising its shell and removes the listener on normal exit or failure. Real node-pty and direct Bun controls confirmed the defect was in the gate; both Git Bash launch variants now preserve the shell.
  • Preserve Windows owned jobs, MSYS containment, real shell-spawn receipts and fallback, resize/clear and final output. Failed resume retries automatically; failed suspension preserves the terminal and permits later explicit retries. Enumerate jobs larger than 64 processes.
  • Discover POSIX process groups asynchronously and preserve pause/resume ownership. Resume only groups this flow-control pause stopped, leaving Ctrl-Z jobs suspended. Permission-denied pauses stop retrying and release acquired stops; partial resumes retain their retry obligation. Revalidate terminal ownership and resume the shell after its jobs.
  • Inspect actual processes when the terminal adapter reports only its spawn filename. Ordinary commands such as vim/npm are visible, login wrappers no longer make idle shells look busy, and failed observations remain conservative. Count background/suspended child work independently for close protection, while preserving existing ambiguous/excluded-agent decisions. Use the existing optional child-evidence field and a conservative boolean for older clients.
  • Support the same Bun adapter in daemon and in-process terminal routes, including launch cancellation and early exits. Preserve bundled search assets/notices, target hashes and executable permissions.
  • Harden the future managed SSH deployment path with configured HTTP/proxy handling, progress-based download timeouts, corrupt-cache recovery, libc fallback detection, structured stop results and install-lock cleanup. It has no production callers; no existing SSH route is switched to it here.
  • Share native ZIP extraction, honor ORCA_UNZIP_BIN on Windows, and remove unused extract-zip. Ordinary desktop builds do not bundle or download the unused deployment template; explicit template builds remain available.

Why

Users can move directly from existing Node installations to a capable bundled runtime without first upgrading Node or installing Bun globally. Driver differences stay behind existing persistence interfaces. Immutable artifact publication and explicit lifecycle acknowledgements protect concurrent readers and saves.

An ORM would not replace the cross-database/file recovery protocols. Ordinary JSON-backend deletion remains separate until every writable owner route is qualified; JSON import, export and selected recovery remain supported.

Linked Issue

No linked issue. Stacked on #22612; focused extraction from #16741.

Visual Proof

N/A — runtime, persistence and packaging changes; no rendered interface changes.

Testing

Current head: 5f12c53, stacked on 7306dd2. All ten native qualification lanes passed. The required general PR verification gate is running on the final head; the native matrix is green. The base PR complete CI is green. The Windows foreground test exposed a real gate-wrapper interrupt defect, now reproduced and fixed on a real Windows host; the corrected native x64 and ARM CI lanes are green.

Gate Current evidence
Local static/build checks After rebase: full normal typechecks, full anti-slop lint, changed-code quality, formatting and commit hooks passed; standalone Bun build passed. 88 focused integration tests passed. Earlier CLI build also passed. After the final Windows gate fix: node typecheck and rebuilt-artifact gate/launcher/job-control suite passed (15 tests).
Node 18 compatibility Real Node 18.20.8 loads the final bundle and hands off to Bun 1.4.2; disposable SQLite preflight acknowledges revision 1. Actual Node 18 → Bun lifecycle tests cover owner death before/after listener installation, nohup and shutdown deadline.
Windows interrupt qualification Current gate and Git Bash tests pass on the real Windows host. Both shell variants preserve the foreground-command interrupt and continue accepting commands. Both Windows x64 and ARM CI passed: 842 tests and 43 platform skips each. Windows x64 also passed actual Node 18 bundle load and Bun handoff.
Real Windows launcher Node 24 → Bun 1.4.2: six lifecycle cases pass (owner death and normal stop before/after handler installation, stalled drain deadline, and configuration exit 78). Old classifier negative control fails with exit 1.
Shutdown and flow-control suite 262 passed, 2 platform skips across 34 files.
Final terminal corrections 74 passed across five focused files, including actual renderer close-probe consumption. Independent review reran the stopped/background-work, sibling-agent and detached-agent reproductions.
Broader daemon/provider run 3,171 passed and 24 skips; its one failed explicit Windows pause-retry expectation exposed a compatibility issue that was fixed and verified by the final focused suite.
SQLite/ZIP contract 19 tests passed each under Node and pinned Bun. Large-rowid writes and following updates remain acknowledged correctly.
Rebuilt artifact/native qualification 881 passed, 14 platform/environment skips on the rebuilt macOS ARM artifact. Real paired-client terminal creation, command/output round trip and shutdown passed. All ten current native CI lanes pass: macOS ARM/x64, Windows x64/ARM, Linux glibc x64/ARM, Ubuntu 20.04 floor x64/ARM, and Alpine/musl x64/ARM. These include current main’s real Windows Git Bash regression.

Negative controls reproduce the Ctrl-Z bug, foreground-name failures, canceled-writer behavior and launcher-lifetime/deadline failures against the prior code. Test groups overlap; counts are not added together. The final independent cross-review found no additional concrete issue in the reviewed scope.

ORCA_BACKGROUND_LAUNCH=1 pnpm build:orcad
ORCA_BACKGROUND_LAUNCH=1 pnpm test:bun:profile --artifact
pnpm tc
pnpm run check:code-quality:changed
  • Local behavior and automated regressions exercised.
  • Runtime, database, deployment, packaging and Windows tests added.

Review

Review artifact closure/startup handoff, SQLite adapter, terminal flow control and foreground/child inspection, then deployment groundwork and failure tests. Both externally reported P1 issues are fixed. All twelve Bun review items received a fix or a correction; Node 18 now has an explicit CI gate.

The earlier AVX2 claim does not apply to pinned Bun 1.4.2: its source documents an SSE4.2 x64 baseline, and the downloaded baseline executables match the pinned executable hashes. No runtime-pin change was needed.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer resources copied or translated.

Notes

  • macOS 13 remains unqualified: no host is available. Bun uses system SQLite, so newer macOS tests do not establish that floor.
  • Backup cancellation remains cooperative during a native Bun SQLite call. The shutdown watchdog remains effective; strict native interruption needs a separate process or upstream support.
  • Managed deployment remains unconnected. Legacy subshell PID migration and stronger incumbent ownership proof are prerequisites before connecting it; unproven owners are not signaled automatically.
  • Windows stale-owner and hard-link-filesystem limits from Persist profile state in SQLite with a durable background writer #22612 remain explicit. Abrupt daemon crashes can also leave small owned launch scratch directories.
  • POSIX output backpressure uses asynchronous process discovery because this Bun version exposes no supported terminal-read pause API; synchronous resume remains only for teardown.
  • Ordinary JSON-backend retirement and stable-channel update rehearsal remain separate. Performance measurements remain workload-specific.
  • Tracking, design, research and measurement documents remain local. The only documentation change here corrects existing operational shutdown guidance.

Checklist

  • Small PR — runtime dependency closure and platform support are substantial.
  • Explained behavior, mechanism and alternatives.
  • Visual proof is N/A with reason.
  • Independently reviewed for correctness and performance.
  • Cross-platform, SSH, folder workspace and mixed-version impact considered.
  • Local validation recorded; fresh CI tracked above.

Published stacked ad-hoc from exact 5f12c53a5f: macOS and Windows downloads. Build run passed for both platforms. This supersedes the earlier 20260925075233 build and includes the verified Windows terminal-interrupt correction.

@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch from 561f43f to 9936257 Compare September 24, 2026 11:00
@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch 8 times, most recently from 78fe4dd to c408d55 Compare September 24, 2026 23:06
@OrcaWin
OrcaWin marked this pull request as ready for review September 24, 2026 23:06

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — full coverage of the 121-file diff at c408d55ed6, plus targeted reads of the surrounding callers and three focused specialist passes.

  • Bundled Bun runtime + handoff — handoffToBundledOrcad, the nonce-keyed profile-state readiness preflight, and artifact identity hashing in src/main/orcad/.
  • Bun SQLite driver — bun-sqlite-database.ts, bun-sqlite-statement.ts, node-sqlite-statement.ts, sqlite-integer-reader.ts, and the SyncDatabase driver selection.
  • Windows Bun PTY — job-object ownership (windows-bun-pty-job.ts), the gate child/launch (windows-bun-pty-gate.ts, windows-bun-pty-launch.ts), the FFI native module, and the runtime/flow-control wrappers.
  • Remote orcad deployment — template materialization/verification, libc detection, remote preflight, and the extracted installOrcadBundle lock lifecycle.
  • Build/packaging — build-orcad*.mjs, watcher/process-tree staging, the zip extractor, macOS template signing, electron-builder wiring, and the new ten-lane Bun CI matrix.

I specifically checked the failure modes most likely to matter here and could not substantiate any: the Bun SqliteStatement binding contract (Bun unwraps a single array argument, so the empty-array binding clears stale state as asserted), VACUUM INTO against the pre-created empty temp every production backup caller uses, suspended-process resume/handle accounting in the Windows job lifecycle, and the .version/readOrcadArtifactIdentity hash agreement. Two latent observations were investigated and ruled non-actionable for this change: Bun's VACUUM INTO refuses a non-empty existing target (Node's backup overwrites) but no caller supplies one, and local-pty-foreground-inspection.ts still passes proc.pid as the shell pid even though the daemon's foreground paths were moved to ptyShellProcessId — unreachable today because the local provider runs on node-pty, where the two are identical.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior Pullfrog review at c408d55ed6: commit 8c5e7ead52 (63 files, +1608/-715), re-read against the full PR diff with three focused specialist passes.

  • Removed the deployment template from desktop packages — deleted sign-mac-orcad-template.cjs and its test, dropped out/orcad-template from electron-builder.config.cjs and the build:desktop/build:release scripts, and moved template verification into build-orcad-template.mjs.
  • Added Windows shell-spawn confirmation — windows-bun-pty-spawn-receipt.ts publishes an atomic pid/error receipt, native-pty-spawn.ts awaits proc.waitForSpawn(), and WindowsBunPtySpawnUnconfirmedError suppresses the PowerShell→cmd fallback whenever a shell may already have run.
  • Made POSIX flow control asynchronous and coalesced — reconcile() re-reads the process table off the event loop and aborts it on shutdown, and posix-pty-process-groups.ts moved off execFileSync onto runProcess/runProcessSync.
  • Hardened shutdown signal handling — extracted installOrcadShutdownSignals, made duplicate SIGINT/SIGTERM/SIGHUP no-ops so the final writer drain can complete, and stopped the Windows launcher from hard-killing a child that already received console shutdown.
  • Repaired corrupt orcad cache entries — findOrcadCachePath recovers a verified .repair-N sibling instead of refusing startup; downloads now use the configured HTTP client and a progress-reset stall timer.
  • Shared ZIP extraction and libc fallback — Windows extracts with inbox System32/tar.exe, and libc detection falls back to getconf/musl-loader evidence when ldd is unavailable.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior Pullfrog review at 8c5e7ead52: commit 35fad32e77 ("support BusyBox terminal process discovery"), 3 files, +329/-44, re-read against the full PR diff with one focused specialist pass on the dialect-probe concurrency.

  • Added BusyBox ps compatibility — src/main/pty/posix-pty-process-groups.ts now routes both async and sync reads through processTableQueries, which tries ps -p + ps -t first and, on an unsupported-selector rejection, falls back to a full-host ps -e -o pid=PROCESS_ID,pgid=PROCESS_GID,tty=TERMINAL_DEVICE_NUMBER. The long =HEADER names are load-bearing: BusyBox sizes each column to its header, so device numbers like 136,1000 are not truncated into another terminal's identity.
  • Serialized the first dialect probe — a process-global psDialect cache plus dialectProbe (waitForPromiseWithSignal) collapses concurrent first reads onto one probe, released in finally; a post-read re-check throws UnsupportedPsSelectionError when another caller has already proven the full-scan dialect, keeping the synchronous teardown path and async readers consistent.
  • Tightened the controlling-tty guard — hasControllingTty now also rejects -, 0, and 0,<minor>, and all ps invocations pin LC_ALL=C with bounded timeout/output.
  • Extended coverage — 201 new unit assertions for dialect caching, rejected-selector fallback, unrelated-failure isolation, shared concurrent probes with independent cancellation, and sync/async dialect sharing; the 16 MiB integration flood test gained a readiness/continue handshake.

I independently confirmed the parts most likely to be wrong: BusyBox 1.36.1 rejects -p/-t with ps: invalid option -- 'p' and accepts the full-scan form while widening columns to the header, and procps-ng 4.0.4 accepts both forms. The specialist pass found no probe leak, no cross-root table mix-up (no await between the psDialect === 'all' re-check and psDialect ??= 'selected'), and no path where an unrelated ps failure (timeout, truncation, signal, permission, or a different rejected option) triggers the full-host scan. The focused unit file passes 36/36.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior Pullfrog review at 35fad32e77: commit 1f3ea8066a ("recover interrupted Bun terminal resumes"), 5 files, +328/-31, re-read against the full PR diff with one focused specialist pass on the new flow-control state machine.

  • Added a resume-retry state machine — bun-pty-process-flow-control.ts replaces the boolean paused with running/paused/uncertain, tracks groupResumeRequired from actually delivered SIGSTOPs, and retries a failed resume every 500 ms on an unref'd timer until a fresh process-table read can release every group. The root pgid is sorted last, so the shell is never resumed ahead of a job group that failed.
  • Preserved the resume obligation through partial failures — a non-ESRCH signal failure inside a resume sweep aborts the sweep before the root and schedules the retry, so a partially stopped tree is never left half released; the fallback now throws instead of root-resuming when groups are required.
  • Cancelled pending work on natural exit — emitExit calls resumeForShutdown() right after exited = true, aborting an in-flight ownership lookup and clearing a queued retry so a late SIGSTOP/SIGCONT cannot target a dead or recycled pid; the Windows job is no longer resumed after exit.
  • Extended tests — the 16 MiB integration flood test now runs with and without a rejected first resume, and new unit coverage exercises retry-on-transient-failure, timer cancellation on pause/shutdown/exit, bounded retries with recovery, root-only suspension, partial-resume re-pause, and natural-exit cancellation.

I traced the synchronous state writes against the async read chains and dispatched an independent pass on the same question; no interleaving was found that loses a SIGCONT obligation, emits a spurious SIGSTOP after resume, or leaves a retry timer firing after exit. The two changed test files pass 39/39 here.

ℹ️ A stopped job group can outlive the shell if it exits mid-resume-retry

emitExit sets exited = true before calling resumeForShutdown(), so the !isExited() guard deliberately skips SIGCONT on natural exit. That is the right trade when the pid may be dead or recycled, and it is not a regression — before this commit no release was attempted on natural exit either. The narrow tail is that a job group successfully SIGSTOPped while a resume retry is still pending (state === 'uncertain') is never released once the root exits. No action requested; noting it only because the retry state machine is new here.

Technical details
# Natural-exit boundary and stopped groups

## Affected sites
- `src/main/daemon/pty-subprocess/bun-pty-process-runtime.ts:68` — `exited = true` is set before the shutdown call
- `src/main/daemon/pty-subprocess/bun-pty-process-runtime.ts:69` — `producerFlowControl.resumeForShutdown()`
- `src/main/daemon/pty-subprocess/bun-pty-process-flow-control.ts:193` — `if (!options.isExited() && state !== 'running')` gates the SIGCONT

## Required outcome
- None. This is informational: confirm the natural-exit no-SIGCONT behavior is the intended trade-off for the new resume-retry state machine, given a stop may have been applied to job groups while a resume retry was outstanding.

## Open questions for the human
- Is there an owned-cleanup path (or an existing tree teardown) that already reaps stopped job groups when the root shell exits naturally, or is that expected to be left to the OS?

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior Pullfrog review at 1f3ea8066a: commits 258c4af954 ("confirm shell suspension before pausing terminal jobs") and 381bb9eadf ("synchronize the Bun terminal fallback fixture"), 6 files, +248/-55, re-read against the full PR diff with one focused specialist pass on the new pause-confirmation state machine.

  • Made pause confirm shell suspension — pause() now sends SIGSTOP to the root synchronously (via numeric os.constants.signals values) and reconcile() only suspends the owned job groups once isPosixPtyRootStopped() proves the root's ps stat begins with T; otherwise the jobs are deliberately left running and state is still recorded as paused. resume() now defers its SIGCONT to a fresh ownership read instead of signalling immediately.
  • Reordered SIGSTOP delivery — signalPosixPtyProcessGroups now signals the shell PGID before the job PGIDs for SIGSTOP (SIGCONT keeps the root last) and aborts the sweep before touching jobs when the shell's own SIGSTOP fails (EPERM throws, ESRCH returns).
  • Added stat to the process-table read — both the selected (ps -p/ps -t) and full-host forms now request the state column, and isPosixPtyRootStopped parses it.
  • Synchronized the Bun flood fixtures — the "discovery unavailable" integration case gates its second flood on a file the test writes so the stability window is meaningful, and the 16 MiB flood fixture gains a stopSignalGapMs dimension that gives Bash time to react between per-group SIGSTOPs.

I could not substantiate a new defect. I verified empirically that BusyBox 1.36.1 accepts stat=PROCESS_STATE on its full-scan form (widening the column to the header, reporting T for a SIGSTOPped process) while still rejecting -p/-t, and that procps-ng 4.0.4 accepts both; the three changed unit files pass here (41/41, 19/19, 24/24). The specialist traced every pause/resume/resumeForShutdown/natural-exit/read-resolution interleaving and found no lost SIGCONT obligation, no spurious stop after a resume, and no resume-retry firing after exit or shutdown.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior Pullfrog review at 381bb9eadf: commit 277c13e055 ("tolerate delayed Windows fixture handle release"), 1 file, +3/-3, test-only.

  • Tolerated late Windows handle release in the Bun terminal fixture — bun-pty-process.integration.test.ts now imports removeTreeSync from src/shared/windows-transient-lock-removal.ts and calls it in runTerminalScript's finally instead of a bare rmSync(directory, { recursive: true, force: true }). On non-Windows it is behaviorally equivalent (same options, no extra retries); on Windows it retries the EBUSY/ENOTEMPTY/EPERM window that a just-exited child or AV scanner can hold.

No production code changed, so there is nothing to re-verify beyond the earlier flow-control review; this commit only hardens test teardown for a failure mode whose assertions had already passed.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@OrcaWin
OrcaWin force-pushed the OrcaWin/np-async-profile-persistence branch from 60cc397 to 6b28239 Compare September 25, 2026 01:17
@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch 2 times, most recently from db5a835 to 9a2596c Compare September 25, 2026 01:33

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at 277c13e055: the branch was rebased onto a base that already carries bundled ripgrep (#22396), plus one new commit db5a8350c7 ("integrate bundled search with Bun artifact verification"), 10 files, +180/-51. Re-read against the full 139-file PR diff with one focused specialist pass on the artifact-identity and exec-bit question.

  • Aligned build and runtime artifact identity — computeOrcadFullVersion now prepends orcadArtifactHashPrefix(target) and appends the optional agent-browser bytes, matching readOrcadArtifactIdentity; build-orcad.mjs passes the exact BUILD_TARGET it writes to .build-target and the same browser-name inputs, so the build's .version and the runtime identity can no longer diverge.
  • Restored executable bits after remote upload — installOrcadBundle now runs an executablePermissionsCommand that chmods bun-runtime and ripgrep/**/rg unconditionally and each of the glibc/musl agent-browser-* binaries when present, because SFTP drops modes. Windows hosts skip it. The materializer and template build mark the same files executable on assembly.
  • Integrated ripgrep into the orcad template/materializer — the template build copies the POSIX rg binaries with the executable bit, and artifactSources marks bun-runtime and /rg executable, consistent with the standalone build.
  • Covered the identity contract with a cross-check test — new it.each(ORCAD_BUN_TARGETS) asserts computeOrcadFullVersion === readOrcadArtifactIdentity for all eight targets, with and without the browser, and that browser bytes change the hash.
  • Dropped a now-redundant static-binary guard — verify-linux-glibc-floor.cjs no longer wraps objdump -T in a "not a dynamic object" catch, since readImportedSymbols only runs when !isStatic && !isMuslTarget; a static bundled ripgrep short-circuits before it.

The one residual observation — requiredArtifacts(true) passes bare 'win32', so orcadArtifactFilenames omits windows-process-tree.node — is unreachable because managed Windows SSH deployment is unsupported and orcad never loads that addon, so it is not raised as an issue.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch from 9a2596c to 78a529a Compare September 25, 2026 02:50
@OrcaWin
OrcaWin force-pushed the OrcaWin/np-async-profile-persistence branch from 9622ea0 to 1b2e701 Compare September 25, 2026 02:50
@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch from 78a529a to 1a07016 Compare September 25, 2026 02:53

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at db5a8350c7: commit 78a529ac56 ("test: qualify headless shutdown and admission on every Bun target"), 1 file, +2/-0.

  • Qualified shutdown and admission tests under the bundled runtime — config/scripts/run-bun-profile-tests.mjs adds src/main/orcad/orcad-entry.test.ts and src/main/orcad/orcad-push-startup.test.ts to the default Vitest file list, so the bounded-shutdown lifecycle and the profile-admission/startup tests now run on every Bun lane (persistence matrix, glibc-floor container, and musl) instead of only under Node.

Both files exist on the head and are self-contained Vitest units: the shutdown test spies process.on/process.exit and drives installOrcadShutdownSignals with fake timers, and the admission test mocks the runtime/entry surface while using only fs-backed acquireProfileStateMaintenance. Neither pulls a native addon or node:sqlite, so adding them to the Bun runner introduces no runtime-specific dependency.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch 3 times, most recently from 07fbaf9 to bcfa5b0 Compare September 25, 2026 05:05

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at 78a529ac56: the branch was rebased onto a newer base (prior head 07fbaf9d37, all 16 prior subjects preserved with rewritten SHAs; net PR LoC moved +9805 → +10435, exactly the new commit's +630 net) plus one genuinely new commit bcfa5b0eda ("Harden Bun terminal fallback and producer pause recovery", 20 files, +757/−127). Re-read against the full 149-file PR diff with two focused specialist passes (flow-control convergence; spawn cancellation/leaks).

  • Generalized the producer transition retry — bun-pty-process-flow-control.ts renames resumeRetry/RESUME_RETRY_MS to transitionRetry/TRANSITION_RETRY_MS, retries pause as well as resume (state === (pauseRequested ? 'paused' : 'running')), and moves the root SIGSTOP out of pause() into reconcile() so a failed root stop also reschedules.
  • Required proven job ownership before suspending — signalProcessGroup('SIGSTOP', table, true) now refuses the root-only fallback, so a shell proven stopped cannot leave unaccounted job groups running while the pane is paused.
  • Threaded an AbortSignal through the native Windows spawn — spawnNativeDaemonPty checks the signal before each attempt and after waitForSpawn, races the receipt with waitForPromiseWithSignal, destroys the wrapper on abort, and suppresses the PowerShell→cmd fallback when the spawn was canceled.
  • Extracted runtime spawn selection — new local-pty-runtime-spawn.ts picks node-pty or the Bun adapter once, so the in-process fallback uses the bundled runtime on POSIX and the gated native wrapper on Windows; local-pty-launch-plan.ts skips the node-pty spawn-helper chmod when Bun owns the PTY.
  • Serialized and cancelable same-session launches — new reserveLocalPtySpawn chains same-id spawns so a late Windows shell receipt cannot start a duplicate shell; runCancelableLocalPtySpawn replaces the canceled boolean with an AbortController whose signal is combined with the client's disconnect signal via AbortSignal.any.
  • Retired sessions that exited before the spawn reply — activateLocalPtySession flags a synchronous onExit, disposes the exit listener, skips startup-command delivery, and reports exitedBeforeSpawnReply so a Bun shell that exits before activation is not served.
  • Extended Windows shell identity to the Bun wrapper — processNameIsSpawnFile marks the wrapper's spawn-file name; inspectLocalPtyChildProcesses reads job membership for those processes, and cwd / foreground / signal operations resolve through ptyShellProcessId (gated shells signal proc.signalProcess).
  • New tests — a pause-retry state-machine unit file, pending-native-spawn cancellation/reservation tests, a Windows identity test, and an isolated esbuild + copied-Bun integration test proving the in-process fallback spawns, reattaches and retires a shell without node-pty; the Bun CI matrix now triggers on src/main/providers/** and runs that integration test.

I ran the seven changed unit files against HEAD (73 tests) — all pass. Two independent specialist passes found no reachable leak, double-serve, or hang: POSIX spawns are synchronous (so the un-threaded signal has no cancellation window), the Windows wait is bounded and destroys the wrapper on abort, and the flow-control invariant state === 'uncertain' ⇒ root stopped and not resumed makes the root-running/uncertain combination unreachable. One residual observation was considered and not raised: a pane whose process groups stay permanently unreadable now retries the pause transition every 500 ms rather than settling at paused (pre-commit code did a root-only stop fallback). That is the commit's stated "retain the obligation until ownership is confirmed" behavior, the root is still stopped, and the loop is bounded by the caller's pause failsafe and cleared on resume/shutdown/exit.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch from bcfa5b0 to e1b1074 Compare September 25, 2026 05:37

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the single new commit since the prior Pullfrog review at ea0553961d.

  • Hardened the Git Bash foreground test against a fork race — agent-foreground-process-git-bash.win32.test.ts (Windows-only) replaces the bare proc.write('sleep 60\r') foreground probe with node -e "…setInterval…" that prints ORCA_FOREGROUND_READY, waits for that marker in the captured output before asserting confirm() is false, and only then sends \x03. The previous ordering could deliver the interrupt while the shell was still forking the child, which is the flake this commit removes.
  • Added assertion labels — 'initial prompt' and 'prompt after interrupt' on the two confirm() assertions for clearer failure output. Diagnostic only.

No production code changed. The unchanged sleep 60 & negative check still covers background-child detection.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at 416853d08f: commits 8b06cdb3ac and a2040fb0b1, 4 files, +47/−6, re-read against the full 165-file PR diff.

  • Kept the Windows Bun gate alive through console interrupts — windows-bun-pty-gate.ts installs a no-op process.on('SIGINT') listener on win32 before waitForGate and removes it in finally, so a Ctrl-C delivered to the console no longer terminates the process supervising the shell's job.
  • Covered the listener lifetime with a unit test — windows-bun-pty-gate.test.ts asserts the SIGINT listener count is +1 while the gate runs and exactly restored on both the child-exit and child-error paths.
  • Asserted terminal survival in the real Git Bash test — agent-foreground-process-git-bash.win32.test.ts now requires dead === false alongside the restored prompt after the interrupt, so the qualification catches a gate that died rather than only a lost prompt.
  • Reused the transient-lock cleanup in the Bun launcher fixture — orcad-bun-launcher.integration.test.ts teardown calls removeTreeSync instead of rm with retries; on non-win32 that is rmSync({recursive, force}) with the same effective semantics, and on Windows it absorbs the late handle-release window.

I verified the gate unit file passes here (7/7). The listener is scoped to the win32 gate worker's supervision lifetime, removed on every exit path, and does not affect parent-initiated teardown (a Windows child.kill() is TerminateProcess, not a console event); the 30 s waitForGate deadline bounds the only window where an interrupt is ignored before any child exists. No specialist pass was warranted for a change this narrow.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

m4air added 25 commits September 25, 2026 01:56
@OrcaWin
OrcaWin force-pushed the OrcaWin/np-bun-profile-persistence branch from 3396618 to 41a3b27 Compare September 25, 2026 08:56

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at a2040fb0b1: a rebase (all prior commit subjects and per-commit stats preserved; a2040fb0b1 → 41a3b27c33, 8b06cdb3ac → 95ed2068f3) plus one test-only commit 5f12c53a5f ("test: tolerate Windows PTY artifact cleanup locks"), 1 file, +3/−2.

  • Tolerated late Windows handle release in the Bun in-process PTY artifact test — src/main/providers/local-pty-bun-artifact.integration.test.ts teardown now imports and calls removeTreeSync from src/shared/windows-transient-lock-removal.ts instead of await rm(directory, { recursive: true, force: true }). On non-win32 it is rmSync({ recursive, force }) with the same effective semantics; on Windows it retries the transient EBUSY/ENOTEMPTY/EPERM handle-release window (8 × 150 ms plus an outer loop for handles that outlive those attempts). No production code changed, and this is the same helper pattern already applied to orcad-bun-launcher.integration.test.ts in 95ed2068f3.

No open Pullfrog review threads remain (the lone prior inline observation on orcad-lifecycle.ts is resolved).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

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