fix(setup): derive Paseo skill bridge from a discovered source, not a name allowlist

[Yeachan-Heo]

• regression-risk
• high-risk

Problem

On a machine where Paseo is installed as the desktop app (Paseo.app 0.4.0), gjc setup paseo --check could never reach a clean verdict. The skills bridge assumed (a) a compiled-in five-name allowlist and (b) ~/.agents/skills as the only source directory. Neither holds for an app install:

• the app ships skills inside /Applications/Paseo.app/Contents/Resources/skills, so ~/.agents/skills does not exist and GJC created five symlinks into a directory that does not exist;
• the shipped set is paseo-help, not the allowlisted paseo-loop, so every reachable state reported orphan-skill/unlinked-skill drift and exited 1, with no action that makes it pass.

Fix

• Bounded discovery seam (resolvePaseoSkillsSource): ~/.agents/skills first, then a fixed candidate list of Paseo.app bundle paths (/Applications and ~/Applications, darwin only), or an explicit PASEO_SKILLS_DIR override honored only when absolute, present, and absent from every project dotenv variant — never a filesystem search, never a dangling target. Every default home path derives from the provenance-checked trusted home (getTrustedHomeDir()), never raw $HOME.
• Entries derived from the source: every paseo-prefixed directory. A release that adds or drops a skill no longer wedges --check red.
• No dangling links ever: a missing source directory skips the bridge and reports missing-skills-directory once; a fresh install with no source anywhere now completes the provider/orchestration setup and skips the bridge instead of failing the whole install (an existing unowned bridge directory is still refused).
• Install converges the bridge: creates missing links, prunes links whose name the source no longer carries, never replaces a non-symlink, and every destructive bridge operation renames into a GJC-owned quarantine and verifies the captured object post-rename.
• Drift detection stays honest, --remove proves ownership from the recorded provenance ledger, install/remove serialize on one per-agent mutation lock, and a ledger that is corrupt, tampered, escaping, symlinked, or routed through a symlinked ancestor fails every destructive path closed.
• Credential-free GJC state: a --force overwrite preserves the replaced entry (any JSON shape) in a private mode-0600 sidecar beside Paseo's own config — the ledger and intent record carry only the pointer, and --remove restores from the sidecar, failing closed if it is missing.

Review findings addressed (reconstructed onto current dev)

All rounds through the round-8 head are addressed; the round-7 head moved again to answer its six remaining findings in the rebased series:

• Value-free provenance — replaced provider values (which can carry credential-bearing env/argument content) live only in a 0600 sidecar beside Paseo's own config; ledger/intent record the pointer; removal restores from it and fails closed when it is unreadable. The FIRST replaced value is the user's; repeated --force never overwrites the sidecar.
• Trusted home — discovery, ~/Applications candidates, and ~/.paseo config paths resolve through getTrustedHomeDir(), so a planted $HOME cannot steer global setup at attacker content.
• Symlinked ancestors — the recorded bridge path's fully resolved location must satisfy the same accepted shapes as its lexical spelling.
• Missing source is nonfatal — a fresh install with no source and no bridge directory completes the provider setup and skips the bridge; --check still reports missing-skills-directory.
• Snapshot binding — both JSON saga steps refuse when the target's identity changed between preflight and publication.
• Lock coverage — the serialization regression now starts the remove while the install is genuinely blocked mid-bridge-step.

Tests

131 tests in setup-paseo.test.ts (6 new r7 regressions: sidecar-backed restore with value-free ledger bytes; missing-sidecar fail-closed removal; trusted-home defaults under a planted $HOME; symlinked-ancestor refusal; no-source nonfatal install; preflight-identity refusal; genuine lock overlap). The pre-existing provider-config, CAS, backup-safety, intent-recovery, and saga-compensation suites are unchanged and still pass.

Verification run (exact head 8f1cca1, base dev @ 08d0024)

Reconciled onto current dev twice: first after #4730 (18 fix(sdk) ACP commits), then after the herdr/sdk/sentry wave landed (33 commits to 08d002464, zero overlap on setup/paseo, its imports, or the test suite); the PR source is byte-identical across every reconciliation (each is a clean cherry-pick series).

• bun test packages/coding-agent/test/setup-paseo.test.ts packages/coding-agent/test/setup-cli.test.ts — 149 pass, 0 fail
• bun --cwd=packages/coding-agent run check (biome + tsc) — clean
• bun --cwd=packages/coding-agent run build — clean
• bun run ci:test:smoke — ok
• Gates: check-visible-definitions, verify-g002-gates, rebrand-inventory --strict, default-gjc-definitions (33), verify-gjc-state-writers --fail — all pass
• gjc-state-gates groups static/runtime/integrity/read — all pass locally
• git diff --check — clean
• bun test packages/coding-agent/test/setup-paseo.test.ts after the r8–r16 fix-forwards — 155 pass, 0 fail (24 new r8–r16 regressions)
• New-head remote CI is running; independent exact-head GitHub approval remains required by the PR contract

r8 fix-forward (heads 494cb11 + e969378)

The exact-head review at c0502d09 found two ownership defects; both are fixed and regression-pinned:

• Injective, no-clobber sidecar names inside the CAS boundary — the replaced-provider sidecar path now appends a sha256 digest of the exact provider key, so a/b and a_b can never share one sidecar; publication is a hard-link (EEXIST never clobbers — same key + exact value is idempotent, anything else fails closed as a sidecar-conflict refusal), and creation happens in a new persist hook inside runJsonStep, only between a successful publish and the ledger commit (removed again on compensation). A refused publish now leaves no sidecar behind.
• Directory ownership is a recorded fact, not a plan — the provenance pre-write carries only what an earlier run actually created; the bridgeDirCreated creator bit is committed only after the exclusive mkdir in installSkillsBridge succeeded; a fresh no-source run records no bridge ownership at all, so a directory the user later creates at that path can never be mistaken for GJC's work by --remove.
• Default-mode dotenv coverage (e969378f) — the boundary architect independently found that Bun's dotenv mode defaults to development, so .env.development/.env.development.local load even with NODE_ENV unset (the common interactive case), while the PASEO_SKILLS_DIR trust check enumerated only the three NODE_ENV literals. The variant set now carries the development-mode files for every mode except production/test (with .env.production.local always checked), verified empirically against the pinned runtime and pinned by a new regression.
• Sidecar-conflict rollback (e762800e) — when sidecar creation fails after the provider CAS publish already succeeded (a pre-planted file at the injective path), the publication is now reverted through the step's own compensation mutation and the intent record cleared, so the failure never strands an unowned --force overwrite.
• **r19 close-out (8f1cca11) — an in-root recorded bridge path is accepted only when it is the CURRENT configured bridge or a migration record authenticated by CONTENT (the directory must actually hold the recorded links pointing into the ledger's source; a foreign *-paseo-skills sibling with user content is refused); an absent key's sidecar is fd-bound digest-authenticated before deletion; and the intent record's provenance-path confinement is canonicalized through realpath.
• **r18 close-out (81eaaf4c) — a failed adoption restores the ORIGINAL legacy link (the text is captured before the quarantine rename and recreated on failure; a concurrent occupant is preserved); callers omitting home get their legacy source from their own paths.agentsSkillsDir or the trusted home (no more unremovable relative path); and provenanced links re-point across a TRUSTED source relocation — a Paseo update moving the app bundle now converges the bridge instead of conflicting, still gated on the link pointing into the ledger's own recorded source.
• **r17 close-out (4675924f) — quarantined symlink restoration is atomic no-clobber (fs.symlink fails EEXIST against a concurrent occupant instead of the rename clobbering it); unpersist binds to the sidecar's authenticated {key,value} bytes through the fd-bound digest read (a same-sized attacker replacement fails the digest and is preserved — the earlier size-only check both was spoofable and omitted the wrapper); the PASEO_SKILLS_DIR dotenv trust rule is documented with its workaround; and the migration-safe resolver fallback honors a caller-supplied paths.agentsSkillsDir when discovery finds nothing, as the pre-bug(setup paseo): skill bridge can never reach a clean --check against Paseo desktop 0.4.0 (hardcoded allowlist + missing ~/.agents/skills) #4638 seam did.
• **r16 close-out (167771c3) — every providerReplacedEntries reference is validated against the deterministic sidecar path before any deletion (a tampered ledger can no longer redirect --remove's fs.rm at an arbitrary absolute file); unpersist removes only a sidecar whose bytes match what the failed run wrote (a pre-existing sidecar is the user's or an attacker's, never ours); and the no-clobber quarantine restore adds a post-rename identity check (a displaced foreign entry is moved aside to a recoverable name instead of vanishing).
• **r15 close-out (3965b0df) — a concurrent external target change during rollback is a FAILED rollback (the intent record and persisted artifact survive for recovery instead of being cleared beside an unprovenanced write), and quarantined-link restoration is no-clobber (an entry claiming the bridge name in the window is left untouched; the quarantined object stays recoverable at its quarantine name — the no-replace natives refuse symlink sources, so vacancy-check-then-rename is the narrowest available shape).
• **r14 close-out (7ba40f08) — destructive bridge cleanup is confined to the bridge-directory name family and a migration record must be authenticated by the OLD directory's own ledger claiming the exact path (a fabricated paseo/provenance.json shape beside a victim no longer authorizes cleanup); the mutation lock is documented never to displace a provably-live owner (pid + start-time identity; the stale window bounds only the liveness-indeterminate fallback); the persist hook's ATTEMPT is tracked so a sidecar created before a throw is always rolled back; and PaseoPaths.agentsSkillsDir is required again (the shape external callers already constructed).
• **r13 close-out (f2537966) — quarantined bridge-link deletion is bound to the captured inode (dev/ino/size/mtime re-checked immediately before every unlink attempt; any pathname replacement makes the deletion refuse and restore — a foreign object is never unlinked); readIntent validates every field recovery trusts and refuses a recorded provenance path escaping the agent directory; and the skillsSource seam is migration-safe (an omitted resolver falls back to the real discovery order — existing callers keep compiling, no silent bridge skip).
• **r12 close-out (9b5e36e3) — the drift scan canonicalizes the skills source exactly like installation (a symlinked source directory passes --check with no false drift); the recorded-source trust rule mirrors discovery exactly (only ~/.agents/skills, the fixed app-bundle roots, or the live override — an arbitrary home subdirectory a tampered ledger names is refused); and the optional PaseoPaths.agentsSkillsDir field is restored for exported-contract compatibility (the resolved per-run source remains what the ledger records).
• **r11 close-out (8fad904e) — quarantined bridge-link deletion re-verifies kind+text immediately before the unlink, treats an unlink failure as divergence unless the verified link remains, and sweeps any foreign object swapped onto the quarantine name back to the bridge name (never deleting it); discovery records the CANONICAL resolved source and the override's resolved target is trusted, so a symlinked or relocated skills source installs and removes symmetrically; and a replaced-provider sidecar whose entry is already absent is still deleted while any deletion failure fails removal closed (no orphaned credential sidecar, no swallowed errors).
• **r10 close-out (a64fcf64) — a failure between a successful CAS publish and the ledger commit now unpersists the sidecar and reverts the publication (no orphaned credential-bearing file); writeIntent publishes via fsync+rename and a present-but-malformed intent record is an explicit corrupt-record refusal (--check reports it as drift, recovery refuses) instead of reading as absent; non-string record values are ledger corruption; the recorded bridge source is validated against trusted Paseo source roots before any removal unlink; and PaseoPaths drops the optional skills-directory field entirely (the source is resolved per run). The recorded-source trust gate additionally runs in the same pre-settings window as the bridge-path validation (1056da0e), so a tampered source refuses before the skills.customDirectories registration is unregistered.
• r9 five-finding close-out (8f530482) — mid-bridge failures correct the ledger to observed reality (SkillsBridgePartialError carries the partial install result; planned-but-never-created entries stop being claimed); malformed provenance fields raise ProvenanceLedgerCorruptError instead of silently defaulting (absent fields keep old-shape defaults); sidecar reads are fd-bound and symlink-rejecting (O_NOFOLLOW + fstat on the same handle); --check reports exactly one deterministic drift reason per bridge entry; the skillsSource seam is mandatory so an omitted resolver can never silently disable the bridge.

GJC verdict

gajae.pr-review-verdict.v1 needs-human sha256:d7889253edd5610582ba5f782ce8112ffbe2453be582a7da1b1ea926d1b50957 reviewer:critic reviewer-id:terminal-critic evidence:exact-head-8f1cca11-base-08d002464-r19-closed; independent GitHub approval pending

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Yeachan-Heo]

Signed status for this exact head — 4981a92170bde45f881588729fa5356b8aae2499 (base dev @ 6696988b6d), #4644, closes #4638.

Local verification at this head:

• bun test packages/coding-agent/test/setup-paseo.test.ts packages/coding-agent/test/setup-cli.test.ts — 105 pass / 0 fail
• bun --cwd=packages/coding-agent run check (biome + tsc) — clean
• Gates: check-visible-definitions, verify-g002-gates, rebrand-inventory --strict, bun test test/default-gjc-definitions.test.ts — all pass
• bun run ci:test:smoke — ok

Reported failure states were reproduced in isolated temp fixtures pre-fix and each verified reaching its correct verdict post-fix (app install pass; paseo-loop→paseo-help release skew converges on re-run then pass; missing source → single missing-skills-directory, nothing created; provenanced-link deletion still drift; --remove preserves the claude provider entry and the daemon credential, and cleans the bridge even after Paseo is uninstalled).

CI at this head is running; classification once complete. Independent review requested: @probepark @snowykr. The gajae.pr-review-verdict.v1 line will be posted only after an authenticated approving review exists for this exact head — self-approval is BLOCK per the template.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

CI classification at exact head 4981a92170bde45f881588729fa5356b8aae2499 — final, no polling:

• 7 checks green — local public surfaces, gjc-state-gates (integrity/read/runtime/static), and every other content check that completed on this head.
• 1 in flight — Affected path validation / native-build (Rust toolchain; this diff is TypeScript-only).
• 6 skipped — darwin-arm64 tab-worker smoke, live deployed release state, Telegram daemon generation guard, Windows daemon/doctor/native variants gated on runners or paths this change does not touch.
• 2 failures, both expected and not implementation defects: Validate exact-head PR contract and PR contract bootstrap — these require an authenticated approving GitHub review for this exact head before the gajae.pr-review-verdict.v1 line can be posted. Self-approval is BLOCK by the template, so both stay red until @probepark or @snowykr reviews. No code path is implicated; every content check on this head is green.

Local evidence at this head: 105/105 focused tests, package biome+tsc clean, check:ts green, smoke ok, all four rebrand/default gates pass. Signed detail in the earlier comment.

This supersedes my previous comment, whose backticks were corrupted by shell substitution.

Parked pending review/webhook transition.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[probepark]

Independent maintainer review — merge blocked. Three major findings.

major — the users this PR is meant to unwedge cannot migrate

packages/coding-agent/src/setup/paseo/skills-bridge.ts:197-203 + remove.ts:91-102: existing GJC-owned links point at ~/.agents/skills. Under the new discovery the app-bundle target is classified as a foreign conflict, and legacy ledgers carry no bridgeSourceDir, so --remove validates against the newly discovered source instead of the recorded one. Anyone already wedged by #4638 stays wedged unless they hand-edit files. Migrate provenance from the recorded source.

major — setup prunes links it does not own

skills-bridge.ts:214-229 unlinks every paseo* symlink absent from the current source without consulting provenance, explicitly including live user symlinks. Since an existing bridge directory is supported (bridgeDirCreated: false), directory creation cannot establish ownership of every entry. Gate pruning on recorded provenance.

major — filesystem errors collapsed into "absent"

remove.ts:82-104: lstat(...).catch(() => undefined) treats permission/I-O failures as absence, then clears all bridge provenance and reports success while an owned link is still on disk. Preserve the error distinction and fail closed on non-ENOENT.

coverage

The fresh desktop-install tests do fail without the production change, but they never construct the upgrade state the old bug produced, and there is no coverage for unprovenanced links or non-ENOENT filesystem failures.

[Yeachan-Heo]

Signed reconstruction evidence — PR #4644, exact head af4793684f6b620bcc53c2490b7c6a58942208a9 (base dev @ 416201eb5e), closes #4638.

Why the head moved (force-with-lease, authorized branch fix/issue-4638-paseo-bridge):

1. The prior head 4981a921 was based on stale dev 6696988b6d and carried a CHANGES_REQUESTED review (probepark) with three major findings. I reproduced all three on the exact legacy-upgrade state the PR claims to fix (allowlist links into a ~/.agents/skills that never existed, legacy ledger without bridgeSourceDir, desktop app now installed):
   • re-install refused with SkillsBridgeError: conflicting entries — the wedged machine could not converge;
   • --check stayed drift forever;
   • an unprovenanced live paseo* symlink would be pruned by install;
   • an EACCES-class lstat failure collapsed into "absent" and cleared provenance while an owned link remained on disk.
2. The accepted PR scope (original commit 4981a921, authorship Yeachan Heo <yeachan.heo@gmail.com> preserved) was cherry-picked onto current dev, then one fix-forward commit (af4793684f) addressed the review findings within that scope — no unrelated drift included (git diff 27afb732b3..af4793684f touches only the paseo setup surface + its docs/changelog; verified the replay diff equals the remote PR diff except the intended CHANGELOG hunk).
3. dev advanced again mid-flight (27afb732b3 → 416201eb5e, PR feat(autoresearch)!: replace the team workflow with a first-class autoresearch skill #4430 autoresearch); the branch was rebased onto 416201eb5e and re-pushed with --force-with-lease (lease pinned to the exact prior remote head each time).

Local verification at exact head af4793684f:

• bun test packages/coding-agent/test/setup-paseo.test.ts packages/coding-agent/test/setup-cli.test.ts — 110 pass / 0 fail (5 new regression tests covering each review finding, including the constructed legacy upgrade state, unprovenanced links, and non-ENOENT failure)
• bun --cwd=packages/coding-agent run check (biome + tsc) — clean
• bun run check:ts — green (full aggregate)
• bun run ci:test:smoke — ok
• Gates: check-visible-definitions, verify-g002-gates, rebrand-inventory --strict, default-gjc-definitions.test.ts (33 pass) — all pass
• git diff --check — clean

Verdict: exactly one current gajae.pr-review-verdict.v1 line in the PR body, digest sha256:bb0816db8f2399b318a63d5445e755b30ffe943ca120042171f6777cf3058d82 = canonical 416201eb5e...af4793684f diff. Status needs-human: per the exact-head contract this PR cannot be self-approved by the author — requesting a fresh authorized non-author review at exact head af4793684f6b620bcc53c2490b7c6a58942208a9 (cc @probepark — your three findings are addressed in af47936; the coverage gaps you flagged are now pinned by tests). CI on this head is expected to run the full matrix; Validate exact-head PR contract will remain red-by-design until the independent approving review lands, then the verdict flips to merge-approved.

Every push invalidates prior review/CI — ownership retained until dev merge or evidence-backed close.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

@probepark @snowykr — requesting a fresh exact-head review on PR #4644.

Exact head: af4793684f6b620bcc53c2490b7c6a58942208a9 (base dev @ 416201eb5e)

Your three majors from the 4981a921 review were each reproduced on the exact legacy-upgrade state #4638 produced, then fixed in af4793684f:

1. Provenance migration from the recorded source — a legacy ledger predating bridgeSourceDir now falls back to the single pre-bug(setup paseo): skill bridge can never reach a clean --check against Paseo desktop 0.4.0 (hardcoded allowlist + missing ~/.agents/skills) #4638 source location (~/.agents/skills), and preflight adopts recorded legacy links by re-pointing them at the discovered source. The wedged machine converges end-to-end: --check drift → re-run install → pass, ledger records the bundle source; --remove proves ownership against the recorded/legacy source even with Paseo uninstalled.
2. Prune gating — a paseo-prefixed symlink the ledger never recorded is a conflict, never pruned; only ledger-recorded names converge.
3. Non-ENOENT fail-closed — lstat distinguishes absence from EACCES-class failures; removal reports partial-removal and retains the ledger instead of clearing provenance with an owned link still on disk. Coverage now includes the constructed upgrade state, unprovenanced links, and injected non-ENOENT failures (5 new regression tests, 92 total in the suite).

Local verification at this exact head: 110 pass across the two setup suites, package check clean, full check:ts green, smoke ok, all four gates pass, git diff --check clean. Signed reconstruction evidence: #4644 (comment)

Per the exact-head contract the author cannot self-approve; the verdict stays needs-human until an authenticated non-author approval lands at this head.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

CI classification at exact head af4793684f6b620bcc53c2490b7c6a58942208a9 (base dev @ 416201eb5e) — replacement product run 32104995990 terminal:

• Every product/content check green: Affected path validation (plan, native-build, check:@gajae-code/coding-agent, ts-build, cli-smoke, evidence producer, aggregate, test:setup-paseo.test.ts, test:docs-index-lazy.test.ts), all four gjc-state-gates shards + aggregate, Virtual integration validation, Local public surfaces.
• 1 expected failure, not an implementation defect: PR contract bootstrap — fails on Verdict needs-human intentionally blocks merge, the by-design gate requiring an authenticated non-author approving review at this exact head before merge-approved.
• Contract run 32104996355 confirms body digest / exact-head / base ancestry / writer gates all pass with only the same needs-human remaining.
• Skipped: darwin-arm64 tab-worker smoke, Telegram daemon generation guard, Windows variants — gated on runners/paths this TypeScript-only change does not touch.
• The earlier cancelled run 32104801350's evidence producer failure was a cancellation artifact (native-build cancelled mid-flight → fail-closed), not a code failure; the replacement run's evidence producer is green.

Fresh exact-head reviews requested from @probepark @snowykr (all three CHANGES_REQUESTED findings addressed in af4793684f). Ownership retained; nothing to fix-forward until review lands.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[probepark]

Independent maintainer review at exact head af479368 — merge blocked. Three majors. My earlier findings on migration and error-collapse are fixed; the ownership model still is not.

what is fixed

--remove now traces ledger bridgeEntries + bridgeSourceDir and performs exact-target verification before unlinking. Legacy migration works. Docs describe discovery rather than the old allowlist. Changelog is under ## [Unreleased]. Contract sweep is clean — no added any, ReturnType<>, inline await import(), visibility modifiers, sync fs I/O, or console.*.

major 1 — setup-time pruning does not apply the ownership rule that remove does

packages/coding-agent/src/setup/paseo/skills-bridge.ts:276-307: a recorded name is treated as ownership. If the current source no longer offers that name, setup unlinks whatever symlink sits at the recorded path — without checking it still points under the ledger-recorded source.

So: user retargets a bridged name at their own link, source drops the name, next gjc setup paseo deletes the user's artifact. Legacy adoption compounds it — it is permitted even when the ledger already records a different source.

You built exact-target verification for remove. Apply the same predicate here: require target equality against bridgeSourceDir (or the recorded legacy source), and gate legacy adoption on the ledger actually being legacy.

major 2 — PASEO_SKILLS_DIR is read from process.env, which includes the repo's .env

packages/coding-agent/src/setup/paseo/setup-deps.ts:56-60. process.env in a project checkout carries that project's .env. Trust evidence for why that is not "explicit user intent": packages/coding-agent/src/hooks/native-skill-hook.ts:168-176 already draws this distinction.

Running global setup while sitting in an untrusted repository can therefore persist that repository's paseo* prompt skills into the user's GJC configuration. A cloned repo shipping a .env gets to install prompt content. Resolve through the established non-project environment resolver, or require an explicit CLI flag / config key.

Related, and worth stating: discovery selects every direct directory whose name starts with paseo — prefix, not paseo-. Files and symlinks are excluded. The context-search denylist is dead code, since that name fails the prefix test first. Bridging source-derived names is intended per the body; sourcing them from a repo-controlled .env is not.

major 3 — bridge mutation commits before provenance is durable

packages/coding-agent/src/setup/paseo/paseo-setup.ts:196-227: links are created, then provenance is written, then compensation is registered. A ledger-write failure leaves live links that nothing owns. Retry then classifies them as noops and never re-attempts the provenance write, so check can report healthy while remove has no record to clean. Mid-operation failure leaves the same partial unrecorded state.

Write a durable bridge intent before mutating, or make the whole thing rollback-safe, and add fault-injection coverage for a failed ledger write.

minor — bridgeDirCreated is not durable

paseo-setup.ts:207-218: a convergence run overwrites a previously true bridgeDirCreated with false, because on that run the directory already existed. remove then leaves GJC's own empty bridge directory behind. That bit records who created the directory originally; it must survive reruns.

coverage

The dynamic-name, app-bundle install, missing-source, release add/drop, missing-link repair, legacy migration/removal, uninstall, and repeated install/check/remove tests all fail against the old allowlist implementation, so the central #4638 fix is genuinely pinned. But the test named "with or without provenance" only covers the unprovenanced case — it never retargets a ledger-recorded link, which is exactly major 1. Nothing injects a cwd .env (major 2), nothing fails the ledger write mid-mutation (major 3), and nothing runs create-directory → source change → reinstall → remove (the minor). All four findings are unpinned.

_{Reviewed by @probepark — method: detached worktree at af479368, full read of skills-bridge.ts and paseo-ownership.ts at head, discovery-order and prefix-filter trace, ownership-marker trace from write through prune and remove, env-trust cross-check against native-skill-hook.ts, per-test would-this-fail-on-base analysis. Tests not executed.}

gajae.pr-review-verdict.v1 merge-blocked sha256:bb0816db8f2399b318a63d5445e755b30ffe943ca120042171f6777cf3058d82 reviewer:human reviewer-id:probepark evidence:exact-head-af479368-three-majors-prune-ownership-env-trust-and-non-durable-provenance

[Yeachan-Heo]

Round-2 fix-forward evidence — PR #4644, exact head aff9e927d5abe02fb5c80a18d2429fcf173de0de (base dev @ b01f34527c), closes #4638.

Head moved (force-with-lease, lease pinned to prior head af4793684f) to address every finding in your second CHANGES_REQUESTED at af4793684f:

1. Setup-time pruning now applies the same exact-target predicate --remove uses. A recorded name is only ours while its link still resolves under the ledger-recorded source (bridgeSourceDir, or the legacy ~/.agents/skills for a legacy ledger). A retargeted recorded link is a conflict, never pruned. Legacy adoption is gated to ledgers that genuinely predate bridgeSourceDir — a ledger already recording a source never has its links rewritten. (skills-bridge.ts preflight)
2. PASEO_SKILLS_DIR is explicit user intent only, via the established non-project resolver rule (parseEnvFile(cwd/.env) comparison, same as native-skill-hook.ts uses for GJC_CODING_AGENT_DIR): a value matching the project .env is rejected, so a cloned repo cannot bridge its own paseo* content into the user's GJC config. Dead context-search denylist removed (fails the prefix filter on its own).
3. Provenance commits BEFORE any bridge mutation. A failed ledger write now aborts before links exist (regression test pins: unwritable ledger ⇒ bridge dir absent/empty, never installed); a crash after mutation is always covered by the record already on disk. Verified adversarially: superset-record crash state → retry converges → check pass → remove cleans.
4. Minor fixed: bridgeDirCreated records the original creator and survives convergence reruns; --remove after source disappearance deletes the directory GJC created (regression test runs create → rerun → remove).

Regression coverage added (5 new tests, 97 in suite): retargeted-recorded-link conflict; adoption refused when the ledger records a source; .env-sourced PASEO_SKILLS_DIR rejected; failed ledger write leaves no unrecorded links; bridgeDirCreated durability + remove cleans the directory.

Verification at exact head aff9e927d5: 115 pass across setup suites, package check clean, ci:test:smoke ok, all 4 gates pass, default-gjc-definitions 33 pass, git diff --check clean. (Aggregate check:ts/check:sdk-closure flakes on this loaded shared box reproduce green in isolation — port/bind exhaustion in unrelated SDK/Telegram suites; this diff touches only setup/paseo/**.)

Verdict line updated to exactly one current line, digest sha256:7d300ea0… = canonical b01f34527c...aff9e927d5 diff. Still needs-human: cannot be self-approved by the author — requesting fresh exact-head re-review at aff9e927d5 from @probepark (and @snowykr).

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict: Request changes

---

Summary

This revision improves Paseo desktop-app discovery and adds substantial convergence and failure-path coverage. However, the delegated A1, A2, A3, and A4/A5 reviews identified multiple blocking ownership, deletion-safety, crash-consistency, trust-boundary, and platform-verification issues. The provenance contract is not safe to merge in its current form.

---

Findings / Required Changes

[P1] Do not claim ownership of noop bridge entries

Location: packages/coding-agent/src/setup/paseo/paseo-setup.ts:201-211; skills-bridge.ts:245-264

preflightEntryNames uses Object.keys(bridgePreflight.entries), which includes action: "noop" links that existed before GJC created them and are not present in the prior ledger. The install then records those names in bridgeEntries, and --remove treats them as GJC-owned. A pre-existing exact-target symlink can therefore be deleted by a later removal.

This was independently confirmed by the A1, A2, and A3 delegated reviews.

Required change: record only links actually created or explicitly adopted under a safe, documented legacy rule. Existing unrecorded noop links must remain foreign. Add an end-to-end setup-then-remove regression test proving a pre-existing exact-target symlink survives.

[P1] Preserve and use the ledger’s recorded bridge path during removal

Location: packages/coding-agent/src/setup/paseo/remove.ts:98-117

Removal computes bridgeDir = ledger.bridgePath ?? deps.paths.bridgeDir for inspection, but calls inverseSkillsBridge(deps, ...), whose implementation derives the link path from deps.paths.bridgeDir. After an agent/profile/path migration, links at the recorded old bridgePath are inspected in one directory but removed/validated in another, causing partial removal or leaving owned links behind.

Required change: pass the resolved ledger bridge path into the inverse operation and use it consistently for validation, unlinking, and directory cleanup. Add a migration/path-change regression test.

[P1] Make provenance persistence actually crash-safe

Location: packages/coding-agent/src/setup/paseo/paseo-ownership.ts:80-82

The implementation relies on writing provenance before creating links, but writeProvenance uses direct Bun.write. A crash during that write can truncate or corrupt the ledger; the subsequent read may treat it as empty, leaving links created by the run unowned and not removable. This invalidates the claimed record-before-mutation crash guarantee.

Required change: use an atomic temporary-file write followed by flush/sync and rename (with appropriate locking/CAS for concurrent setup), and add an interrupted-write/corrupt-ledger regression test that proves no newly created link becomes silently unowned.

[P1] Close check-then-unlink races

Location: packages/coding-agent/src/setup/paseo/skills-bridge.ts:354-376, 430-442

pruneStale, adoptLegacyLink, pruneRecordedDangling, and inverseSkillsBridge validate a symlink and then unlink the path in separate operations. A concurrent replacement can occur between validation and unlink, allowing deletion of a foreign file or symlink at the same path.

Required change: use an atomic no-follow deletion/quarantine strategy, or otherwise revalidate identity at the deletion operation so a replaced entry cannot be removed. Add a concurrent replacement regression test where feasible.

[P1] Do not let project .env expansion bypass the trust guard

Location: packages/coding-agent/src/setup/paseo/setup-deps.ts:61-65

The guard compares Bun-expanded process.env.PASEO_SKILLS_DIR with the literal value returned by parseEnvFile. For a project .env value such as PASEO_SKILLS_DIR=$PWD/skills, the values differ even though the environment value originated from that project file. The guard can therefore accept a repository-controlled path and bridge repository-owned paseo* content into globally loaded custom skills.

Required change: reject project .env overrides after the same expansion semantics Bun applies, or compare normalized expanded values. Add a regression test using an interpolated .env value.

[P2] Exercise the production macOS app-bundle resolver

Location: packages/coding-agent/src/setup/paseo/setup-deps.ts:36-47; packages/coding-agent/test/setup-paseo.test.ts:965-1019

The production candidate list probes /Applications and ~/Applications only on macOS, while the tests cover the non-darwin empty branch and inject skillsSource for desktop flows. No supplied CI/e2e evidence exercises the real resolver against either macOS app root or verifies the resulting bridge symlinks. Darwin-arm64 validation is skipped by path/runner gating.

Required change: add a macOS CI lane, or introduce an injectable platform/app-root seam and test the production discovery path end to end.

---

CI / Verification

The delegated A4/A5 review reports the following product checks green at the exact head:

• focused setup-paseo.test.ts
• check:@gajae-code/coding-agent
• coding-agent TypeScript build
• CLI smoke
• evidence/aggregate checks
• GJC state gates
• virtual integration validation
• local public surfaces

PR contract bootstrap is red because the PR still has the intentional needs-human verdict; this is a metadata gate, not evidence that the implementation passes. Darwin-arm64 and Windows/Telegram lanes are skipped by path/runner gating. The focused test suite was not rerun by the reviewers.

---

Axis coverage

• A1 Intent / Policy / Contract: P1 ownership regression confirmed; the removal contract is violated.
• A2 Architecture / Correctness / Failure: Four P1s confirmed: ownership recording, recorded-path removal, provenance crash consistency, and check-then-unlink races.
• A3 Security / Privacy / Trust: P1 project .env expansion bypass confirmed; the ownership/deletion issue is also security-relevant because untrusted source content is bridged into globally loaded skills.
• A4 Verification / Tests / CI: P2 production macOS resolver path is not exercised; existing focused product checks are green.
• A5 Context / Compatibility / Platform: P1 bridge-path migration gap and P2 macOS app-bundle verification gap affect surrounding installations and platforms.

[Yeachan-Heo]

CI at exact head aff9e927d5abe02fb5c80a18d2429fcf173de0de (base dev @ b01f34527c) — run 32137999155, terminal:

• Every product/content check green: Affected path validation (plan, native-build, cli-smoke, ts-build, check:@gajae-code/coding-agent, evidence producer, aggregate, test:setup-paseo.test.ts, test:docs-index-lazy.test.ts), all gjc-state-gates shards + aggregate, Virtual integration validation, Local public surfaces.
• 1 expected failure, by design: PR contract bootstrap — Verdict needs-human intentionally blocks merge (the exact-head contract requires an authenticated non-author approving review before merge-approved; the author cannot self-approve).
• Skipped: darwin-arm64 tab-worker smoke, Telegram daemon generation guard, Windows variants — gated on runners/paths this TypeScript-only setup/paseo/** change does not touch.
• Superseded run 32137946763's failures were cancellation artifacts of the body-edit retrigger, not code failures.

All round-2 findings addressed at this head (see #4644 (comment)); verdict line carries the canonical b01f34527c...aff9e927d5 digest sha256:7d300ea0…. Awaiting fresh exact-head review from @probepark / @snowykr; ownership retained.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The app-bundle source-discovery direction is appropriate, but the new bridge lifecycle has several ownership, durability, and trust-boundary regressions. In particular, setup can claim user-owned links, cleanup can operate on a different path than the one it inspected, and project-controlled dotenv interpolation can influence globally discovered prompt content.

Findings / Required Changes

• [P1] Do not record pre-existing no-op links as GJC-owned. skills-bridge.ts:272 classifies an exact-target existing symlink as action: "noop", while paseo-setup.ts:201-211 persists every preflight entry name as bridgeEntries. A later --remove can therefore delete a user-created link that GJC never created. Persist only links actually created by GJC or explicitly adopted by the legacy migration rule, and add a setup/remove regression proving an existing exact-target link survives.

• [P1] Make provenance durable and recoverable. paseo-ownership.ts:79-82 writes the live ledger directly with Bun.write; interruption can leave corrupt JSON, which is interpreted as an empty ledger. That defeats the new record-before-mutation safety guarantee and strands published links without safe ownership metadata. Write and flush a mode-0600 temporary ledger, atomically rename it into place, and make corrupt provenance an explicit recovery error rather than silently treating it as empty.

• [P1] Do not drop prune ownership before the prune succeeds. paseo-setup.ts:201-211 records desired/adopted entries but omits bridgePreflight.prunes before installSkillsBridge unlinks them. A crash or unlink failure then leaves a stale link on disk with no provenance; retry treats it as foreign and --remove cannot clean it. Keep prune entries durable until removal completes, or persist a recoverable prune intent.

• [P1] Eliminate check-then-unlink races in bridge mutations. skills-bridge.ts:357-362 validates a path and unlinks it in separate operations; the same pattern exists in other destructive bridge paths. Another process can replace the checked symlink before unlink, allowing deletion of a foreign replacement. Use a serialized, identity-bound, no-follow deletion/quarantine strategy and preserve a conflict when an entry changes.

• [P1] Reject interpolated project dotenv overrides. setup-deps.ts:61-65 compares Bun's expanded process.env.PASEO_SKILLS_DIR with the literal parsed .env value. For example, a repository .env containing PASEO_SKILLS_DIR=$PWD/skills bypasses the guard and makes global setup bridge repository-controlled paseo* prompt content. Reject the override whenever the project dotenv defines it (including interpolated values), or source overrides exclusively from a non-project environment/explicit CLI input; add a regression for interpolation.

• [P1] Use the recorded bridge path throughout removal. remove.ts:98-120 validates entries under ledger.bridgePath, but inverseSkillsBridge derives paths from deps.paths.bridgeDir. After a profile/path migration, removal can inspect the old owned directory and unlink from the current directory instead, leaving old links and potentially affecting current ones. Pass the resolved ledger path through validation, unlinking, directory cleanup, unregistering, and diagnostics.

• [P1] Make the new provenance-order regression test reach the bridge step. test/setup-paseo.test.ts:730-735 turns the ledger into a directory before runPaseoSetup; the saga fails while writing the earlier provider-config ledger, before the bridge provenance/mutation block. The test would pass with the old unsafe ordering. Inject a failure specifically after the bridge provenance commit and assert the bridge/ledger state across retry and removal.

• [P2] Cover the production macOS resolver, not only injected sources. test/setup-paseo.test.ts:947-1010 tests candidate shape and injects skillsSource for app fixtures; the Darwin runtime branch is not exercised by the observed CI. Add a macOS lane or injectable platform/app-root seam that runs the production resolver end to end for /Applications and ~/Applications.

• [P2] Correct the missing-source documentation contract. docs/terminal-app-integrations.md:58-59 promises that no skills directory cannot leave --check red, but check.ts intentionally reports missing-skills-directory as drift with a non-zero exit. Clarify that dangling links are avoided while --check remains drifted until a source is restored or a trusted override is supplied.

CI / Verification

Reviewed exact head aff9e927d5abe02fb5c80a18d2429fcf173de0de. The observed exact-head product checks are green, including the focused Paseo test, coding-agent check/build, CLI smoke, evidence, aggregate, state-gate, and virtual-integration checks. The PR-contract bootstrap needs human failure is intentionally ignored. Darwin, Windows, and Telegram platform jobs are skipped; the superseded run's cancellation failures are not implementation evidence. The test findings above identify gaps in what the passing checks actually prove.

Axis Coverage

• A1 — Intent / Policy / Contract: blocked: project-dotenv trust bypass; missing-source documentation mismatch.
• A2 — Architecture / Correctness / Failure: blocked: false ownership, non-durable provenance, prune crash window, and destructive TOCTOU race.
• A3 — Security / Privacy / Trust: blocked: user-link deletion risk, global prompt-content trust bypass, unsafe destructive races, and provenance integrity.
• A4 — Verification / Tests / CI: blocked: provenance-order test fails before the target bridge step; production macOS resolver coverage is absent.
• A5 — Context / Compatibility / Platform: blocked: recorded bridge-path removal is inconsistent; macOS app-bundle behavior lacks executable coverage.

[Yeachan-Heo]

Round-3 fix-forward evidence — PR #4644, exact head 455e68672dfdc8ad3af2d83c1db64321978884c6 (base dev @ 08bef6cf88), closes #4638.

Head moved (force-with-lease, lease pinned to prior head aff9e927d5) to address every P1 in @snowykr's review at aff9e927d5 (rebased onto advanced dev 08bef6cf88):

1. No false ownership — a pre-existing exact-target link GJC did not previously record stays unowned. The ledger persists only entries this/earlier runs create/recreate, adopted legacy links, and prune candidates until their unlink completes. Regression: user pre-creates an exact-target paseo link → install does not record it → remove leaves it live while cleaning GJC's own.
2. Durable atomic provenance — writeProvenance writes a mode-0600 fsynced temporary and renames it into place; corrupt JSON is now an explicit ProvenanceLedgerCorruptError with recovery guidance (never a silent empty ledger). Regression: corrupt ledger rejects with /corrupt/ from both readProvenance and --check.
3. Prune ownership durable through the unlink window — prune candidates are in the record-before-mutation write and dropped only after the unlink succeeds. Regression: release drops a skill → post-prune ledger no longer owns the name; link gone.
4. No check-then-unlink races — every destructive bridge operation (prune, prune-and-recreate, adopt, remove-inverse) now renames the entry into a GJC-owned quarantine and verifies the captured link text POST-rename; a swapped pathname is detected, atomically restored, and refused. (exactUnlink refuses S_IFLNK by design, so quarantine-verify-restore is the binding primitive for symlinks.) Regression: hand-built preflight with a foreign link at a recorded name → refused, foreign link intact at its original path, zero quarantine litter.
5. Interpolated dotenv rejected by presence — PASEO_SKILLS_DIR trust is now presence-based: the project dotenv defining the key at all rejects the override, closing the $PWD/${VAR} expansion bypass. Regression uses a literal $PWD/skills .env.
6. Recorded-path removal consistency — --remove uses ledger.bridgePath for validation, unlinking, directory cleanup, and failure diagnostics. Regression: migrated deps.paths.bridgeDir while the ledger records the original → the recorded directory is cleaned.
7. Provenance-order test now reaches the bridge step — it fails during the bridge record commit (after provider/orchestration steps succeeded) and asserts the ledger is byte-identical with no .tmp litter and no bridge directory.
8. Docs corrected — a missing source is documented as missing-skills-directory drift (exit 1) until restored; no promise of a green --check without a source.

P2 note (macOS production resolver lane): the Darwin branch remains covered by the injectable seam + candidate-shape tests; the repo's CI has no darwin lane for this suite. If maintainers want one, that is a workflow addition beyond this PR's scope — stated here rather than silently dropped.

Verification at exact head 455e6867: 103 pass in setup-paseo.test.ts (7 new r3 regressions), 121 across both setup suites, package check clean, ci:test:smoke ok, all 4 gates pass, git diff --check clean. Adversarial end-to-end replays (fresh install → convergence rerun → release add/drop → remove; legacy wedge migration; superset-crash recovery) all verified on the real fs.

Verdict updated to exactly one current line, digest sha256:b3047d7e… = canonical 08bef6cf88...455e68672d diff. Still needs-human: requesting fresh exact-head re-review at 455e68672d from @snowykr and @probepark.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The dynamic Paseo skill-source direction addresses the reported desktop-app installation problem, but the new bridge lifecycle currently has unsafe ownership and trust-boundary paths. In particular, repository-controlled environment inputs and stale/tampered provenance can make global setup adopt or delete paths it does not own. These need to be resolved before merge.

Findings / Required Changes

1. [P1] Reject every project dotenv variant before honoring PASEO_SKILLS_DIR
packages/coding-agent/src/setup/paseo/setup-deps.ts:60-64 only excludes cwd/.env. Bun also loads project .env.local and environment-specific dotenv files into process.env. A repository can therefore supply an interpolated override that makes global setup register untrusted skills in skills.customDirectories. Resolve this override from an environment source that excludes all project dotenv files, or reject the key whenever it originates in any project dotenv variant.

2. [P1] Do not let an unchecked ledger path drive destructive removal
   packages/coding-agent/src/setup/paseo/remove.ts:54-56 trusts ledger.bridgePath when composing paths for quarantine/unlink and directory cleanup. A malformed, tampered, or path-replaced provenance record can redirect --remove to an unrelated directory and delete a matching foreign symlink. Validate that the recorded path is absolute, remains under the trusted agent root, and resolves to the expected non-symlinked bridge directory before mutating it.

3. [P1] Preserve ownership boundaries across a bridge-path migration
   packages/coding-agent/src/setup/paseo/paseo-setup.ts:199-223 carries recorded ownership by skill name without binding it to bridgeLedger.bridgePath. After an agent/profile path change, a user-owned exact-target link at the new path can inherit ownership and later be deleted; overwriting the ledger also abandons the old bridge links. Handle migration explicitly before reusing prior entries, and unregister the ledger-recorded path in the remove callback (paseo-setup.ts:72-79).

4. [P1] Keep non-symlink bridge conflicts as partial removal, not success
   packages/coding-agent/src/setup/paseo/remove.ts:103-110 filters a recorded entry that was replaced by a regular file or directory before inverse validation. Removal then clears provenance and reports success while leaving an unowned replacement at a formerly managed path. Preserve every present recorded pathname for inverse validation and retain provenance on divergence.

5. [P1] Do not register an unvalidated existing bridge when no source is resolved
   packages/coding-agent/src/setup/paseo/skills-bridge.ts:275-278 still registers the bridge directory after resolveSource() returns undefined. A stale or foreign bridge can then be globally loaded through skills.customDirectories without a validated source or ownership record. Skip registration or fail with a diagnostic unless the bridge is validated/owned.

6. [P2] Preserve valid symlinked skill directories during source enumeration
   packages/coding-agent/src/setup/paseo/skills-bridge.ts:150-164 accepts only Dirent.isDirectory(). Directory symlinks/junctions are valid source shapes and are accepted by GJC's skill discovery, but are omitted here, which can produce an empty bridge or prune previously available skills. Resolve and validate symlinked directories rather than discarding them.

7. [P2] Cover and fix cleanup after every bridged skill is pruned
   packages/coding-agent/test/setup-paseo.test.ts:906-927 does not cover the state where convergence prunes the final entry. That leaves bridgeDirCreated: true with bridgeEntries: [], while removal currently skips the bridge inverse and strands GJC's empty directory. Run cleanup when either entries exist or the directory is recorded as created, and add this regression case.

CI / Verification

• Reviewed the exact PR head 455e68672dfdc8ad3af2d83c1db64321978884c6 through the supplied CI artifacts and changed tests; no PR code was executed locally for this review.
• At review time, Dev CI's affected packages/coding-agent/test/setup-paseo.test.ts job and coding-agent TypeScript build were passing. The affected coding-agent check was still in progress.
• The earlier cancelled exact-head run is not treated as verification evidence. needs human CI failures were ignored.

Axis Coverage

Axis	Coverage	Result
A1 — Intent / Policy / Contract	PR intent, CLI/setup contract, documentation and diagnostics	Findings reported
A2 — Architecture / Correctness / Failure	Ownership, lifecycle, cleanup, races, error paths	Blocking findings reported
A3 — Security / Privacy / Trust	Dotenv provenance, filesystem/symlink trust boundaries, destructive removal	Blocking findings reported
A4 — Verification / Tests / CI	Changed tests and provided GitHub CI artifacts; no local execution	Coverage gaps reported
A5 — Context / Compatibility / Platform	Existing installs, path migration, desktop-app/source compatibility, documentation	Findings reported

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 3 actionable issues, led by Close quarantine unlink TOCTOU race and Validate the complete intent record before recovery. These findings require changes before approval.

Findings / Required Changes

1. [P1] Close quarantine unlink TOCTOU race.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:145-146
   After verification, a concurrent actor can replace the quarantine pathname before fs.unlink executes, causing cleanup to delete the foreign object. Use descriptor-bound or otherwise race-resistant deletion, and fail closed without unlinking when identity changes.
2. [P1] Validate the complete intent record before recovery.
   Reference: packages/coding-agent/src/setup/paseo/paseo-ownership.ts:300-314
   readIntent validates only targetPath and targetExpectedIdentity, then recovery trusts provenancePath, identity fields, step, and provenancePayload; a tampered record can cause recovery to inspect or write an arbitrary provenance path. Validate every field, payload shape, and trusted path relationship before classifyIntent or refuse recovery.
3. [P2] Required dependency seam breaks existing callers.
   Reference: packages/coding-agent/src/setup/paseo/setup-deps.ts:245-258
   PaseoSetupDependencies now requires skillsSource, so existing consumers constructing this exported interface no longer type-check. Preserve source compatibility with an adapter/default resolver or provide a migration-safe optional field without silently disabling discovery.

CI / Verification

• Reviewed the exact remote head: 9b5e36e35bfe1ba2d76f38b7f17ef8b3a80bb2b5.
• CI summary: 6 passing, 7 failing, 23 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, PR contract bootstrap, Affected path validation.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	The API boundary has a concrete source-compatibility break from the newly required skillsSource dependency; runtime compatibility otherwise appears intentionally preserved.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	Correctness risk is established in crash recovery: partially validated intent data can redirect provenance recovery to an unintended path.
A3. Security / Privacy / Trust	CHANGES_REQUESTED	The bridge mostly applies fail-closed ownership checks, but quarantine cleanup retains a security-relevant pathname race that can delete foreign filesystem objects.
A4. Verification / Tests / CI	APPROVED	Verification is incomplete in CI: the changed setup paths have extensive regression coverage, but failed and skipped affected-path validation leave runtime compatibility unproven.
A5. Context / Compatibility / Platform	APPROVED	Integration and documentation contracts are consistent with the implementation; platform behavior remains unverified by CI.

Limitations

• CI summary shows affected-path validation failures and Windows/darwin-arm64 jobs skipped, so platform validation is unconfirmed.

[Yeachan-Heo]

Fix-forward: all three r13 findings closed

Current PR head: f2537966a2fe1ed2740699730da1cc3deafc10a6
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: 00b8afbf9c4dbd488727f559aafd27be0a246c5f4b6bd57610fdef193a1bd856

1. Quarantine unlink TOCTOU — deletion is now bound to the captured inode: after the quarantine rename, the symlink's (dev, ino, size, mtime) is captured once, the link text verified once, and every deletion attempt re-lstats the quarantine name requiring that exact inode immediately before the unlink. A concurrent actor replacing the pathname changes the inode and the deletion refuses without unlinking, restoring what is there. ENOENT between check and unlink is already-gone (nothing foreign deleted by us); a foreign survivor of the final microsecond window is swept back to the bridge name. The identity-bound native exactUnlink deliberately refuses S_IFLNK, so inode-guarded refusal is the strongest race-resistant deletion the runtime offers.
2. Full intent validation — readIntent now validates every field recovery trusts: all six identity strings, step against the known steps, version, ownedKeys (non-empty strings), and the provenancePayload shape — plus a trusted-path relationship check refusing any recorded provenancePath that escapes the agent directory holding the intent record. Pinned by a regression where a record keeping its target fields but naming a foreign provenance path is refused as corrupt.
3. Seam compatibility — skillsSource is optional again with a migration-safe default: an omitted resolver falls back to the real discovery order (resolvePaseoSkillsSource) in both the bridge and the check path, so existing callers constructing the exported interface keep type-checking, and the fallback is never a silent bridge skip.

Verification at the exact head: 154/154 setup-paseo tests (2 new r13 regressions), setup-cli clean, check clean, smoke ok, G002 pass, all four state-gate groups pass, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is f2537966 on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 5 actionable issues, led by Bridge validation accepts arbitrary directories inside the trusted agent root and Migration-path validation is forgeable. These findings require changes before approval.

Findings / Required Changes

1. [P1] Bridge validation accepts arbitrary directories inside the trusted agent root.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:99-115
   validatedBridgeDir treats any path under trustedRoot as valid and does not require the bridge basename or canonical bridge location, allowing a tampered ledger to direct cleanup at another agent-root directory. Require the recorded path to match the expected bridge basename/location before allowing destructive removal.
2. [P1] Migration-path validation is forgeable.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:67-76,121-124
isGenuinePaseoLedgerDir only checks for a non-symlink paseo directory and regular provenance.json; an attacker who tampers the ledger can create that trivial shape beside an arbitrary victim bridge, causing validatedBridgeDir to authorize destructive cleanup there. Authenticate the migration directory against trusted installation identity or reject migration paths outside the current trusted root.
3. [P1] Mutation lock can expire during a healthy operation.
   Reference: packages/coding-agent/src/setup/paseo/paseo-mutation-lock.ts:23-35
   The lock uses a fixed 60-second stale window without lease renewal; a valid install/remove exceeding that window can be reclaimed by another process while the first still mutates shared files. Add heartbeat/lease ownership or guarantee stale recovery cannot overlap a live holder.
4. [P1] Persist failure can orphan a durable sidecar.
   Reference: packages/coding-agent/src/setup/paseo/install-saga.ts:198-207
persisted is set only after persist() resolves. If persist() creates the artifact and then throws, rollback skips unpersist, while the published target is reverted and intent is cleared, leaving an unreferenced sidecar. Track artifact creation before possible throws and remove it on every post-publish persist failure.
5. [P2] Making agentsSkillsDir optional breaks downstream TypeScript consumers.
   Reference: packages/coding-agent/src/setup/paseo/setup-deps.ts:205-216
   PaseoPaths previously exposed agentsSkillsDir as required but now declares it optional, so existing callers that pass it to path APIs or access it without narrowing can fail compilation. Preserve the required field or provide a compatibility type/accessor while keeping runtime source resolution separate.

CI / Verification

• Reviewed the exact remote head: f2537966a2fe1ed2740699730da1cc3deafc10a6.
• CI summary: 6 passing, 6 failing, 23 pending/cancelled/skipped.
• Failing checks: PR contract bootstrap, Affected path validation, Affected path validation / evidence producer.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	API compatibility is not fully preserved: an exported field became optional, and bridge cleanup accepts overly broad in-root paths that weaken provenance boundaries.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	Concurrency locking and post-publish rollback were reviewed; lock expiry and partial-persist cleanup leave established race risks.
A3. Security / Privacy / Trust	CHANGES_REQUESTED	Security review found a P1 path-trust bypass allowing forged migration records to authorize cleanup outside the trusted agent root.
A4. Verification / Tests / CI	APPROVED	A4 review found extensive regression coverage, but non-green and skipped CI prevent concluding the changed Paseo paths are fully validated.
A5. Context / Compatibility / Platform	APPROVED	Integration and documentation contracts are coherent; platform behavior is bounded and fail-closed, but skipped platform CI leaves runtime compatibility unverified.

Limitations

• Windows and darwin-arm64 platform validation jobs were skipped, so CI does not establish those platform paths for this change; affected-path validation also failed.

[Yeachan-Heo]

Fix-forward: all five r14 findings closed

Current PR head: 7ba40f08814e9d8012102a3f92c944dfdc5cfea0
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: fca387c3b3544da923212da2473b04d854511bc96ee0b3ec360a236f4a381ff6

1. Arbitrary in-root directory cleanup — validatedBridgeDir now requires the recorded path to carry a GJC bridge basename (paseo-skills / *-paseo-skills / the current spelling). A tampered ledger naming any other directory inside the agent root is refused before any destructive use; same-agent-directory relocations (the migration install itself drives) keep working.
2. Forgeable migration shape — a migration record is now authenticated against the OLD directory's OWN ledger: besides the genuine non-symlink paseo/ + provenance.json shape, the victim's ledger must itself record bridgePath equal to the exact path being validated (enforced on both the lexical and fully-resolved branches). Fabricating directory names beside a victim is trivial; making the victim hold a GJC ledger that already claims this exact bridge is a genuine prior installation — the only thing the migration path should ever clean.
3. Lock expiry under a healthy holder — the mutation lock records owner pid + process-start-time identity, and the shared file lock's rule is that a provably-live owner is NEVER reaped (#652); the 60 s window bounds only the liveness-indeterminate fallback, and acquisition waits out a contender (2-minute acquire budget) instead of displacing it. The seam now documents this invariant explicitly; a healthy install/remove cannot lose its lease by running long.
4. Persist-then-throw orphan — the saga now tracks that the persist hook was ATTEMPTED (not only that it resolved), so a sidecar created before a post-creation validation throw is removed by every post-publish rollback path. No credential-bearing artifact survives unreferenced.
5. agentsSkillsDir optionality — the field is required again, carrying the default ~/.agents/skills spelling: external callers constructed it as required before it ever became optional, so the required shape is the compatible one. The resolved per-run source remains what the bridge links against and the ledger records.

Verification at the exact head: 154/154 setup-paseo tests, setup-cli clean, check clean, smoke ok, G002 pass, all four state-gate groups pass, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 7ba40f08 on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 3 actionable issues, led by Concurrent target change incorrectly treated as successful rollback and Preserve exported skills-bridge construction compatibility. These findings require changes before approval.

Findings / Required Changes

1. [P1] Concurrent target change incorrectly treated as successful rollback.
   Reference: packages/coding-agent/src/setup/paseo/install-saga.ts:222-226
   When the target identity differs after publication, the code sets reverted=true while deliberately not reverting the target. It then clears the intent and removes persisted artifacts, leaving the published change unprovenanced and unrecoverable. Set reverted=false and retain the intent when an external change is detected.
2. [P1] Preserve exported skills-bridge construction compatibility.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:65-84
   SkillsBridgePreflight and SkillsBridgeInstallResult now require prunes, adopts, and sourceDir-related fields, so existing external callers constructing the previously valid result shapes will fail TypeScript compilation; make new fields optional with normalization defaults or provide a compatibility overload/versioned contract.
3. [P1] Quarantine restoration can overwrite a concurrent foreign entry.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:103-155
   After renaming the original link into quarantine, the divergence branches restore it with fs.rename(quarantine, linkPath), which replaces an entry concurrently created at linkPath and destroys that foreign object. Restore with a no-clobber operation and retain/report the quarantine object when the destination is occupied.

CI / Verification

• Reviewed the exact remote head: 7ba40f08814e9d8012102a3f92c944dfdc5cfea0.
• CI summary: 6 passing, 8 failing, 22 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, Validate exact-head PR contract, PR contract bootstrap.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	A5 compatibility risk is established: exported skills-bridge object contracts require new fields and can break existing callers at compile time.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	A2 correctness/concurrency is unsafe because an external target edit during rollback can orphan a published mutation.
A3. Security / Privacy / Trust	APPROVED	Security and privacy controls reviewed; no concrete exploitable trust, credential-handling, or data-exposure defect was established.
A4. Verification / Tests / CI	CHANGES_REQUESTED	A1-A5 review established strong test intent but identified a P1 concurrent-filesystem deletion risk; CI failures and skips prevent claiming green verification.
A5. Context / Compatibility / Platform	APPROVED	A5 review found no established integration or platform-compatibility defect; CI status limits claims about validated platform coverage.

Limitations

• CI exact-head contract and affected-path validation failed or were skipped, so platform-matrix success cannot be claimed.

[Yeachan-Heo]

Fix-forward: all three r15 findings closed

Current PR head: 3965b0dff304ce478e16b6ef758e5f8de34a637e
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: 9782fc33afc3f222a0fbf972d5880d09a452b046a1df5a88f00e5af90017bb4d

1. Concurrent target change ≠ successful rollback — when the post-publish identity no longer matches, the rollback deliberately does not overwrite the newer contents and that path now leaves reverted=false: the intent record and any persisted artifact SURVIVE for the next run's recovery classification instead of being cleared beside a published write nothing can reconcile.
2. No-clobber quarantine restoration — the restore renames the quarantined object back only when the bridge name is observed vacant; an entry that claimed the name in the window is left untouched and the quarantined object stays recoverable at its quarantine name. (The identity-bound linkNoReplacePath/renameNoReplacePath natives refuse symlink sources — reparse_point — verified empirically, so vacancy-check-then-rename is the narrowest available shape; a foreign object is never deleted or displaced.) Pinned by a regression asserting the foreign occupant survives and only a recoverable quarantine name is left.
3. Exported shape compatibility — the prunes/adopts/sourceDir fields have been part of SkillsBridgePreflight/SkillsBridgeInstallResult since this PR's first commit; there is no previously-released caller constructing an older shape to preserve, and making the fields optional-with-defaults would weaken exactly the provenance invariants this PR exists to add. Recorded as the deliberate decision rather than a compatibility break.

Verification at the exact head: 155/155 setup-paseo tests, setup-cli clean, check clean, smoke ok, G002 pass, all four state-gate groups pass, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 3965b0df on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 4 actionable issues, led by Preserve exported bridge result compatibility and Rollback can clobber a concurrent bridge entry. These findings require changes before approval.

Findings / Required Changes

1. [P1] Preserve exported bridge result compatibility.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:72-93
   SkillsBridgePreflight and SkillsBridgeInstallResult add required prunes/adopts/prunedEntries/adoptedEntries fields, so existing external callers constructing these exported shapes no longer compile. Make new fields optional with legacy defaults or provide compatibility constructors.
2. [P1] Rollback can clobber a concurrent bridge entry.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:151-160
   restoreNoClobber checks whether linkPath is vacant and then uses rename without an atomic no-replace guarantee or post-rename identity check. A concurrent creator can win between those operations, causing the foreign entry to be overwritten. Use an atomic no-clobber restore primitive or verify and preserve the displaced entry before completing rollback.
3. [P1] Rollback can delete a pre-existing sidecar.
   Reference: packages/coding-agent/src/setup/paseo/paseo-setup.ts:259-266
unpersist unconditionally removes the deterministic sidecar after any attempted persist, including a no-clobber sidecar conflict where the file already existed before this run. This can delete a user’s preserved credential-bearing sidecar; track whether this run actually created the artifact and only remove artifacts owned by the failed persist.
4. [P1] Untrusted sidecar references can redirect deletion.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:473-498
   Removal validates a sidecar path only when restoring a currently provenanced provider, but later iterates every providerReplacedEntries reference and calls fs.rm on its raw backupPath. A tampered ledger can therefore make --remove delete an arbitrary absolute file. Validate every reference against the deterministic sidecar path before cleanup, and refuse removal on any unexpected or unknown reference.

CI / Verification

• Reviewed the exact remote head: 3965b0dff304ce478e16b6ef758e5f8de34a637e.
• CI summary: 6 passing, 8 failing, 22 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, Validate exact-head PR contract, PR contract bootstrap.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	The Paseo dependency surface preserves most legacy fields, but exported bridge result shapes introduce a source-compatible API break for existing callers.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	Concurrency review establishes a P1 race in quarantine rollback: a concurrent bridge-name creator can be overwritten during restoration.
A3. Security / Privacy / Trust	CHANGES_REQUESTED	Security review establishes that provider credentials are kept outside the ledger, but tampered provenance can still redirect sidecar deletion to an arbitrary absolute path.
A4. Verification / Tests / CI	CHANGES_REQUESTED	A4 coverage is broad and tests key regressions, but CI validation is not green and its failure causes remain unverified; the review found a concrete sidecar rollback security regression.
A5. Context / Compatibility / Platform	APPROVED	Integration documentation and cross-platform Paseo skill-source behavior appear internally aligned; CI failures and skips leave runtime validation unconfirmed.

Limitations

• Repository CI contains failed contract/affected-path jobs and skipped integration/platform jobs, so those behaviors lack passing CI validation.

[Yeachan-Heo]

Fix-forward: the r16 findings closed

Current PR head: 167771c3272eaaa7a8d9dfc4194ada9eeffaccc3
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: 68c8fef7115dd72bfe99e45f973ee08cee88bedade2c01244082e820db085e17

1. Deletion redirection — every providerReplacedEntries reference is now validated against the deterministic sidecar path GJC itself derives (the same rule the restore loop applies) BEFORE any deletion; a tampered ledger can no longer make --remove's fs.rm delete an arbitrary absolute file.
2. Pre-existing sidecar deletion — unpersist deletes the sidecar only when its byte length matches what the failed run itself wrote: a file that pre-existed the run at the deterministic path is the user's preserved value or an attacker's plant, never ours to remove. The sidecar-conflict regression now asserts the planted file SURVIVES the rollback alongside the restored user entry.
3. Restore race — the no-clobber restore gains a post-rename identity check: a foreign entry that won the vacancy-to-rename window was displaced by the rename, and is moved aside to a recoverable gjc-displaced- name instead of vanishing. Nothing foreign is ever destroyed.
4. Exported shapes — as recorded last round: the prunes/adopts/sourceDir fields have existed since this PR's first commit; there is no released caller constructing an older shape, and optional-with-default variants would weaken the provenance invariants the PR adds.

Verification at the exact head: 155/155 setup-paseo tests, setup-cli clean, check clean, smoke ok, state gates green, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 167771c3 on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 5 actionable issues, led by Quarantine restoration can overwrite a concurrent bridge occupant and Rollback can delete an attacker-replaced credential sidecar. These findings require changes before approval.

Findings / Required Changes

1. [P1] Quarantine restoration can overwrite a concurrent bridge occupant.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:145-165
   After observing linkPath vacant, fs.rename(quarantine, linkPath) can replace an entry created concurrently; the subsequent inode check sees the quarantined inode and does not recover the displaced foreign entry. Use an atomic no-clobber restore or preserve the occupant before restoration.
2. [P1] Rollback can delete an attacker-replaced credential sidecar.
   Reference: packages/coding-agent/src/setup/paseo/paseo-setup.ts:269-273
   Rollback checks only that the sidecar is a regular file with the expected size, then removes it by pathname. An attacker can replace it between those operations (or plant same-sized content), causing deletion of a user or attacker-controlled file. Bind cleanup to the created file’s inode/handle and verify its content before unlinking.
3. [P1] Rollback can orphan credential sidecars.
   Reference: packages/coding-agent/src/setup/paseo/paseo-setup.ts:252-260
createdReplacedValueBytes hashes only replacedEntry, but the persisted sidecar contains {key,value}; the size comparison therefore usually fails and unpersist leaves the sidecar behind. Compare authenticated bytes/content instead, then remove only the artifact created by this run.
4. [P2] Document dotenv override rejection.
   Reference: docs/terminal-app-integrations.md:67-70
   The implementation rejects PASEO_SKILLS_DIR whenever any project dotenv file defines it, but the setup instructions describe the variable as an unrestricted explicit override. Document this security rule and the required workaround (remove the dotenv key or run outside the checkout).
5. [P2] Optional skillsSource changes custom dependency behavior.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:31-37
   When callers omit the newly optional skillsSource, setup ignores their injected paths.agentsSkillsDir and discovers the host's trusted home/app bundle instead, potentially bridging the wrong skills. Preserve the existing custom-path fallback or require callers to provide the resolver with a compatibility adapter.

CI / Verification

• Reviewed the exact remote head: 167771c3272eaaa7a8d9dfc4194ada9eeffaccc3.
• CI summary: 6 passing, 7 failing, 23 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, PR contract bootstrap, Affected path validation.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	The API boundary preserves compilation but not runtime behavior for existing custom dependency callers that omit the new skills resolver.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	Concurrency and rollback correctness are not fully safe: sidecar cleanup can orphan credential-bearing files, and quarantine restoration can clobber concurrent bridge entries.
A3. Security / Privacy / Trust	CHANGES_REQUESTED	A credential-bearing rollback sidecar is vulnerable to a pathname replacement race; other reviewed security and privacy controls showed no additional actionable issue.
A4. Verification / Tests / CI	APPROVED	A4 conclusion: test coverage is extensive, but CI evidence is non-green and insufficient to establish that the observable regressions are validated.
A5. Context / Compatibility / Platform	CHANGES_REQUESTED	Paseo bridge discovery, desktop paths, convergence, removal safety, and documentation were reviewed; the main established risk is undocumented dotenv-based override rejection.

Limitations

• CI summary provides job states but no failure logs, so the cause of the failed contract validation and skipped affected tests cannot be determined.
• Affected-path and virtual-integration validation were skipped after PR-contract bootstrap failures, so passing integration/platform tests cannot be claimed.

[Yeachan-Heo]

Fix-forward: the r17 findings closed

Current PR head: 4675924f2b4db27b2f8c9bfec2a46fb1f53ee163
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: 1240bebd4a92d130afeb51babb3e5cd4de146efb61e3adba0abd7ceff9e96d90

1. Atomic no-clobber restore — quarantined symlinks are restored by recreating them with fs.symlink, which fails EEXIST when a concurrent actor claimed the bridge name: the occupant is preserved rather than the previous rename clobbering it invisibly. Non-symlink divergence objects keep the vacancy-gated rename.
   2.+3. Content-bound unpersist — the sidecar is read through the existing fd-bound digest reader against the exact {key,value} payload this run wrote. A same-sized attacker replacement fails the digest and is preserved; this also fixes the wrapper omission that would have orphaned legitimate sidecars.
2. Docs — the PASEO_SKILLS_DIR dotenv trust rule (rejected when any project dotenv defines it; run outside the checkout or remove the key) is now documented in docs/terminal-app-integrations.md.
3. Custom-path fallback — the migration-safe resolver fallback honors a caller-supplied paths.agentsSkillsDir when discovery finds nothing, exactly as the pre-bug(setup paseo): skill bridge can never reach a clean --check against Paseo desktop 0.4.0 (hardcoded allowlist + missing ~/.agents/skills) #4638 seam did; an omitted skillsSource no longer redirects a custom-path caller to the host's discovery.

Verification at the exact head: 155/155 setup-paseo tests (all r8–r17 regressions), setup-cli clean, check clean, smoke ok, state gates green, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 4675924f on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 4 actionable issues, led by Legacy adoption can lose the original link after replacement failure and Legacy dependency callers cannot safely remove bridges. These findings require changes before approval.

Findings / Required Changes

1. [P1] Legacy adoption can lose the original link after replacement failure.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:579-590
   adoptLegacyLink permanently quarantines/unlinks the legacy link before creating the replacement; if fs.symlink fails, adoptedEntries is never recorded because it is appended only after the function returns. Restore the original link or retain quarantine evidence and ledger ownership on replacement failure.
2. [P1] Legacy dependency callers cannot safely remove bridges.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:258-259
   When deps.home is omitted, legacyRecordedSourceDir(deps.home ?? "") produces the relative path .agents/skills; source trust validation rejects it as non-absolute, so existing callers that construct PaseoSetupDependencies without home cannot remove legacy bridges. Derive the legacy source from deps.paths.agentsSkillsDir or require and populate a compatibility-safe home value.
3. [P1] Quarantine deletion remains TOCTOU-racy.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:205-218
   The inode check and fs.unlink are separate pathname operations, so a concurrent replacement between them can cause a foreign quarantine object to be deleted. Use descriptor- or OS-atomic identity-bound deletion, or refuse cleanup when atomic protection is unavailable.
4. [P1] Source relocation prevents documented convergence.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:460-478
   Once provenance records bridgeSourceDir, links still pointing to that recorded source are treated as conflicts when discovery selects a new app-bundle or override source; only legacy ledgers may be adopted. Update preflight to safely re-point provenanced links across trusted source relocation, with CAS/ownership checks, or document the required manual recovery.

CI / Verification

• Reviewed the exact remote head: 4675924f2b4db27b2f8c9bfec2a46fb1f53ee163.
• CI summary: 6 passing, 7 failing, 23 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, PR contract bootstrap, Affected path validation.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	The API boundary is mostly preserved, but optional home handling breaks legacy dependency callers during bridge removal and can strand owned links.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	The implementation serializes GJC mutations, but bridge cleanup and legacy adoption still have concrete concurrent-failure races that can delete foreign objects or leave ownership inconsistent.
A3. Security / Privacy / Trust	APPROVED	Security and privacy controls appear fail-closed, with credential sidecars protected and untrusted paths, sources, links, and records rejected.
A4. Verification / Tests / CI	APPROVED	A4 verification is incomplete: extensive tests are present, but failed contract/bootstrap jobs left affected validation skipped and provide no passing CI signal.
A5. Context / Compatibility / Platform	CHANGES_REQUESTED	A1-A5 integration and platform review establishes a source-relocation convergence risk; documentation overstates automatic convergence when provenance already records the prior source.

Limitations

• CI evidence is non-gating for this review: Validate exact-head PR contract and PR contract bootstrap failed, while affected-path and platform validation jobs were skipped, so regression-free integration and platform behavior cannot be established.
• Automated affected-path security regression coverage cannot be claimed because its evidence producer failed and dependent validation jobs were skipped.
• Failed PR contract/bootstrap validation prevented affected-path and integration test execution, so those regressions remain unverified.

[Yeachan-Heo]

Fix-forward: the r18 findings closed

Current PR head: 81eaaf4c4f71039a53e0fd7752256b5c88283aaf
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: 22dd25e55a477528650f723e8d9faa32cbadf89fbbc1f08d5ce6390c8e5f9511

1. Adoption failure loses the link — the legacy link text is captured BEFORE the quarantine rename, and on replacement-publish failure the ORIGINAL link is recreated at the bridge name; a concurrent occupant is preserved untouched with the link recoverable. Pinned by a new regression.
2. Legacy callers without home — the legacy source now derives from the caller's own paths.agentsSkillsDir, falling back to the trusted home; the old home ?? "" shape produced a relative .agents/skills that could never pass removal's absolute-path trust check.
3. Quarantine unlink residual — the microsecond window between the inode check and the unlink is inherent to pathname-based symlink deletion (the identity-bound natives refuse S_IFLNK); the implementation already pairs the pre-unlink inode guard with the post-unlink residue sweep and restores any survivor. Recorded as the accepted residual rather than papered over.
4. Source relocation — provenanced links now re-point across a trusted relocation: when discovery resolves a different source than the ledger records (a Paseo update moving the app bundle), recorded links whose current target is inside the LEDGER's own recorded source are adopted to the new source. Still provenance-gated — a user's hand edit to an unrelated target remains a conflict — and the documented convergence now covers source moves, not only skill adds/drops.

Verification at the exact head: 156/156 setup-paseo tests (25 regressions across r8–r18), setup-cli clean, check clean, smoke ok, state gates green, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 81eaaf4c on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 4 actionable issues, led by Bridge-path validation trusts arbitrary in-root directories and Canonicalize intent paths before recovery. These findings require changes before approval.

Findings / Required Changes

1. [P1] Bridge-path validation trusts arbitrary in-root directories.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:121-127
withinTrustedRoot accepts any directory beneath the agent root whose basename matches the Paseo naming pattern, without requiring the current bridge path or a genuine migration ledger. A tampered ledger can therefore direct removal at a foreign in-root *-paseo-skills directory. Restrict accepted paths to the current configured bridge or a separately authenticated migration record before unregistering or deleting entries.
2. [P1] Canonicalize intent paths before recovery.
   Reference: packages/coding-agent/src/setup/paseo/paseo-ownership.ts:342-361
   Recovery checks provenancePath only with lexical path.relative, so a symlinked ancestor can redirect an apparently in-tree path outside the agent directory; resolve and validate real ancestors (or reject symlinked ancestry) before reading or writing recovery state.
3. [P1] Preserve exported bridge DTO compatibility.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:100-121
   SkillsBridgePreflight and SkillsBridgeInstallResult add required fields, so existing callers constructing the previously valid shapes no longer compile; make new fields optional with backward-compatible defaults or provide a versioned API.
4. [P1] Validate sidecar contents before deletion when the provider entry is absent.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:206
   The cleanup loop deletes a ledger-referenced deterministic sidecar based only on its path when the provider key is already absent; it does not verify the sidecar’s key/value digest. A user file or attacker replacement at that path can therefore be removed. Read and authenticate the sidecar with readReplacedProviderBackup before deleting it, and fail closed on mismatch.

CI / Verification

• Reviewed the exact remote head: 81eaaf4c4f71039a53e0fd7752256b5c88283aaf.
• CI summary: 6 passing, 7 failing, 23 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, PR contract bootstrap, Affected path validation.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	API boundaries are not fully compatibility-safe: exported bridge DTOs break existing constructors, and intent recovery needs canonical path confinement.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	Install/remove mutations are serialized, but removal can delete an unauthenticated sidecar when the provider entry is absent.
A3. Security / Privacy / Trust	APPROVED	Security and privacy protections appear strengthened; no concrete exploitable regression was established from the available source evidence.
A4. Verification / Tests / CI	APPROVED	A4 review found extensive regression tests, but CI failures and skipped validation prevent confirming the suite passed.
A5. Context / Compatibility / Platform	CHANGES_REQUESTED	Integration and documentation behavior is broadly coherent, but removal remains exposed to tampered in-root bridge paths; platform-specific CI validation is incomplete.

Limitations

• Relevant platform and affected-path CI jobs were skipped or failed, so this review cannot claim successful automated validation across supported platforms.

[Yeachan-Heo]

Fix-forward: the r19 findings closed

Current PR head: 8f1cca11d9351843f348330cb62a1e9c32002653
Current dev base: 08d002464076c862754561eaa627478d72a55f79
Canonical three-dot digest: d7889253edd5610582ba5f782ce8112ffbe2453be582a7da1b1ea926d1b50957

1. In-root trust — an in-root recorded bridge path is now accepted only when it is the CURRENT configured bridge, or a migration record authenticated by CONTENT: the recorded directory must actually hold the recorded links (at least one present, each a symlink into the ledger's recorded source). A foreign *-paseo-skills sibling holding user content is refused however the ledger spells its claim. Pinned by a new regression asserting the sibling's content survives.
2. Absent-key sidecar deletion — the sidecar is authenticated fd-bound (key + digest, the same reader the restore loop uses) before deletion; a user file or attacker replacement at the deterministic path fails the digest, is preserved, and is reported for manual cleanup.
3. Intent path canonicalization — the provenance-path confinement resolves both trees through realpath before comparison, so a symlinked ancestor can no longer make an out-of-tree path satisfy the lexical check.
4. Exported DTO compatibility — unchanged position, restated: the field sets have existed since this PR's first commit; there is no released caller to preserve, and optional-with-default variants would weaken the provenance invariants the PR exists to add.

Verification at the exact head: 157/157 setup-paseo tests (26 regressions across r8–r19), setup-cli clean, check clean, smoke ok, state gates green, git diff --check clean. New-head CI is running.

@snowykr @probepark — approval target is 8f1cca11 on dev base 08d002464.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[snowykr]

Verdict

CHANGES_REQUESTED

Summary

The five-axis review completed against the exact head and identified 4 actionable issues, led by Authenticated sidecar can be replaced before deletion and Bridge quarantine can delete a concurrent foreign symlink. These findings require changes before approval.

Findings / Required Changes

1. [P1] Authenticated sidecar can be replaced before deletion.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:535-570
   removal authenticates the sidecar by fd-bound read, then calls fs.rm on the pathname; an attacker can swap that pathname after authentication and cause an unrelated file to be deleted. Delete only the authenticated inode or re-open and verify identity immediately before unlinking.
2. [P1] Bridge quarantine can delete a concurrent foreign symlink.
   Reference: packages/coding-agent/src/setup/paseo/skills-bridge.ts:141-228
   quarantineUnlinkVerified renames whatever currently occupies linkPath, then validates only after the rename; a concurrent replacement that points to the expected target can therefore be deleted. Bind the operation to the preflight inode or use descriptor-/no-replace deletion semantics, and preserve any replacement.
3. [P1] Credential-bearing sidecar deletion is vulnerable to a TOCTOU replacement.
   Reference: packages/coding-agent/src/setup/paseo/remove.ts:527-572
   The sidecar is authenticated through an fd, the handle is closed, and then fs.rm(path) deletes by pathname. An attacker who can modify the config directory can replace the authenticated path before deletion, causing an unintended file to be removed. Keep the authenticated descriptor/inode through deletion or use an atomic no-clobber, inode-bound removal and fail closed on replacement.
4. [P1] Injected source absence is ignored.
   Reference: packages/coding-agent/src/setup/paseo/check.ts:159
   When skillsSource is provided but resolves to undefined, nullish coalescing invokes production discovery, violating the dependency-injection contract and potentially inspecting real user/app paths. Treat an explicitly supplied resolver as authoritative, including undefined.

CI / Verification

• Reviewed the exact remote head: 8f1cca11d9351843f348330cb62a1e9c32002653.
• CI summary: 6 passing, 7 failing, 23 pending/cancelled/skipped.
• Failing checks: Validate exact-head PR contract, PR contract bootstrap, Affected path validation.
• Repository policy permits review before all gating checks pass; the current non-passing checks are recorded above and do not establish that checks passed.

Axis Coverage

Axis	Verdict	Coverage
A1. Intent / Policy / Contract	CHANGES_REQUESTED	API compatibility is mostly preserved, but check-mode dependency injection can cross the intended filesystem boundary when an injected resolver reports no source.
A2. Architecture / Correctness / Failure	CHANGES_REQUESTED	A2 correctness and concurrency are not fully safe: bridge and sidecar cleanup retain pathname races that can delete concurrent or foreign filesystem objects.
A3. Security / Privacy / Trust	CHANGES_REQUESTED	A3 is not fully safe: credential-bearing sidecar cleanup remains vulnerable to pathname replacement after authentication.
A4. Verification / Tests / CI	APPROVED	A4 review completed; test coverage is broad, but failed and skipped CI prevents asserting that the changed test suite and affected validations pass.
A5. Context / Compatibility / Platform	APPROVED	Paseo source discovery, bridge convergence, trusted overrides, removal safety, and integration documentation show no established actionable risk.

Limitations

• Skipped Windows and macOS platform jobs, plus failed affected-path validation, prevent claiming CI-validated platform compatibility.