Skip to content

fix(config): wait for background installs on dispose#3

Open
Astro-Han wants to merge 6 commits intodevfrom
fix/config-scoped-deps
Open

fix(config): wait for background installs on dispose#3
Astro-Han wants to merge 6 commits intodevfrom
fix/config-scoped-deps

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

Summary

  • wait for background config dependency installs during config instance teardown
  • cover the Windows config-install:win32 lock leak with a regression test that models disposal plus a follow-up install
  • keep the fix scoped to config lifecycle cleanup, without broad refactors

Root Cause

On Windows, config dependency installs share the global flock key config-install:win32. We started those installs in the background during config loading, but instance disposal did not wait for them to settle. A previous instance could therefore keep holding the lock after teardown, and later tests would time out waiting on the leaked lock.

Testing

  • bun test ./test/config/config.test.ts
  • bun test ./test/tool/registry.test.ts
  • bun test ./test/snapshot/snapshot.test.ts -t "snapshot state isolation between projects"
  • bun test ./test/project/worktree.test.ts

@Astro-Han
Copy link
Copy Markdown
Owner Author

Updated this PR with two follow-up commits:

  • ff8ec43be fix(config): wait for background installs on dispose
  • a201d0a91 fix(config): harden scoped dependency installs

What changed in this update:

  • Reworked config-scoped dependency installs so waiting on the Windows global config lock is interruptible, while the actual install phase remains protected once the lock is acquired.
  • Fixed the disposeAll() path so background config installs waiting on the lock do not hang instance teardown.
  • Restored real failure propagation from Config.waitForDependencies() instead of reducing install failures to warning logs only.
  • Stopped swallowing lock release errors.
  • Aligned the tool.registry caller with the new error semantics.
  • Tightened config tests by isolating OPENCODE_CONFIG_DIR cases from shared global config state.
  • Added and strengthened Windows regressions for:
    • disposing while a background config install is waiting on the win32 lock
    • aborting while waiting on the win32 lock

Re-verified locally after the latest changes:

  • bun run typecheck
  • bun test ./test/config/config.test.ts
  • bun test ./test/tool/registry.test.ts
  • bun test ./test/provider/provider.test.ts -t "plugin config providers persist after instance dispose"

Also ran a fresh-eyes review loop on the final diff. The last review came back with no P0/P1 findings.

@Astro-Han
Copy link
Copy Markdown
Owner Author

Quick status update on this PR.

The scope has narrowed quite a bit. The recent commits have all been in the same area: config-scoped dependency installs, tool discovery timing, and Windows lock cleanup. The latest commit adds a retry in Flock.release() for transient lock-dir removal errors on Windows, plus a regression test for that path. Before pushing, I re-ran bun turbo typecheck and bun turbo test:ci locally on macOS, and both passed.

The current blocker in the latest CI run is on Ubuntu. It fails in packages/opencode/test/tool/registry.test.ts, in the case does not wait for unrelated global config installs before importing local tools with bare imports. That test is specifically checking that local tool discovery does not get blocked by an unrelated global install. In CI it timed out waiting for the local tool id late, so that expectation is still not being met there.

Windows is still running as I post this. bun install --frozen-lockfile and typecheck have completed, and the test step is in progress. Once that job finishes, I will know whether we are looking at the same wait-path on both platforms or one Ubuntu-specific issue plus something separate on Windows.

Assuming Windows does not reveal a different failure first, the next fix should stay narrow: adjust the registry/config wait boundary so unrelated global installs do not stall local tool loading.

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