fix(install): verify release assets exist before trusting advertised version; test isolation fixes#404
Merged
Merged
Conversation
The v0.44.0 release failed on an order-dependent test flake:
- init-project.test.ts ("vscode returns auto-installed") exercised the
real configureVscodeSettings with mocked credentials, writing the
marketplace URL into the runner's REAL ~/.config/Code/User/settings.json
(and developers' real VS Code settings on local runs).
- vscode-settings.test.ts overrode HOME per-test, but Bun caches
os.homedir() on Linux, so the override was silently ignored and the
tests read/wrote the real (now polluted) settings file.
- Bun's test-file execution order is non-deterministic across runs, so
the failure only appeared when init-project.test.ts happened to run
first - the same tree passed CI on the release PR and failed on main
13 minutes later.
Fixes (test-only, no production changes):
- init-project.test.ts: spy out configureVscodeSettings so initProject
never writes user-scope state.
- vscode-settings.test.ts: replace HOME env overrides with
spyOn(os, "homedir").mockReturnValue(tempDir) - verified the spy
reaches named imports in other modules. Keep the APPDATA env override
for the Windows branch, which reads Bun.env at call time.
- vscode-settings.test.ts: restore saved env by deleting originally
unset keys instead of Object.assign stringifying undefined.
- ARCH-005: capture both rules (mock os.homedir(), never write real
user-scope state in tests).
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Sweep of all test files overriding HOME/APPDATA/XDG_* envs, auditing each against what the production code actually reads: - env overrides are SAFE for consumers of paths.ts resolvers (archgateHomeDir, opencodeConfigDir, opencodeDbPath, cursorUserDir, copilotSessionStateDir) and upgrade.ts/install-info.ts - all read Bun.env.HOME / USERPROFILE / XDG_* at call time. No changes needed for clean, upgrade, auth, binary-upgrade, credential-store, sentry, telemetry*, update-check, opencode-settings, plugin-install-cleanup, session-context-opencode tests. - session-context.test.ts / session-context-cursor.test.ts worked around Bun's homedir() caching by writing into the REAL ~/.claude/projects and ~/.cursor/projects with unique names. Now spy os.homedir() into a mkdtemp tempHome instead (readClaudeCode- Session/readCursorSession call homedir() at call time). - session-context-copilot.test.ts created real ~/.copilot/session-state and tracked created dirs for cleanup. Now overrides Bun.env.HOME to a mkdtemp tempHome (copilotSessionStateDir reads Bun.env at call time). - plugin-install.test.ts was the worst case: installCursorPlugin / installOpencodePlugin tests mocked fetch and Bun.spawn but NOT the filesystem prep in installEditorPluginBundle - every test run created dirs under and DELETED archgate-* agents/skills from the developer's real ~/.cursor and ~/.config/opencode. Now redirects Bun.env.HOME + XDG_CONFIG_HOME to a mkdtemp tempHome file-wide. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…-install tests
mock.module("../../src/helpers/platform") is process-global: it replaces
the whole platform module (only resolveCommand exported, everything else
undefined) for every test file in the run and is not undone by
mock.restore(). ARCH-005 forbids mock.module on first-party modules for
exactly this reason - it is the same order-dependent flake class as the
user-scope pollution fixed in the previous commits.
Convert to the documented pattern: import * as platform + per-test
spyOn(platform, "resolveCommand") installed in beforeEach and restored
via mock.restore() in afterEach. The spy reaches the named import inside
plugin-install.ts through Bun's live bindings.
Full suite passes, confirming no other test file depended on the leaked
global mock.
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
mock.restore() already restores all spyOn spies, but restore it explicitly for symmetry with spawnSpy and clearer intent. (CodeRabbit review feedback on PR #401.) Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…tall-cleanup
CI failed after the previous commit removed plugin-install.test.ts's
mock.module("platform"): plugin-install-cleanup.test.ts ALSO globally
mocked the platform module (and paths). On Linux file order the cleanup
file runs first, its leaked module mock is what plugin-install's new
spyOn wrapped and "restored" to, and platform.test.ts / doctor.test.ts
then saw a bare mock returning undefined instead of the real
resolveCommand. The two competing mock.modules had been masking each
other; removing only one exposed the other.
Convert the last first-party mock.modules to the ARCH-005 patterns:
- platform: per-test spyOn(platform, "resolveCommand") in beforeEach,
restored in afterEach.
- paths: no mock at all - cursorUserDir(), opencodeConfigDir(), and
internalPath() read Bun.env.HOME / XDG_CONFIG_HOME at call time, so
the existing tempDir env override (now including HOME, saved and
restored per-test) isolates the real resolvers. Drops the
cursorDirOverride indirection.
- env restore now deletes originally-unset keys instead of assigning
undefined.
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…version The static version endpoint (cli.archgate.dev/version.json) is deployed by Cloudflare when the release PR merges to main - before the GitHub release is created and 10-15 minutes before release-binaries.yml uploads the platform assets. During the v0.44 incident the release job failed after the merge, so the endpoint advertised a version with no installable assets and both install scripts failed. Harden version resolution in install.sh and install.ps1: - Verify the platform asset actually exists (HEAD request) before trusting any advertised version - On a miss, warn and fall back to walking the 10 most recent GitHub releases (newest first), installing the newest one whose asset exists - releases/latest alone is not enough since a release is published before its assets finish uploading - Pinned ARCHGATE_VERSION now fails fast with a clear error when its asset is missing instead of dying mid-download - Strip CR from jq/grep output in install.sh: jq on Windows Git Bash emits CRLF, which left a carriage return on every parsed tag and broke the release walk Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…arse Resolves the ARCH-010 prefer-bun-json warning in mergeCursorHooks. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
- version.json is deployed before release assets exist (v0.44 incident) - jq on Windows Git Bash emits CRLF line endings - review-context misses uncommitted changes when a base ref is detected (tracked in #403) Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR enhances installer robustness by validating that platform-specific release assets exist before download, documents associated patterns and the test isolation policy, tightens Copilot session test coverage to ensure the missing-directory error path is exercised, and simplifies JSON parsing in the plugin install helper. ChangesInstaller robustness and documentation
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying archgate-cli with
|
| Latest commit: |
724041b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://52b8a08a.archgate-cli.pages.dev |
| Branch Preview URL: | https://fix-vscode-settings-test-pol.archgate-cli.pages.dev |
Contributor
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/helpers/session-context-copilot.test.ts (1)
229-236:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest intent no longer matches setup for missing-directory behavior.
Line 229’s case says “directory does not exist,” but
beforeEach(Line 29) always createsstateDir, so this path is never exercised. RemovestateDirinside this test before callingreadCopilotSession()to validate the intended branch.Suggested fix
test("returns error when session-state directory does not exist", async () => { + rmSync(stateDir, { recursive: true, force: true }); const result = await readCopilotSession( "/nonexistent/path/that/wont/match" ); - // This may return "no sessions found for this project" or "no session-state dir" - // depending on whether ~/.copilot/session-state/ exists expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("No Copilot CLI session-state directory found"); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/helpers/session-context-copilot.test.ts` around lines 229 - 236, The test "returns error when session-state directory does not exist" currently never exercises the missing-directory branch because beforeEach creates stateDir; modify the test to remove or unset that directory before calling readCopilotSession (e.g., delete the stateDir or set stateDir to a non-existent path) so readCopilotSession("/nonexistent/path/that/wont/match") truly hits the "no session-state dir" behavior; update the test to reference the same stateDir variable used in beforeEach and ensure cleanup is performed so subsequent tests are unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@install.ps1`:
- Around line 63-65: The empty catch block in install.ps1 should log the caught
error for debuggability while still falling through to the GitHub API; update
the catch associated with the preceding try (the block that currently just
comments "Fall through to GitHub API") to emit a verbose/debug message (e.g.,
via Write-Verbose or Write-Debug) that includes the exception ($_ or
$_.Exception.Message) and context about what operation failed, then continue to
fall through as before.
---
Outside diff comments:
In `@tests/helpers/session-context-copilot.test.ts`:
- Around line 229-236: The test "returns error when session-state directory does
not exist" currently never exercises the missing-directory branch because
beforeEach creates stateDir; modify the test to remove or unset that directory
before calling readCopilotSession (e.g., delete the stateDir or set stateDir to
a non-existent path) so readCopilotSession("/nonexistent/path/that/wont/match")
truly hits the "no session-state dir" behavior; update the test to reference the
same stateDir variable used in beforeEach and ensure cleanup is performed so
subsequent tests are unaffected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad936983-442f-421d-9502-db3c76c93c64
📒 Files selected for processing (13)
.archgate/adrs/ARCH-005-testing-standards.md.claude/agent-memory/archgate-developer/MEMORY.md.claude/agent-memory/archgate-developer/feedback_no_prod_changes_for_tests.mdinstall.ps1install.shsrc/helpers/plugin-install.tstests/helpers/init-project.test.tstests/helpers/plugin-install-cleanup.test.tstests/helpers/plugin-install.test.tstests/helpers/session-context-copilot.test.tstests/helpers/session-context-cursor.test.tstests/helpers/session-context.test.tstests/helpers/vscode-settings.test.ts
…n.json fallback Address CodeRabbit review on PR #404: - session-context-copilot: beforeEach always created stateDir, so the 'directory does not exist' test never hit that branch. Remove the dir inside the test and assert the specific error message. - install.ps1: log the version.json lookup failure via Write-Verbose instead of swallowing it silently before falling back to the GitHub API. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Merged
rhuanbarreto
pushed a commit
that referenced
this pull request
Jun 10, 2026
# archgate ## [0.45.1](v0.45.0...v0.45.1) (2026-06-10) ### Bug Fixes * **install:** verify release assets exist before trusting advertised version; test isolation fixes ([#404](#404)) ([be5e7d9](be5e7d9)) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Access Restrictions The command can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - JSON must be valid, otherwise the command will be ignored - Parameters apply only to the current release execution - The command can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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
1 participant
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
Two areas of work bundled on this branch:
1. Installer hardening (v0.44 incident follow-up)
docs/public/version.jsonis deployed by Cloudflare when the release PR merges to main - beforerelease.ymlcreates the GitHub release and ~10-15 min beforerelease-binaries.ymluploads platform assets. During the v0.44 incident the release job failed after the merge, so the endpoint advertised a version with no installable assets and both install scripts failed with a download 404.install.shandinstall.ps1now:releases/latestalone is not enough, since a release is published before its assets finish uploadingARCHGATE_VERSIONwith missing assets now produces a clear error pointing at the releases pageinstall.sh: jq on Windows Git Bash emits CRLF, which left a carriage return on every parsed tag and silently broke the release walkAlso fixes the ARCH-010
prefer-bun-jsonwarning insrc/helpers/plugin-install.ts(last remaining check warning).2. Test isolation fixes (earlier commits on this branch)
Stops tests from touching real user-scope directories and replaces first-party
mock.moduleusage withspyOn(vscode-settings, plugin-install, session-context, init-project).Testing
bun run validatepasses (1262 tests, 39/39 ADR rules, 0 warnings)install.sh: POSIX syntax check, live resolution test, simulated missing-asset fallback (resolves next installable release), pinned-bogus-version error path, full e2e install into a temp dirinstall.ps1:Parser::ParseFileclean, ASCII-only verified (ARCH-021), same resolution scenarios exercised in isolationRelated
review-contextmisses uncommitted working-tree changes when a base ref is detected (found while validating this change)Summary by CodeRabbit
Bug Fixes
Documentation