test(server): isolate automation registry on Linux - #671
Merged
Conversation
The stale-registration pressure test only redirected LOCALAPPDATA, which has no effect on Linux. CI therefore published the live bridge in the Linux registry but attempted to seed stale registrations in a missing Windows-style directory. Redirect both supported registry roots to the test temp directory, disable WSL secondary discovery for the test, derive the directory through the production resolver, and restore every environment variable during cleanup. This keeps the test hermetic on Windows, Linux, macOS, and WSL. Validated with the eight automation-plugin tests repeated twenty times, the server TypeScript check, and git diff checks. The original Linux CI failure was reproduced from both failed #647 runs.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33866655912 Artifacts expire in 7 days.
|
pascalandr
commented
Sep 5, 2026
pascalandr
left a comment
pascalandr
left a comment
Contributor
Author
There was a problem hiding this comment.
Gatekeeper review of fc206729
No blocking code findings remain.
The change isolates the failing stale-registry case across Windows, native Linux, and WSL by pinning both LOCALAPPDATA and XDG_RUNTIME_DIR, clearing WSL_DISTRO_NAME, deriving the production-selected registry directory, and restoring every environment variable in finally.
Validation
- PR Build Validation is green on Linux and Windows, including the complete server test suite and the Tauri crate.
- PR artifact builds are green on every required desktop target.
- The branch contains one focused test-only change and
git diff --checkis clean.
Verdict
Gatekeeper-approved. Repository policy still requires an approving review from a reviewer with write access; that is the only remaining merge blocker.
This was referenced Sep 5, 2026
pascalandr
added a commit
to pascalandr/CodeNomad
that referenced
this pull request
Sep 6, 2026
Complete NeuralNomadsAI#676's worker-only fix by reading native geometry before any client-state mutex and queueing only the latest capture per known window. Move/resize/DPI and zoom handlers no longer wait on atomic publication, and restore seeds clamped normal bounds before maximization. Ownership validation also leaves the in-memory mutex free during disk reads. Merge captures under the serialized writer before rollback snapshots. Preserve admission during tentative clear, disable and removal so failed publication cannot lose concurrent events; discard those speculative events after a successful destructive commit. Close admission before draining, retry previously merged but unpublished captures, and retain cross-host fencing until final persistence completes. Keep a single lazy worker and capacity-one queue, but consume callback ownership while idle and release the scheduler mutex before joining. Concurrent stop callers wait for the same drain while late UI submissions return promptly. Add sixteen Rust regressions beyond the original three scheduler tests, an Electron parity test, and an independent macOS ARM64 CI test gate. Windows validation passes all 158 Rust tests at default parallelism, all 190 Electron tests, UI/Electron typechecks and a release build. The same source-extraction harness that reproduces the old disk/UI lock failures now passes all five assertions over ten repetitions. Linux CI still depends on the separate NeuralNomadsAI#671 registry fix; an interactive macOS smoke test remains necessary.
pascalandr
added a commit
that referenced
this pull request
Sep 6, 2026
Complete #669's compatibility diagnosis without blocking the shared backend while checking a selected executable. Run bounded asynchronous version/help subprocesses and require positive service command evidence, rather than accepting arbitrary exit-zero output. Keep the updater's independent version-only contract and custom version labels unchanged. Recognize legacy root help from successful wrappers as well as failed status commands on host and WSL. Revalidate explicit binary additions even when an older version is cached, and match the launch-error marker precisely so ordinary missing-file and configuration diagnostics cannot be hidden by a matching path. Add real subprocess and HTTP regressions for backend responsiveness, read-only validation, quoted/Unicode paths, stdout/stderr and exit codes. Cover capability parsing, execution failures, updater behavior and unrelated launch errors; add a focused Linux/Windows/macOS ARM64 CI matrix without folding in #671's separate registry fix. Validation: 391 server tests passed with 2 skipped, 189 Electron tests passed, and 79 focused tests passed on Windows. Server/UI/Electron typechecks, production server/UI builds and diff/workflow checks pass. Real V1 1.18.25 is rejected and V2 beta 19151/19192 connects to the existing authenticated service without changing the daemon. Negative controls reproduce the published head's missed diagnostics, cached-version bypass and synchronous probe stall. Interactive Electron/macOS verification remains outstanding.
shantur
pushed a commit
that referenced
this pull request
Sep 6, 2026
## Summary Closes #676. This PR now addresses both mechanisms behind the window-state hang, rather than only reducing the thread count: - one lazy named worker, a capacity-one wakeup queue, and a 250 ms debounce; - native getters run before **any** client-state lock, including explicit close/shutdown captures; - move/resize/scale-factor/zoom handlers enqueue only the latest geometry per known window and never acquire the disk writer lock; - serialized publication merges queued captures, preserves normal bounds across maximize/fullscreen, and seeds clamped restore bounds before maximizing; - rollback preserves events arriving during snapshot/clear/disable/removal publication, including the window whose policy is tentatively changing; a successful destructive commit discards those speculative events; - shutdown closes capture admission, joins the worker without retaining the scheduler/writer mutex, drains an omitted wakeup, and retries captures merged by a failed flush before ownership release; - no application handle retained by the idle worker; - ownership-file validation in `load_window` moved outside the shared in-memory state lock. Electron already debounces native geometry; its production code is unchanged, with a new parity regression covering bursts, maximization, explicit flush, and closed-window cleanup. ## Why the initial version was insufficient The original head `bb38b1a2` fixed thread proliferation, but `capture_window_in_memory` still held `write_lock` across native getters. A slow fsync could block the UI through that mutex; a secondary-thread capture could also hold it while waiting for the same UI thread that was handling a move/resize event. This follow-up retains the useful scheduler behavior and completes the capture/persistence separation in the same PR. No competing #676 PR is being introduced. ## Before/after evidence An external Rust harness compiles the exact production capture, flush, release and scheduler function bodies, replacing only the native/disk adapters. It counts real Windows OS threads and uses controlled channel gates rather than relying on a randomly timed freeze. | Assertion | Base `52f0e629` | Initial PR `bb38b1a2` | Completed PR | | --- | --- | --- | --- | | 128 flush requests | +128 threads | +1 thread | +1 thread | | Capture completes before blocked disk publication is released | FAIL | FAIL | PASS | | UI can complete move/resize while a secondary capture waits for a native getter | FAIL | FAIL | PASS | | Explicit capture/flush retains geometry | PASS | PASS | PASS | | Future envelopes and ephemeral windows remain protected | PASS | PASS | PASS | Ten repetitions per compared version produced the same outcomes. This is a controlled reproduction of the concurrency mechanisms, not a claim of having run the reporter's macOS GUI session. ## Committed regression coverage The crate now includes **19 focused scheduler/capture tests** (the original three plus sixteen additional regressions), using the actual `ClientState`, native-read seam, scheduler and atomic-write adapter. Coverage includes: - 1,000 captures during a stalled disk write; bounded memory and latest geometry; - no client-state mutex held by native getters; simulated secondary getter/UI event cycle; - one worker/one trailing wakeup, callback lifetime, concurrent stop, cheap late submission and no restart after release; - independent windows, maximization/fullscreen, zoom, unknown-window rejection; - successful and failed destructive publication, simultaneous events on the mutating and another window, retry after failed flush; - final persistence and cross-host ownership handoff; - secondary ownership and future-envelope fencing. ## Validation - Full Windows Tauri crate: **158 tests passed**, including default test parallelism. - Electron native suite: **190 tests passed**. - UI/Electron typechecks passed. - Official Tauri resource preparation and server/UI builds passed during verification. - Windows release executable builds successfully. - `cargo fmt --check`, `git diff --check`, and workflow YAML parsing passed. - Added an independent **macOS ARM64 (`macos-26`) Tauri test job** to exercise the crate on the reported platform; packaging now depends on it as well. Its remote result must be checked before merging. ## CI dependency and limits The inherited Linux automation-registry test failure remains isolated in **#671**. #675 was closed as its duplicate and is not part of this fix. No duplicate registry patch or unrelated PR was merged into this branch. After #671 is integrated, update this branch and require the complete matrix before merge. Windows controlled reproductions and tests are not a substitute for a macOS ARM64 interactive launch/move/resize smoke test. Persistent disk failures are still logged and cannot guarantee saving to an unavailable disk. No claim of absolute absence of side effects is made. Maintenance note: `packages/tauri-app/src-tauri/src/client_state.rs` remains oversized (about 1,160 lines); focused capture and scheduler code/tests are kept in sibling modules instead of undertaking an unrelated refactor.
pascalandr
added a commit
that referenced
this pull request
Sep 6, 2026
Update #672 with dev at 81aa4d2, including the integrated #671 registry isolation, #674 POSIX shell selection and #677 window persistence fixes. This brings in the previously missing Linux CI prerequisite without duplicating its patch. Resolve the sole conflict in pr-build.yml by retaining both the OpenCode compatibility matrix and macOS ARM64 Tauri test job, and requiring both before packaging. Every non-build job is structurally identical to its originating parent and every parent build dependency is preserved. No application code needed manual conflict resolution. Validation after integration: 391 server tests passed with 2 skipped, all 192 Electron tests passed, and server/UI/Electron typechecks passed. YAML parsing, exact parent-job comparisons and git diff --check pass. The complete remote matrix must rerun on the combined branch before merging the PR.
pascalandr
added a commit
that referenced
this pull request
Sep 6, 2026
## Summary Closes #669. CodeNomad V2 requires the official OpenCode V2 shared-service lifecycle. A legacy binary must not be accepted merely because `--version` succeeds, nor should opening a folder display a raw legacy CLI help dump. This is an actionable compatibility fix, **not V1 runtime support or automatic installation**: install/select `opencode2` to open the workspace. - Validate the selected executable using only `--version` and `service --help`; neither probe starts or changes the daemon. - Require positive service-help evidence for `start`, `status`, and `get`, without pinning a release number or rejecting custom version labels. - Recognize legacy root help, including ANSI/CRLF, stdout/stderr, and exit-zero wrappers. Preserve the localized V2 diagnosis for previously saved incompatible paths during the actual host/WSL lifecycle. - Run both validation probes asynchronously with a 5-second subprocess timeout and 64 KiB output limit, leaving the shared backend responsive while a CLI is slow. - Treat saved versions as display metadata only: explicit add/browse always validates the path again. - Match the compatibility error code precisely, including its JSON HTTP envelope, so unrelated missing-file/configuration errors are not hidden by a matching filename. - Reuse the existing translated message in all ten locales. No daemon ownership, V1 fallback, package update, or native-shell-selection change. ## Follow-up verification of the original PR The original head `226e2d7b` already fixed the exact reported failure with a real V1 binary. Independent rechecking found and corrected adjacent gaps in this same PR: | Check | Original PR | Completed PR | | --- | --- | --- | | Real OpenCode 1.18.25 rejected; real V2 accepted | PASS | PASS | | Empty/unrelated successful help rejected | FAIL | PASS | | Legacy help with exit zero localized at launch | FAIL | PASS | | Other backend work proceeds during slow probes | FAIL | PASS | | Explicit add rechecks a persisted V1 version | FAIL | PASS | | Unrelated diagnostic mentioning `opencode_v2_required` retained | FAIL | PASS | The new regression tests were run against the old production code first: three assertions failed (false capability acceptance, exit-zero help, unrelated-error masking), then passed after the fix. A separate source-extraction check executes the unchanged published probe and component validation body: two 250 ms subprocess delays block the old probe for about 638 ms, versus a Promise returned in about 5 ms after the fix; a cached V1 version previously produced zero validation calls and now produces one and rejects the path. ## Validation of the compatibility fix (`903d9159`) - Full server suite on Windows: **391 passed, 2 skipped**. - Electron native suite: **189 passed**. - Focused compatibility/lifecycle/updater/HTTP/UI diagnostic suite: **79 passed** on Windows. - Server, UI and Electron typechecks passed; server and UI production builds passed. - `git diff --check` and workflow YAML validation passed. - Real CLI checks: V1 **1.18.25** refused with `opencode_v2_required`; V2 beta builds **19151/19192** accepted; authenticated discovery of the existing shared daemon succeeds. No service stop/restart or settings mutation was performed. - Actual subprocess fixtures exercise spaced/apostrophe/Unicode paths, stdout/stderr and exit codes. HTTP integration proves that validation neither mutates preferences nor blocks another request. - Existing regressions retain stopped/running daemon discovery, startup environment isolation, password-error redaction, deadlines, WSL command forwarding, authenticated health, shared-service ownership and updater behavior. - Independent focused CI matrix passed on **Linux and macOS 26 ARM64** (77 passed, 2 Windows-only skips each) and **Windows** (79 passed), in run `34036886140`. ## Update with current `dev` / conflict resolution Updated this branch with `dev` at `81aa4d24`, which now includes **#671, #674 and #677**. The only textual conflict was in `.github/workflows/pr-build.yml`, where #672 and #677 added CI jobs at the same location. Both the OpenCode compatibility matrix and the macOS ARM64 Tauri tests are retained, with both required by the package-build gate. Parsed workflow comparisons verify that every non-build job is identical to its originating parent and no build dependency was dropped. The previously blocking Linux automation-registry fix **#671 is now included through `dev`**, rather than duplicated. The full CI matrix is rerunning on this combined state; require that result before merge. Local checks after resolving the conflict: **391 server tests passed, 2 skipped; 192 Electron tests passed; server/UI/Electron typechecks passed; workflow parent-preservation assertions and `git diff --check` passed**. The conflict resolution itself changes no application code. Automated CLI/HTTP tests are not an interactive Electron/macOS ARM64 smoke test on the reporter's machine, and cannot establish the absolute absence of every possible side effect. Maintenance: files already touched by the original PR remain oversized: `packages/server/src/api-types.ts` (536 lines) and `packages/ui/src/App.tsx` (855 lines). This follow-up does not grow those files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LOCALAPPDATA,XDG_RUNTIME_DIR, andWSL_DISTRO_NAMEafter the testRoot cause
Both final CI runs for #647 failed in
prunes stale registry pressure before limiting discovery. The test redirected onlyLOCALAPPDATA, which Linux ignores, then wrote fixtures into a hard-coded Windows registry path that did not exist.Validation
node --import tsx --test packages/server/src/opencode/automation-plugin.test.tsnpm run typecheck --workspace @neuralnomads/codenomadgit diff --checkThis fixes the deterministic Linux failure from PR Build Validation runs 33862538051 and 33863984160.