Opt Claude Code agents out of the operator's plugins and skills - #61
Draft
TON14 wants to merge 19 commits into
Draft
Opt Claude Code agents out of the operator's plugins and skills#61TON14 wants to merge 19 commits into
TON14 wants to merge 19 commits into
Conversation
Windows still defaults stdout to cp1252, which turns the routing summary's separators into `?` and makes Chinese prompt output unprintable.
Agent commands were built as POSIX shell strings and handed to `create_subprocess_shell`, which is cmd.exe on Windows, so `mkdir -p` failed on the first call. Process control used `os.killpg`/`SIGKILL`/`SIGHUP`, none of which exist there, so every timeout and Ctrl+C raised AttributeError. Rather than translate the shell strings per platform, remove the shell from the agent path entirely. Everything it was doing is a real subprocess argument: cd <ws> && ... -> cwd= VAR=value <cmd> -> env= layered onto the scrubbed parent environment ... < prompt.md -> the prompt is written to the child's stdin mkdir -p / chmod -> native pathlib calls on the local environment Commands are argv lists now, so `shlex.quote` is gone and no model-supplied value (model id, path, MCP config) can reach a shell parser. `Environment.exec` stays as an explicit escape hatch for callers that genuinely want a shell. Also fixed, all found by running it for real on Windows: - process_group: platform split, CREATE_NEW_PROCESS_GROUP plus `taskkill /T`, and named operations instead of POSIX-only signal numbers. - MAX_PATH: run directories reach 260 characters easily, and the failure is deceptive because mkdir succeeds on the shorter parent while open() on the file inside raises FileNotFoundError. New utils/paths applies the \\?\ prefix, but only to paths that actually exceed the limit. - screenshot(): PowerShell on Windows, screencapture on macOS, the existing X11 tools on Linux. - Deny rules for the claude_code backend now also emit the drive-letter form, so harness-owned paths are actually hidden from the auditor on Windows. Adds tests over LocalEnvironment, the process-group primitives and adapter argv construction, none of which had any coverage before. Test doubles for agent CLIs move to tests/fake_cli, which ships a .cmd shim on Windows because CreateProcess cannot run a `#!/bin/sh` script. BREAKING: CommandAgentAdapter takes argv=/env= instead of command_template=. `EpisodeResult.metadata["command"]` is now the argv list rather than a string; existing readers already accept both.
`_ensure_dir_nofollow`, `_atomic_bytes_write`, `_append_jsonl`, the worker-log open, the dashboard's approval and artifact readers and the manager's event append all require O_NOFOLLOW, O_DIRECTORY and dir_fd, so the run tree could not be written on Windows at all. Each grows a Windows branch that keeps what the platform can express -- reparse-point refusal, atomic replace, the hard-link check before truncation, and the same event-id sequencing -- and documents the one guarantee that cannot be reproduced without directory descriptors: the check-then-use window is not anchored. POSIX behaviour is untouched. Stop/abort go through the process-group helpers instead of os.killpg with raw signal numbers, and name the two signals so a receipt still distinguishes a stop from an abort on a platform that has no SIGKILL.
Creating a symlink needs SeCreateSymbolicLinkPrivilege, which an ordinary account only holds under Developer Mode, so the no-follow tests failed on the fixture rather than on the behaviour they cover. They now skip with a reason where the privilege is missing and run unchanged where it exists. Only the test body is wrapped: pytest's own tmp_path bookkeeping also makes symlinks, and swallowing that would skip the entire suite. The Content-Disposition fixture also asked for a filename containing a quote, which Windows reserves outright; that half of the fixture now only runs on POSIX.
Agent CLIs are launched as plain subprocesses now, so command construction behaves identically on every platform, and run directories escape MAX_PATH automatically.
A run died the moment the provider answered 429, overloaded, or dropped the
connection, which on a long-horizon task throws away hours of work for a
condition that clears on its own.
`_run_role_episode` now backs off exponentially for the two failure kinds that
are actually transient -- rate_limit and network -- and retries: 60s doubling to
a 900s cap, up to 8 attempts or 2 hours total, tunable through
LH_HARNESS_PROVIDER_RETRY_{MAX_ATTEMPTS,BASE_SECONDS,CAP_SECONDS,MAX_TOTAL_SECONDS}
with MAX_ATTEMPTS=0 restoring the old fail-fast behaviour. Terminal kinds
(authentication, quota, model_unavailable, timeout) return immediately as
before; retrying those wastes time or masks a real hang. The wait is visible:
an `agent_runtime_retry` event and a `role_retry` progress record carry the
attempt, the delay and the provider's own message, and a cancel during backoff
still returns a cancelled episode.
Classification also learns the subscription-limit shapes that were previously
read as success: 529, a session/usage limit result whose `is_error` is false but
whose `api_error_status` is 429, and a `rate_limit_event` record whose status is
"rejected".
A Python 3.13+ venv Scripts\python.exe is a launcher that starts the real interpreter as a child process. The supervisor records the launcher pid from Popen, while the worker compares it to os.getpid() of the real interpreter, so every supervised run died with "reservation belongs to another process". The worker identity on Windows now includes os.getppid() (the launcher), and the supervisor_pid lineage check accepts a live recorded parent there, since the grandparent is not portably reachable. POSIX behaviour is unchanged.
Three test files added in 0.1.7 regressed the Windows test run: - test_resume.py and test_resume_routes.py monkeypatch os.killpg in their supervisor fixtures; the attribute does not exist on Windows, so every test in both files errored at setup. The patch now tolerates the absence and also stubs the Windows delivery helper the supervisor uses instead. - test_agent_registry.py wrote `#!/bin/sh` probe stubs, which CreateProcess cannot execute; they now go through tests/fake_cli with Python bodies, which builds the same sh launcher on POSIX. - test_reasoning_effort_chain.py read adapter.command_template, the shell string this branch replaced with argv lists; the assertions now inspect adapter.argv. The expected tokens are unchanged. No test semantics change; the full suite now passes on Windows (405 passed, 42 skipped - the documented symlink-privilege skips).
Dropping the shell took a side effect with it: `sh -c` rewrites `PWD` from `getcwd()` when it starts, so `cd <workspace> && ...` kept the variable and the real working directory in agreement for free. Executing argv directly passes `cwd=` to the OS and leaves the launcher's `PWD` inherited unchanged. That is not cosmetic, because not everything asks the OS. OpenCode resolves the directory its tools operate in from `PWD`, so an agent launched this way worked in whatever directory the operator happened to run `lh-harness` from -- writing files outside the workspace the run promised to contain them in, while the transcript kept naming plausible paths and the auditor confirmed them there. `PWD` is now derived from the `cwd` the child actually gets, in the local environment and in the supervisor's worker launch alike, and `OLDPWD` is dropped rather than blanked so `cd -` behaves like a fresh shell instead of failing. Verified end to end on Linux with all three installed backends launched from a directory outside the workspace: OpenCode now stays inside it, Claude Code and the DeepSeek Harness CLI are unaffected, as they read the working directory from the OS.
`test_deny_rules_cover_drive_letter_paths` guarded its Windows-only assertion by
looking for a colon in `Path("C:/runs/logs").resolve()`. On POSIX that string is
a *relative* path, so it resolves to `<cwd>/C:/runs/logs` -- which contains a
colon as well, and the guard was therefore true everywhere. The Windows branch
ran on Linux and failed there, the one red test in an otherwise green suite.
The dsh npm launcher is a .CMD shim, so its arguments travel through cmd.exe and its 8191-character command-line limit. The headless runner takes the task as a positional argument, and a role prompt is far larger than the limit, so every DeepSeek episode on Windows died in 0.3s with "The command line is too long" before reaching the provider. The headless runner resolves its `task` config from the command line, but a later --patch layer may override that row with a literal - the same override mechanism the runner already uses for the model. On Windows the prompt now rides in the existing per-episode patch file as a JSON-escaped (valid YAML) scalar, and the positional argument shrinks to a fixed placeholder that only satisfies the non-empty check. POSIX keeps passing the prompt as the positional argument, exactly as verified on Linux. Verified against dsh 0.1.0-rc.7 on Windows with a 19 KB prompt: the override is honoured and the episode completes.
The patch-layer task route was guarded by `os.name` in the runner and in its test alike, so a POSIX run of the suite never executed the Windows branch at all -- and that branch is a contract with dsh's patch precedence, not with the OS: if dsh ever stopped preferring the patch override, agents would silently receive the placeholder string as their task, and only a Windows machine could have noticed. The delivery is now a parameter (`task_via_patch`, defaulting to the platform rule), the placeholder is a named constant, and a parametrised test drives both routes on both platforms with a 9KB prompt full of YAML-hostile characters: the patch route must keep the prompt off the command line and carry it as one exactly-escaped scalar, the positional route must keep the patch file free of the `headless-runner` row. Behaviour at the defaults is unchanged and still covered by the existing test.
The PR's own history is the argument: the branch was verified green on each platform by hand, and each round of hand-verification still found something the other platform could not see (a POSIX-only test guard, a cmd.exe-only command-line limit). A matrix of ubuntu + windows at both ends of requires-python (3.10 and 3.14) makes that check automatic for every push and pull request. The suite needs no Node toolchain -- the Web bundle is a packaging artifact -- so the job is checkout, setup-python, `pip install -e ".[test]"`, pytest. The Windows symlink fixtures skip themselves on runners without SeCreateSymbolicLinkPrivilege, which is expected and green.
The first admin-privileged CI run caught three Windows bugs the local test machine could not see, because SeCreateSymbolicLinkPrivilege changed which tests actually ran and a busier pid space changed what a stray probe could hit. The severe one: two code paths asked "is this process alive" with `os.kill(pid, 0)`. That is the POSIX idiom -- and a kill switch on Windows, where os.kill can only deliver console control events and unconditionally calls TerminateProcess for every other signal value, zero included. The supervisor's `_is_alive` poll would kill the very worker it was checking on (or a pid-reuse victim), and the venv-launcher owner check in `--supervised` adoption would kill the supervisor whose reservation it was validating. On the CI runner, a status poll probing a test's fake pid terminated a live runner process and took pytest down with it (`KeyboardInterrupt`, suite dead after 27 tests). Both sites now use `process_alive`, a real observe-only probe: `tasklist` on Windows, `os.kill(pid, 0)` where it actually means "probe", with pid <= 0 rejected outright (POSIX group semantics, Windows idle process). Also from that run: * The Windows atomic-write branch refused a symlinked *destination*, where the POSIX branch's rename atomically evicts the link and installs a private regular file. Refusing is strictly weaker -- the crash-report writer swallows the OSError and the planted link survives for a later, less careful writer -- and os.replace already operates on the name, never the target. The refusal is gone; Windows now evicts exactly like POSIX. * The supervisor's idempotency lock on Windows surfaced the shared helper's "secure control-bus locking" message; it now reports "secure supervisor locking" like the POSIX branch, keeping the message a caller can match on. New cross-platform tests cover all three: the probe must see a live child and leave it running (the old Windows code kills it right there), must see a dead one, must reject nonsense pids; the atomic write must evict a symlinked destination without touching the file it pointed at.
Two corrections to the previous commit's own additions, both caught by the first Windows CI round that got past the liveness-probe kill: * ``_run_quiet`` reached ``tasklist``/``taskkill`` through ``subprocess.run``, which resolves ``Popen`` through the module namespace at call time -- and the supervisor tests stub ``subprocess.Popen`` with a fake worker that is no context manager, so every resume-path status refresh on Windows died with ``AttributeError: __enter__``. The probes now hold the real ``Popen`` captured at import, the same pattern service.py already documents for its worker spawn. * The both-routes dsh test drove the *positional* delivery with a 9KB multi-line prompt on every platform. On Windows that argument cannot fit through the .CMD shim -- the exact limitation the patch route exists to bypass -- so the test was asserting the platform bug away. The big hostile prompt now exercises the patch route only; the positional case proves just the route itself with a plain prompt.
…run" This reverts commit cbf36a8.
A harness agent runs under the operator's account, so `claude` loads
everything that account has accumulated: plugins, skills, hooks,
user-level CLAUDE.md. None of it was chosen for the run -- it spends
context tokens in every episode, a skill description can trigger on task
text it was never meant for, and a hook can rewrite tool calls the
harness believes it controls. The harness already isolates the DeepSeek
CLI behind its own DSH_HOME and scopes MCP with --strict-mcp-config;
Claude Code now gets the same property, opt-in and selective:
claude_isolation = true
claude_allowed_plugins = ["playwright"]
claude_allowed_skills = ["graphify"]
(config keys, with matching --claude-isolation /
--claude-allowed-plugin / --claude-allowed-skill flags; naming anything
in an allow-list implies isolation, because an allow-list only means
something against a clean slate.)
Isolation passes `--setting-sources project`, which drops exactly the
account-level layer while keeping what the workspace repo itself
declares -- deliberately not `--bare`, which also skips the account's
OAuth credentials and fails every keyless run with "Not logged in"
(verified against claude 2.1.239). Allowed plugins resolve through the
CLI's own installed_plugins.json rather than by guessing cache paths,
with bare names rejected as ambiguous when two marketplaces both carry
them. A skill has no standalone CLI switch, but a plugin may carry
skills, so the allowed skills are copied -- not symlinked, Windows
accounts usually cannot make links -- into one synthesised plugin under
the run's tmp tree, which also freezes the skill for the run's duration.
With nothing re-admitted, --disable-slash-commands closes the explicit
/skill-name escape hatch too.
Verified live: an isolated run completes with a clean audit; the skill
list visible to the agent shrinks to the CLI's built-ins plus exactly
the allowed entries (lh-harness-allowed-skills:graphify), and the
operator's other plugins and skills are gone from every role.
Isolation without an allow-list also passed --disable-slash-commands, which disables *every* skill -- including the ones that ship inside the CLI itself (init, code-review, security-review, ...). That made the built-ins' availability depend on whether the allow-list happened to be empty: allow one user skill and they all came back. Incoherent, and not what the feature promises: it removes the operator account's layer, not the CLI's own capabilities. Project-only setting sources already keep user skills from resolving, so the flag added nothing to account isolation -- it only subtracted the built-ins. Gone; built-in skills now behave identically whether or not anything was re-admitted, exactly like the CLI's built-in tools.
[""] is the natural but wrong way to write an empty allow-list -- it is an array holding one empty name, and the validator rejected the whole config with a message that never said what to write instead. The refusal now names the correct spellings (omit the key, or []), and the config template's examples use placeholder names instead of real ones from the author's machine, which read as if they were enabled.
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
1 participant
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.
The problem
A harness agent is a subprocess of the operator's account, so the
claudeCLI loads everything that account has accumulated — plugins, skills, hooks, user-level CLAUDE.md. None of it was chosen for the run: it spends context tokens in every episode of every role, a skill description can trigger on task text it was never meant for, and a hook can rewrite tool calls the harness believes it controls. The harness already isolates the DeepSeek CLI behind its ownDSH_HOMEand scopes MCP with--strict-mcp-config; Claude Code had no equivalent.The feature
Opt-in and selective, in
.lh-harness/config.toml(with matching CLI flags):Naming anything in an allow-list implies isolation — an allow-list only means something against a clean slate. Defaults unchanged: without opting in, agents behave exactly as before.
Mechanics (verified against claude 2.1.239)
--setting-sources project: drops the account-level layer, keeps what the workspace repo itself declares. Deliberately not--bare— bare mode also skips the account's OAuth credentials, so every keyless run dies with "Not logged in" (reproduced directly).installed_plugins.json— no cache-path guessing; a bare name carried by two marketplaces is rejected as ambiguous rather than guessed.--plugin-dir. The copy also freezes the skill for the run.--disable-slash-commandscloses the explicit/skill-nameescape hatch too.Testing
tests/test_claude_isolation.py): default unchanged, flag composition, manifest resolution incl. ambiguity, synthesised plugin contents, path-traversal rejection in names. Full suite on the branch: 465 passed, 2 skipped.--claude-allowed-skill graphifyexactlylh-harness-allowed-skills:graphifyis re-admitted.Result: complete, clean audit, argv in episode metadata carries--setting-sources projectand the synthesised plugin dir.