Repository navigation
refactor(runner): share execution context across shell launch paths - #1408
Conversation
swjnxyf
left a comment
There was a problem hiding this comment.
Reviewed the 21-file Runner shell execution-context refactor at a840cde13ffac3379651d5c8b577e27864b90279 against the prior dispatch, structured/interactive/detached process, script, Skill, build, format and internal-search paths. Policy checks, config generation and profile cache, cwd restrictions, timeout/cancellation propagation, preparation/start hooks, and internal environment overrides remain wired consistently; no blocking behavioral differences found. The PR's complete Linux/Windows/macOS CI run is green. I also checked out the exact PR head in an isolated worktree on mini and ran cargo test --locked --offline -p webcodex-runner --bin webcodex-runner shell -- --test-threads=4 (149 passed, 0 failed). LGTM.
Runner launch paths repeatedly forwarded configuration generation, policy, shell configuration, profile registry/cache, cwd, timeout, and cancellation through long positional argument lists. Carry these required borrowed fields in
ShellExecutionContextacross synchronous dispatch, structured Jobs, Skill resources, interactive/detached preparation, builds, formatting, and internal search.Process and script entry points now accept their start/preparation hooks directly, removing three production forwarding wrappers and one test-only profile wrapper. Private
ProcessOutputOptionsnames the environment, hooks, output mode, and deadline choices at the internal process boundary. Command preparation and execution bodies retain their existing behavior, including policy checks, process ownership, cancellation, bounded I/O, preparation fences, and start notification order. No wire or persisted representation changes are involved.The change spans 21 files, removes 17 argument-count lint suppressions, and reduces production code by 258 lines (153 lines net including expanded test coverage). Moving functions into impl blocks accounts for part of the larger diff; reviewing with whitespace ignored makes the substantive changes clearer.
Validation on macOS:
runner_skills::tests: 6 passed.runner-real-process-tests: profile timeout reclaims the descendant process group; Rust build observes selected cached artifacts; Python and Rust formatter profile preparation obey the total deadline; rustfmt stdin leaves sources untouched.cargo check --locked -p webcodex-runner --bin webcodex-runnerpassed without warnings. Test compilation with--features runner-real-process-tests --no-run, package formatting, andgit diff --checkpassed.Native Windows/Linux execution was not run locally.