[nwparker]

	Files	Added	Deleted	Net
Test	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​158	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​157
Prod	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​44	$\color{#cf222e}{\Huge{\mathbf{−}}}$​5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​39

ELI5

Stale overlay entries could direct cleanup outside the owned directory or through a replaced parent. Cleanup now checks the owned boundary and rejects redirected parents.

What Changed

Remove only entries recorded in the overlay manifest, after checking that their parent directories still belong to Orca and are not symbolic links. Retain the complete original cleanup contribution on main; the startup and manifest prerequisite from #24762 has merged.

Why

Checking each recorded entry preserves runtime-created directories and the user's source configuration. Replacing the entire overlay would lose those distinctions.

Linked Issue

Focused contribution from #13287, following merged #24762. #13287 remains open for its other work.

Visual Proof

N/A — background filesystem cleanup adds no rendered controls. The retained combined OpenCode startup evidence is linked below.

Testing

• Installed OpenCode exercised in the retained before/after runs.
• Automated tests cover the cleanup boundaries.

On exact source a7540059, 169 tests across 14 files, all three desktop typechecks, full owned TypeScript lint, the configured repository quality scan and formatting pass. Formatting checked 116 eligible files; four timing records are excluded by the existing formatter configuration. Recorded test processes closed. Tests use private ephemeral loopback ports; the process receipt is not an exhaustive endpoint inventory.

Installed OpenCode 2.0.16 reproduced lost startup authorization and incorrect inherited configuration in the retained earlier runs. The repaired combined c8fdfdbe app completed fresh and nested Unicode startup turns, used one combined status/prompt overlay, claimed each brief once, preserved the original user configuration and reported canonical working → done. Its genuine worker selected the app's private CLI and completed worker_done exactly once. All 319 recorded process incarnations and four private ports closed. Screenshots and actual evidence remain pinned to that source on main 2b1acd0d. This publication has its own static checks and does not claim an app run on the current commit. Fresh public CI is pending.

Review

Two independent reviews checked all 124 retained files and every other main file. The normal merge retains predecessor and contributor ancestry; its resulting tree is main e8310d5a plus the five cleanup files. The two owned conflicts select the merged startup lifecycle and the reviewed cleanup service. Main c4e8735f adds 21 files with no overlap in this scope, so CI will test the combined merge.

Agent skill upstream boundary

• Not applicable; no upstream skill-installer material is copied.

Notes

Credit Adnan Khan (@akhan157). Retain the complete original cleanup scope and author ancestry, including its earlier reader prerequisite. Local, SSH and folder workspaces continue to use the existing execution-host cleanup boundary; no remote protocol changes are introduced.

Checklist

• Scope retained and focused on this layer.
• Explained the user-facing change, mechanism and approach.
• Visual/interaction evidence identified above, or N/A with reason.
• Self-reviewed for correctness, security and performance.
• Cross-platform, folder workspace and SSH execution boundaries considered.
• Appropriate tests, typechecks, formatting and changed-line checks pass; fresh public CI is pending.

[pullfrog]

✅ No new issues found.

Reviewed changes

This run is scoped to what changed since the prior pullfrog review (a6879b0): the head merged the declared base into the branch. The PR's own authored layer is unchanged.

• Base merge brings the branch current — d12f6b5e merges the declared base nwparker/OpenCode-Native-Prompt-Intent (tip 85efe85e) into the head. The merge is clean: head vs its base is still exactly the five authored overlay-cleanup files (+202/−6), byte-identical to the previously approved state, so no PR-authored content changed.
• Merged-in location.sync startup hydration — the merge pulls the base's opencode-startup-prompt-source.ts change into the head tree: the plugin now requires ctx.data.location.sync, calls it once at setup, and the delivery poll returns early until locationReady. These files are identical to the base tip and do not appear in this PR's own diff; I re-read both the change and its tests against OpenCode v2.0.16 (location.sync loads location + config; list is a pure read) and found no claim-before-hydration race and no fail-open path.

The confinement logic itself is untouched by this merge and remains as previously verified against adversarial manifest inputs.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Scoped to what changed since the prior pullfrog review (d12f6b5e): the head merged the declared base's new tip 1dc953cf8c into the branch, which advances the base-owned startup-hydration gating. The PR's authored overlay-cleanup layer is unchanged.

• Base merge keeps the layer current — 0dd7371f merges nwparker/OpenCode-Native-Prompt-Intent (1dc953cf8c) into the head. Head vs base is still exactly the five authored overlay files (+202/−6), byte-identical to the previously approved state.
• Startup readiness is now location-bound — the merged-in plugin no longer latches a single locationReady boolean at setup. The poll derives locationKey(location) from ctx.location and only proceeds when it matches readyLocation; on any change it re-invokes ctx.data.location.sync, clears readiness, and re-checks the key after the claim await and again immediately before keymap.dispatch. These two files are identical to base tip, so they are base-owned and outside this PR's own diff.
• New regression tests — four cases pin the behavior: a directory change mid-sync triggers a second sync to the new location and delays the claim until it resolves; no sync before a concrete location exists; a workspace-only change re-syncs; and a claim granted after the composer location changes never inserts or dispatches.

I verified the new ref shape against OpenCode v2.0.16: LocationRef is { directory: AbsolutePath, workspaceID?: WorkspaceID } (packages/schema/src/location.ts) and the location query keys on directory (packages/protocol/src/groups/location.ts), so { directory, workspaceID } is exactly valid. The gating is fail-closed: an unresolved or rejected sync drops the intent, and every asynchronous boundary before dispatch re-checks the location key.

The confinement logic itself is untouched by this merge and remains as previously verified against adversarial manifest inputs.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

This run is scoped to what changed since the prior pullfrog review (0dd7371f): the head merged the declared base's new tip, which is itself a merge of current main. The PR's authored overlay-cleanup layer is unchanged.

• Clean base merge keeps the layer current — 15f3e7ab merges nwparker/OpenCode-Native-Prompt-Intent (89aa00a9) into the head. git show --cc --name-only finds no conflict or evil-merge hunks, so it is a clean auto-merge; head vs the declared base is still exactly the five authored overlay files (+202/−6), and git diff 0dd7371f HEAD for those five files is empty.
• Merged-in main delta stays outside this layer — the ~150-file net delta between the two heads is main's content (browser, claude, ipc, runtime, renderer, shared agent-title). None touch src/main/opencode/ or src/main/pty/, so no module the authored files import changed with the merge.

The confinement logic itself is untouched by this merge and remains as previously verified against adversarial manifest inputs (absolute paths, .. traversal, dangling/intermediate symlinks, Windows junctions).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b9ce5ac2-cef5-4082-a5b8-f696f8b807fd

📥 Commits

Reviewing files that changed from the base of the PR and between c4e8735 and a754005.

📒 Files selected for processing (5)

• src/main/opencode/hook-service-overlay-confinement.test.ts
• src/main/opencode/hook-service.ts
• src/main/opencode/opencode-overlay-manifest.test.ts
• src/main/pty/overlay-mirror.test.ts
• src/main/pty/overlay-mirror.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

Overlay cleanup now checks ancestor paths and refuses removal when an ancestor is a symlink or has an unexpected lookup error. OpenCode overlay setup validates the overlay root, overlay directory, and plugins directory. If validation fails, it returns the source configuration directory. Manifest cleanup uses scoped removal. Tests cover symlinked paths, missing paths, stale plugin entries, and invalid manifest data.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to a7540

The overlay confinement changes are mergeable after normal checks. A pre-existing replacement race remains a separate hardening concern, but this change does not worsen it.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a7540

The change strengthens cleanup boundaries without expanding filesystem authority. A pre-existing directory-replacement race remains, but the reviewed changes reduce exposure to already-replaced directories rather than introduce or worsen that race.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Exploiting concurrent replacement requires control sufficient to replace an overlay ancestor during setup or cleanup. A redirected recursive removal could affect an entire resolved subtree that the application process can delete; redirected mirroring could write outside the intended overlay. Effective exposure is bounded by operating-system permissions, not the intended overlay boundary. The inspected flow does not establish a remote initiation path or an increase in process privileges.

Security Findings and Attack Paths

• observed — Both retained Security findings concern the same cleanup region: ancestor validation is followed by separately resolved path-based removal. Base-to-head comparison shows that this replacement race already existed in recursive removal and overlay writes. The PR adds guards without increasing the inspected callers' authority or exposure, so these retained findings are recorded as pre-existing security facts rather than active PR architecture concerns.
• inferred — A process able to replace a checked parent after validation can redirect subsequent cleanup or mirroring despite the new checks. Stable-link rejection is meaningful counterevidence against an already-replaced directory, but does not prevent this concurrent check/use attack.

Trust Boundaries and Controls

• observed — Unsafe owned directories cause setup to forfeit the overlay and return the original source configuration. Tests assert that replacing each of the overlay root, source overlay, and plugins directory with a link preserves outside sentinels. The production caller propagates the returned configuration path and excludes this host-side setup for WSL launches.

Resilience and Maintainability Implications

• inferred — Manifest ownership limits ordinary cleanup to recorded contributions, but the persistent shared overlay and best-effort transition do not establish atomicity, exclusive ownership during rebuilding, or rollback after interruption. These are existing limitations relevant to maintaining confinement across repeated setup, not demonstrated regressions from this PR.

Hardening Proposals

• proposed — For stronger confinement, evaluate operations anchored to verified directory identities, or explicitly restrict who can replace overlay ancestors. Add concurrent-replacement and interrupted-rebuild tests covering both deletion and writes; additional preflight checks alone should not be treated as atomic protection.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files.
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: keeping OpenCode overlay manifest cleanup within owned directories.
Description check	✅ Passed	The description follows the required template and explains the change, reason, issue context, visual-proof exception, testing, review, upstream boundary, notes, and checklist. It also distinguishes cu…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

✅ No new issues found.

Reviewed changes

This run is scoped to what changed since the prior pullfrog review (15f3e7ab): the head merged the current main tip into the branch. The PR's authored overlay-cleanup layer is unchanged.

• Clean main merge keeps the layer current — a7540059 merges main (e8310d5a4f, the #24762 startup-brief fix) into the head. git show --cc --name-only reports no conflict or evil-merge hunks, so it is a clean auto-merge.
• Authored overlay layer is byte-identical — git diff 15f3e7ab a7540059 -- src/main/opencode src/main/pty is empty. All five authored files, the inherited manifest reader, and every sibling module in those two directories are unchanged from the previously approved state.
• Merged main delta stays outside this layer — the ~219-file net delta between the heads is main's content. None of it touches src/main/opencode/ or src/main/pty/, and the shared modules the authored files import (app-environment, opencode-config-directory, opencode-tui-plugin-install, opencode-plugin-atomic-write) are not in it, so no module this layer depends on changed with the merge.

The confinement logic itself is untouched by this merge and remains as previously verified against adversarial manifest inputs (absolute paths, .. traversal, dangling/intermediate symlinks, Windows junctions).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}