Export WARP_SKILL_DIRS as absolute paths to the agent shell - #15256
Draft
warp-agent-staging[bot] wants to merge 2 commits into
Draft
Export WARP_SKILL_DIRS as absolute paths to the agent shell#15256warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
WARP_SKILL_DIRS is injected holding paths relative to the environment working directory. The driver's own skill loader resolves those against working_dir, but the raw relative value also leaks into the agent's shell, where skills read it directly (e.g. the github skill's sed/cd snippet that locates the factory's factory.yaml). Once an agent cd's into a product repo, those relative paths resolve to nothing, so skill resolution silently yields empty and the factory label / footer is lost. The environment working directory is only known absolutely on the client, so resolve there: in AgentDriver::new, re-export an absolute WARP_SKILL_DIRS into the agent's environment, resolved against working_dir via the existing resolve_skills_dirs helper. This flows through the terminal driver and third-party harness runners, so Oz, Claude, and Codex agents all see absolute paths regardless of cwd. The server-side relative value and the driver's own loader are unchanged. Fixes warpdotdev/warp-server#15372 Co-Authored-By: Warp <agent@warp.dev>
Two adversarial-review findings on the WARP_SKILL_DIRS fix: 1. Correctness: the absolute value was assembled with `to_string_lossy`, so a `working_dir` containing non-UTF-8 bytes (possible on Unix) would be corrupted with U+FFFD replacement characters, exporting paths that no longer name the real skill directories. Assemble the comma-separated value directly as an `OsString` from each path's raw `OsStr` bytes. 2. Test isolation: the tests mutated the global `WARP_SKILL_DIRS` via unsafe `set_var`/`remove_var` (only loosely coordinated by `serial_test`) and unconditionally deleted any inherited value. Split a pure `absolute_skill_dirs(working_dir, dirs)` from the thin env-reading wrapper and drive the tests through it with `PathBuf` inputs, removing all process-env mutation and the serial attribute. Also adds a `cfg(unix)` regression test that builds a non-UTF-8 working directory and asserts the raw 0xff byte survives with no U+FFFD replacement sequence. Fixes warpdotdev/warp-server#15372 Co-Authored-By: Warp <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
WARP_SKILL_DIRSis injected into a Factory agent's environment holding paths relative to the environment working directory (e.g.factory-dev/frank/agents/implementation/skills,factory-dev/frank/skills). The driver's own skill loader already resolves these againstworking_dir, but the raw relative value also leaks into the agent's shell, where skills read it directly — for example thegithubskill'ssed/cdsnippet that parsesWARP_SKILL_DIRSto locate the factory'sfactory.yamland resolve the factoryname/alias.Agents routinely
cdinto a product repo. Once they do, the relative entries resolve against the wrong directory,sedreads nothing, and skill resolution silently yields empty strings — producing a bare factory label or a missing footer (SEV2, warpdotdev/warp-server#15372).The environment working directory is only known absolutely on the client (the server injects a relative value because it cannot know the sandbox mount path). So this fixes it at the authoritative place that can resolve it:
AgentDriver::newre-exports an absoluteWARP_SKILL_DIRSinto the agent's environment, resolved againstworking_dir. Because this value flows through the terminal driver and the third‑party harness runners, Oz, Claude, and Codex agents all see absolute paths regardless of their current working directory. The server-side relative value and the driver's own loader are left unchanged.Linked Issue
warpdotdev/warp-server#15372
ready-to-specorready-to-implement.Changes
absolute_skill_dirs(working_dir, dirs)helper inapp/src/ai/agent_sdk/driver.rsthat resolves relative entries againstworking_dir(absolute entries pass through) and re-joins them.absolute_skill_dirs_envis now a thin env-reading wrapper over it.OsStringfrom each path's rawOsStrbytes — not viato_string_lossy— so aworking_dircontaining non-UTF-8 bytes (possible on Unix) is preserved verbatim instead of being corrupted withU+FFFD.AgentDriver::newinserts that absolute value into the agent's resolved env vars, overriding the relative value inherited from the environment. ReturnsNone/no-op whenWARP_SKILL_DIRSis unset or empty.Testing
Non-UI change (environment-variable resolution), so no visual proof applies.
Regression tests in
app/src/ai/agent_sdk/driver_tests.rs(all drive the pure helper withPathBufinputs — no globalset_var/remove_var, noserial):absolute_skill_dirs_makes_relative_entries_absolute— a relative value like the reported one becomes absolute (joined onto the working dir), which is exactly what makes the shell snippet resolve from any cwd.absolute_skill_dirs_passes_absolute_entries_through— absolute entries are untouched; only relative ones are joined.absolute_skill_dirs_empty_is_none— empty input leaves the variable untouched.absolute_skill_dirs_preserves_non_utf8_working_dir(cfg(unix)) — a non-UTF-8 working dir keeps its raw0xffbyte with noU+FFFDreplacement sequence.Commands run (in
warp/):cargo test -p warp --lib absolute_skill_dirs→4 passed; 0 failedcargo check -p warp --lib→ clean./script/formatandcargo fmt -p warp -- --check→ cleancargo clippy -p warp --lib→ cleanI have manually tested my changes locally with
./script/runAgent Mode
This PR was created by Oz (running Codex).