Skip to content

fix(run-claude-cli): reap every leg-1 process before the next leg starts - #801

Closed
kyle-sexton wants to merge 1 commit into
mainfrom
fix/799-leg-isolation
Closed

fix(run-claude-cli): reap every leg-1 process before the next leg starts#801
kyle-sexton wants to merge 1 commit into
mainfrom
fix/799-leg-isolation

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 20, 2026

Copy link
Copy Markdown
Member

Closes #799

Summary

Leg isolation hardening for run-claude-cli. A process a leg starts and detaches (&, nohup, setsid, a double fork) could keep running after the leg ended, including while a later leg ran. Each leg now ends only after every process it left is killed and confirmed gone. If any process survives, the next leg does not start.

Fix

After each leg, runLegs calls reapLeg, which repeats these steps:

  1. legProcesses lists every live process whose real uid is the runner user's and whose /proc/<pid>/stat start time is no earlier than the launcher's own. The launcher and its ancestors are left out.
  2. reapLeg sends SIGSTOP and then SIGKILL to each listed process. A stopped process cannot fork again before it is killed.
  3. It lists again, up to 50 rounds.

The next leg starts only when a listing comes back empty. If processes remain, runLegs throws leg N left K process(es) running after it ended; no further leg runs, and the step fails closed. The same check also runs after the last leg, so nothing outlives the step's legs.

Why this mechanism rather than the others considered:

  • It covers setsid and double forks. The listing does not depend on process group, session, or parent. setsid changes the session and process group, and nohup, & and a double fork change the parent, but none of them changes the real uid or the start time. Changing the real uid takes privilege, which the runner user does not have after runner isolation removes sudo. Zombies (Z/X) count as dead.
  • A process-group kill alone misses setsid.
  • bwrap --unshare-pid would put the whole leg in a PID namespace, but it breaks the CLI's nested bwrap sandbox (open /proc/3/ns/ns failed, see run-claude-cli: a process a leg detaches outlives the leg's timeout #799).
  • A per-leg cgroup needs a delegated cgroup subtree. Whether the runner user has one on hosted ubuntu-24.04 without sudo is unverified.
  • PR_SET_CHILD_SUBREAPER has no Node binding, so it would need a native helper.

The /proc sweep needs neither new privilege nor a new binary.

runLegs and run take optional listLegProcesses and kill parameters. The tests use them to narrow the sweep to their own processes, so a local test run never signals a process the developer started.

Verification

  • node --test .github/actions/run-claude-cli/*.test.cjs: 42 pass, 0 fail. The 3 new tests:
    • legProcesses lists the processes left by &, nohup, setsid -f and a double fork, matched by the pid each one wrote for itself, and never lists the test process or its parent. This runs the unscoped production listing and only reads it.
    • A key leg that leaves all four running and ends in a credit stop: every one of them is dead when the subscription leg is spawned, and that leg runs.
    • The same leg with a kill that does nothing: the run throws before leg 2, and only the key leg ran (fail closed).
  • Mutation check: with reapLeg short-circuited to report no survivors, the two leg tests fail.
  • node --test .github/actions/report-lane-outcome/*.test.cjs: 45 pass.
  • Still needed before ready: a hosted-runner probe in pr-pipeline-sandbox (the run-claude-cli-ro-binds-probe shape). It should check that the sweep finds no unrelated runner-user process started during the step, and that a detached child is gone before leg 2.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q6B1MWhz73WFRYyyrKjtjY

@kyle-sexton @claude
fix(run-claude-cli): reap every leg-1 process before the next leg starts
f01e627 
After each leg, list every process whose real uid is the runner user's and
that started after the launcher (excluding the launcher and its ancestors),
stop and kill them, and start the next leg only once a listing comes back
empty. A survivor fails the run. The listing does not depend on process
group, session or parent, so it covers &, nohup, setsid and double forks.

Closes #799

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q6B1MWhz73WFRYyyrKjtjY
This was referenced Oct 20, 2026
Closed
Merged
@kyle-sexton

kyle-sexton commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

Superseded by #803, which runs the subscription leg in its own job instead of reaping processes within one job (decision: #799 (comment)).

kyle-sexton added a commit that referenced this pull request Oct 20, 2026
@kyle-sexton @claude
fix(run-claude-cli)!: run the subscription leg as its own job (#803)
7b36242 
Closes #799

## Summary

Runs the API-key leg and the subscription leg as separate jobs instead
of two legs of one step, per the design decision on #799.
`run-claude-cli` now runs one leg per invocation, and `intake-triage`
splits into a key job and a subscription job.

## Fix

- `run-claude-cli`: exactly one of `api-key` and `subscription-token`
must be set (fails closed on none or both); the in-step fallback loop is
removed. A key leg that ends in a credit stop sets `lane-class` to
`credit-stop`, still written after the launcher cuts back the step
files.
- `verifyManagedSettings` also requires `allowManagedHooksOnly: true` or
`disableAllHooks: true`; `apply-claude-managed-settings` (whose default
already sets `allowManagedHooksOnly`) refuses settings that set neither.
- `intake-triage.yml`:
  - `claude-intake-triage` (caller's runner) runs the key leg only.
- `claude-intake-triage-subscription` runs only when that job's
`lane-class` is exactly `credit-stop` (or its key was empty), on a fixed
`ubuntu-24.04` runner, with a fresh checkout and the same
managed-settings, sudo/docker removal and bubblewrap steps. It takes no
artifact or cache from the first job, and it is the only job that
references the OAuth secret.
- The job that triaged applies and reports.
`claude-intake-triage-status` reads both jobs and reports one outcome.
  - Caller inputs, secrets and workflow outputs are unchanged.
- README: documents single-leg use and the two-job pattern for callers.

- Review follow-ups:
- The step's file-command files are bound read-only in the leg's bwrap
namespace, including for any process the leg leaves behind. After the
leg they are restored to their exact content, and only then does the
launcher write its outputs.
- The CLI version defaults to 2.1.304, the version `npm view` returned
on 2026-10-20, instead of `latest`.
- The intake header notes that rerunning only the subscription job does
not rerun the author gate. This is accepted because a rerun needs write
access.

**Breaking for direct `run-claude-cli` callers:** passing both
credentials now fails, and a key credit stop reports `credit-stop`
instead of `rate-limit`. Versioning: this repository keeps no changelog
file and cuts SemVer tags with `release-tag.yml`; the `!` title marks
the break, and the release after merge should take the breaking bump (on
`v0.x`, a minor bump per the README). The pr-verify migration in
claude-code-plugins follows that release and pins to it.

## Verification

- `node --test .github/scripts/*.test.cjs`: 471 pass, 0 fail
- `node --test .github/actions/report-lane-outcome/*.test.cjs
.github/actions/run-claude-cli/*.test.cjs`: 88 pass, 0 fail, 0 skipped,
including a background process that tries to append
`lane-class=credit-stop` after the leg ends (it gets EROFS and the
output keeps the launcher's value) and a same-size rewrite that is put
back byte for byte
- New tests cover:
- exactly-one-credential enforcement (none, both, `run` refuses both
before any leg starts)
  - no second leg after a credit stop, and the `credit-stop` lane-class
- the managed-settings hook requirement, and the action default passing
the launcher's check
- workflow structure: only the subscription job references the OAuth
secret, its fixed `runs-on`, its exact gate, no artifact or cache, steps
equal to the key job's but for the credential, and the status job's
outcome selection
- Deliberately breaking the fixed `runs-on`, the status class mapping
and the exactly-one rule each made the new tests fail.
- `actionlint`, `zizmor --config .github/zizmor.yml .` (no findings),
`biome ci`, `markdownlint-cli2 README.md`, `typos`,
`editorconfig-checker` and composite-run shellcheck: all clean.

## Related

- Follow-up (out of scope here): #804, give the model a read-only token
and move label writes to a model-free job.
- Supersedes #801 (same-job process reap); that draft can be closed.
- Design: the last comment on #799.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01Q6B1MWhz73WFRYyyrKjtjY

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

run-claude-cli: a process a leg detaches outlives the leg's timeout

1 participant

@kyle-sexton