Skip to content

fix(opencode): release loaded instances before fixture deletion#513

Merged
Astro-Han merged 2 commits into
devfrom
codex/fix-windows-instance-lifecycle
May 9, 2026
Merged

fix(opencode): release loaded instances before fixture deletion#513
Astro-Han merged 2 commits into
devfrom
codex/fix-windows-instance-lifecycle

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

@Astro-Han Astro-Han commented May 9, 2026

Summary

Adds a directory-scoped instance disposal path and uses it before export tests remove loaded session project fixture directories.

Why

The post-merge Windows advisory job still failed in unit-windows-opencode-session with EBUSY while deleting a git-backed temp project directory. The failure was a lifecycle contract bug in the test/runtime boundary: the directory had been loaded into InstanceStore, so directory-scoped resources must be released before the fixture removes it.

Related Issue

No issue. Follow-up to the post-merge Windows advisory failure after PR #511.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please check that disposeDirectory is a no-load disposal path and cannot bootstrap or mutate missing directories just to clean them up.

Risk Notes

Low. This adds a production lifecycle API, but the current call site is test cleanup only. Product export behavior is unchanged. disposeDirectory is intentionally no-load and releases only already-loaded directories; callers must only use it once the directory is no longer needed by the active task. Windows impact is positive because loaded fixture directories are released before deletion.

How To Verify

Red test: InstanceStore suite failed before implementation because store.disposeDirectory was undefined
Instance store tests: 7 pass, 0 fail
Export tests: 32 pass, 0 fail
Combined focused tests: 39 pass, 0 fail
Typecheck: bun run --cwd packages/opencode typecheck passed
Diff check: git diff --check passed
Windows advisory: workflow_dispatch run 25605476612 passed, including unit-windows-opencode-session in 8m31s

Screenshots or Recordings

Not applicable. No visible UI changes.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, primary area, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

  • New Features

    • Introduced directory-scoped instance disposal capability, allowing you to dispose of individual directory instances independently without affecting other loaded instances. This provides more granular control over resource management and complements existing disposal mechanisms.
  • Tests

    • Added tests to validate directory-scoped disposal behavior and ensure the cleanup process works correctly.

Review Change Stack

@Astro-Han Astro-Han added bug Something isn't working windows Windows-specific P2 Medium priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels May 9, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 49 minutes and 49 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 13ac40eb-9458-4c39-8bfa-866364b6cfcc

📥 Commits

Reviewing files that changed from the base of the PR and between 3fe73ae and 521f9d8.

📒 Files selected for processing (1)
  • packages/opencode/test/session/export.test.ts
📝 Walkthrough

Walkthrough

This PR adds disposeDirectory(directory: string) to dispose loaded instances for a specific directory. The capability is implemented in the instance store, wrapped by the runtime, exposed on the public Instance API, and tested to verify it only disposes already-loaded entries without triggering bootstrap of missing directories.

Changes

Directory-scoped Instance Disposal

Layer / File(s) Summary
Interface Contract
packages/opencode/src/project/instance-store.ts
InstanceStore.Interface is extended with disposeDirectory(directory: string): Effect.Effect<void>.
Store Implementation
packages/opencode/src/project/instance-store.ts
disposeDirectory resolves the directory, awaits its deferred load, and removes or disposes the entry depending on load success.
Service Layer Wiring
packages/opencode/src/project/instance-store.ts
The implemented disposeDirectory is included in the service object returned by layer.
Runtime & Public API
packages/opencode/src/project/instance-runtime.ts, packages/opencode/src/project/instance.ts
instance-runtime.disposeDirectory wraps the store call via AppRuntime.runPromise; Instance.disposeDirectory resolves the directory, delegates to runtime, and removes it from the module-level cache.
Tests & Updated Usage
packages/opencode/test/project/instance-store.test.ts, packages/opencode/test/session/export.test.ts
New test verifies disposeDirectory disposes only loaded entries; export tests updated to call Instance.disposeDirectory before deleting directories from disk.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through the disposal lane,
Clearing directories—no bootstrap pain!
Load or absent, the logic gleams bright,
Services cleaned with precise, careful might.
Tests stand guard o'er each cached domain! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(opencode): release loaded instances before fixture deletion' directly and clearly describes the main purpose of the changeset: adding directory-scoped disposal and using it to release resources before fixture cleanup.
Description check ✅ Passed The PR description comprehensively addresses all required sections including Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, and a fully completed Checklist, meeting all template requirements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-windows-instance-lifecycle

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a disposeDirectory method to the instance runtime, store, and public API, enabling explicit disposal of specific project directories and their associated resources. This change is integrated into the test suite to ensure file locks are released before attempting to delete directories. A review comment suggests that reducing the file system removal retry count from 30 to 5 on Windows might be too aggressive and could lead to flakiness in environments with transient OS-level locks.

Comment thread packages/opencode/test/session/export.test.ts Outdated
@Astro-Han Astro-Han merged commit 0b2d670 into dev May 9, 2026
20 checks passed
@Astro-Han Astro-Han deleted the codex/fix-windows-instance-lifecycle branch May 9, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P2 Medium priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions windows Windows-specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant