Skip to content

feat(miner-hands): CLI-subprocess CodingAgentDriver - #4531

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
jeffrey701:feat/miner-cli-subprocess-driver-v2
Jul 10, 2026
Merged

feat(miner-hands): CLI-subprocess CodingAgentDriver#4531
JSONbored merged 1 commit into
JSONbored:mainfrom
jeffrey701:feat/miner-cli-subprocess-driver-v2

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Closes #4266

What

Implements the CodingAgentDriver seam (#4262) by running the coding agent (claude/codex) as a subprocess in the attempt's scoped working directory. The spawn primitive is injected — a generalized version of src/selfhost/ai.ts's SpawnFn, redeclared here so gittensory-engine stays standalone (no import from src/) — so the driver is fully testable without a real child process.

Reuses the shared safety primitives (not duplicated)

Both come from subprocess-env.ts (#4284), the engine-hosted source of truth:

  • Strict allowlisted env via buildAllowlistedEnv + SUBPROCESS_CLI_ENV_ALLOWLIST — the child gets only home/proxy/TLS/locale/XDG keys plus caller-supplied extras, never the full host env (a coding-agent subprocess is prompt-injectable, so runtime credentials must not leak into it).
  • redactSecrets — any subprocess output kept as a transcript or folded into an error is redacted first (well-known token shapes + a caller-supplied knownSecrets list).

Behavior

  • Exit 0ok: true.
  • Non-zero/absent exit code → ok: false with a redacted <command>_exit_<code>: <detail> error (falls back to exit <code> when the subprocess wrote no stderr).
  • Timeout → ok: false with a distinct <command>_timeout_<ms>ms error.
  • Detecting which files changed is a sibling concern (a git diff over the worktree, feat(miner-hands): git-worktree-per-attempt isolation primitive #4269), so changedFiles is [].

API (packages/gittensory-engine/src/miner/cli-subprocess-driver.ts)

createCliSubprocessCodingAgentDriver(options)CodingAgentDriver, with CliSubprocessSpawnFn (the injectable spawn type) and CliSubprocessDriverOptions (command, spawn, timeoutMs, parentEnv/env, knownSecrets, and an overridable buildArgs).

Testing

npx vitest run test/unit/cli-subprocess-driver.test.ts
npm run typecheck

6/6 tests pass (success, non-zero-exit redaction, no-stderr fallback incl. a null code, timeout, env-allowlisting, secret-redaction + custom argv) via an injected fake spawn; typecheck clean. New engine file at 100% line + branch coverage (18/18, 18/18, 3/3).

Implements the CodingAgentDriver seam (JSONbored#4262) by running the coding agent
(claude/codex) as a subprocess in the attempt's scoped working directory. The
spawn primitive is INJECTED — a generalized version of src/selfhost/ai.ts's
SpawnFn, redeclared here so gittensory-engine stays standalone (no import from
src/) — so the driver is fully testable without a real child process.

Reuses (not duplicates) the two safety primitives already centralized in
subprocess-env.ts (JSONbored#4284): the child gets a STRICT allowlisted env via
buildAllowlistedEnv (never the full host env — a coding-agent subprocess is
prompt-injectable), and any subprocess output surfaced in a transcript or folded
into an error is run through redactSecrets first, plus a caller-supplied
knownSecrets list. A non-zero/absent exit code or a timeout yields ok:false with
a redacted error; exit 0 yields ok:true. Detecting which files changed is a
sibling concern (a git diff over the worktree, JSONbored#4269), so changedFiles is [].

- packages/gittensory-engine/src/miner/cli-subprocess-driver.ts:
  createCliSubprocessCodingAgentDriver + the injectable CliSubprocessSpawnFn type
  and CliSubprocessDriverOptions (command, spawn, timeout, env allowlist inputs,
  knownSecrets, and an overridable argv builder).
- packages/gittensory-engine/src/index.ts: barrel re-export.
- test/unit/cli-subprocess-driver.test.ts: success, non-zero exit (redacted),
  no-stderr fallback, timeout, env-allowlisting, and secret-redaction + custom
  argv — all via an injected fake spawn (100% branch coverage).

Closes JSONbored#4266
@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 9, 2026 23:00
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (b069dde) to head (f626388).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4531   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files         422      423    +1     
  Lines       37574    37592   +18     
  Branches    13724    13732    +8     
=======================================
+ Hits        35335    35353   +18     
  Misses       1583     1583           
  Partials      656      656           
Files with missing lines Coverage Δ
...ttensory-engine/src/miner/cli-subprocess-driver.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 9, 2026
@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-09 23:15:16 UTC

3 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Implements the CLI-subprocess CodingAgentDriver by spawning claude/codex via an injected spawn function, reusing subprocess-env.ts's allowlisted env and redactSecrets rather than duplicating them, and correctly distinguishes ok:true (exit 0), timeout, and non-zero/absent-exit-code failures with a redacted `<command>_exit_<code>`/`<command>_timeout_<ms>ms` error shape. Redaction runs before truncation (redactSecrets(...).slice(...)), so no secret can survive partially through the char-limit cut, and the change stays scoped (one new file, one barrel export, one test file, no src/ import). Test coverage is solid across the meaningful branches: exit 0, non-zero with stderr, non-zero with no stderr including a null code, timeout, env allowlisting vs full-env leak, and secret redaction with a custom buildArgs override.

Nits — 5 non-blocking
  • test/unit/cli-subprocess-driver.test.ts:51,60 embed realistic-looking `sk-ant-...`/`my-injected-longkey` strings purely to exercise the redaction regex — not real secrets, but worth a comment noting they're synthetic so future secret-scanners/reviewers don't flag them as leaks.
  • No test exercises the MAX_TRANSCRIPT_CHARS (8000) / MAX_ERROR_DETAIL_CHARS (500) truncation paths in cli-subprocess-driver.ts:52-53, even though truncation is user-visible behavior.
  • A throwing custom `buildArgs` (cli-subprocess-driver.ts:84) propagates as an unhandled rejection out of `run()` rather than an `ok:false` result, unlike every other failure mode in this driver — worth at least a doc note on the intended contract.
  • Add a test asserting transcript/error-detail truncation at the constants in cli-subprocess-driver.ts:52-53.
  • Document in `CliSubprocessDriverOptions.buildArgs`'s JSDoc that a throwing builder surfaces as a rejected promise, not a driver-shaped `ok:false` error, so callers wrap it if they want uniform failure handling.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4266
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 106 registered-repo PR(s), 45 merged, 4 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 106 PR(s), 4 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Partially addressed
The PR delivers the CodingAgentDriver with an injected SpawnFn, env allowlisting, redaction, and a solid fake-spawn test suite covering success/non-zero-exit/timeout/redacted-secret cases, matching most of the issue's scope. However it uses one flat default timeout (120000ms) instead of the explicitly-requested per-effort timeout ladder mirroring CLAUDE_EFFORT_TIMEOUT_MS/CODEX_EFFORT_TIMEOUT_MS an

Review context
  • Author: jeffrey701
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 106 PR(s), 4 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (issue #4307, issue #4266)
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored
JSONbored merged commit b7a4477 into JSONbored:main Jul 10, 2026
10 checks passed
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. and removed gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 10, 2026
@JSONbored JSONbored added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. and removed gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(miner-hands): CLI-subprocess CodingAgentDriver (reuse SpawnFn/redactSecrets/EFFORT_TIMEOUT_MS pattern)

2 participants