fix(adapter-utils): repair referenced-source ignore resolution type break and symlink descendant check (#12201)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Referenced projects are staged into run sandboxes, and a recent
change made each staging site resolve the project's Git-ignored paths
explicitly (`ignoreResolution` on `SandboxAdditionalSource`)
> - One test call site was left without the now-required property, so
`tsc` fails in adapter-utils and every Build/Typecheck CI job on master
is red (surfaced when the TypeScript 7 bump landed in the same window)
> - Separately, the new descendant check compares a logical caller path
against the physical toplevel git prints, so any symlinked path fails
ignore resolution spuriously — three of the suite's own tests fail on
macOS because temp dirs live under the `/var` → `/private/var` symlink
> - This pull request supplies the missing property at the test call
site and makes the descendant comparison symlink-safe via realpath on
both sides
> - The benefit is a green master again, plus referenced-project staging
that works from symlinked checkouts and temp directories
## Linked Issues or Issue Description
No public issue exists; the underlying problem follows the bug-report
template.
**What happened?**
`packages/adapter-utils` fails `tsc` on master:
`src/sandbox-managed-runtime.test.ts(2284,29): error TS2741: Property
'ignoreResolution' is missing in type '{ localPath: string; projectId:
string; }' but required in type 'SandboxAdditionalSource'.` Every
Build/Typecheck CI job is red. Independently,
`resolveReferencedSourceIgnore` returns `{ kind: "failed", reason:
"referenced project path is not a descendant of its own Git top level:
/var/... under /private/var/..." }` for any symlinked project path, and
three tests in the suite fail on macOS.
**Expected behavior**
Master typechecks. A referenced project whose path reaches git through a
symlink (macOS temp dirs, symlinked checkouts) resolves its ignore set
normally, and the descendant check still fails closed for genuinely
foreign paths.
**Steps to reproduce**
1. `pnpm --filter @paperclipai/adapter-utils exec tsc --noEmit` on
master — TS2741 at `sandbox-managed-runtime.test.ts:2284`.
2. On macOS: `pnpm vitest run
packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — three
`resolveReferencedSourceIgnore` tests fail with the "not a descendant"
reason above.
**Paperclip version or commit**
master `29d12045f`.
## What Changed
- `sandbox-managed-runtime.test.ts:2284`: the one call site missing
`ignoreResolution` now passes `{ kind: "other" }`, matching every
sibling call site from the same change.
- `sandbox-managed-runtime.ts`: `resolveReferencedSourceIgnore` resolves
both the git toplevel and the caller's `localPath` through a new
`physicalPath` helper (realpath with a resolve fallback) before the
descendant comparison. Git prints physical toplevels, so both sides must
be physical; the fallback keeps the check failing closed when a path
vanishes mid-run.
## Verification
- `pnpm vitest run
packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 61/61 pass
on macOS (previously 58 passing, 3 failing, plus the typecheck break).
- `tsc --noEmit` in `packages/adapter-utils` is clean.
## Risks
- Low. The behavioral change is confined to path normalization before an
existing comparison; a realpath failure falls back to the prior string
comparison, so the fail-closed property is preserved.
## Model Used
Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — agentic
coding session with tool use (code search, editing, local test
execution).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
parent
29d12045f4
commit
a5c2add7be
|
|
@ -2281,7 +2281,7 @@ describe("sandbox managed runtime", () => {
|
|||
adapterKey: "test-adapter",
|
||||
client,
|
||||
workspaceLocalDir: localWorkspaceDir,
|
||||
additionalSources: [{ localPath: referencedDir, projectId: "proj-first" }],
|
||||
additionalSources: [{ localPath: referencedDir, projectId: "proj-first", ignoreResolution: { kind: "other" } }],
|
||||
onProgress: (line) => { lines.push(line); },
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -212,6 +212,23 @@ export function referencedSourceIgnoreExcludeEntries(resolution: ReferencedSourc
|
|||
* are otherwise not comparable. The caller treats `null` as a resolution
|
||||
* failure (fail closed), never as "nothing to exclude".
|
||||
*/
|
||||
/**
|
||||
* Physical (symlink-resolved) form of a path, falling back to the plain
|
||||
* resolved form when realpath fails (e.g. the path vanished mid-run — the
|
||||
* downstream descendant check then fails closed on the string form).
|
||||
* Git prints physical toplevels, so both sides of the descendant comparison
|
||||
* must be physical too; comparing a logical path against a physical one makes
|
||||
* any symlinked project or temp directory (macOS `/var` → `/private/var`)
|
||||
* fail resolution spuriously.
|
||||
*/
|
||||
async function physicalPath(input: string): Promise<string> {
|
||||
try {
|
||||
return await fs.realpath(input);
|
||||
} catch {
|
||||
return path.resolve(input);
|
||||
}
|
||||
}
|
||||
|
||||
function relativizeUnderGitToplevel(input: { toplevel: string; localPath: string }): string | null {
|
||||
const toplevel = path.resolve(input.toplevel);
|
||||
const localPath = path.resolve(input.localPath);
|
||||
|
|
@ -263,7 +280,10 @@ export async function resolveReferencedSourceIgnore(localPath: string): Promise<
|
|||
if (!scan) {
|
||||
return { kind: "other" };
|
||||
}
|
||||
const offset = relativizeUnderGitToplevel({ toplevel: scan.toplevel, localPath });
|
||||
const offset = relativizeUnderGitToplevel({
|
||||
toplevel: await physicalPath(scan.toplevel),
|
||||
localPath: await physicalPath(localPath),
|
||||
});
|
||||
if (offset === null) {
|
||||
return {
|
||||
kind: "failed",
|
||||
|
|
|
|||
Loading…
Reference in New Issue