Skip to content

fix: resolve a worker's engine binary from its own provider, not the global default - #20

Open
aaroncoville wants to merge 2 commits into
mainfrom
upstream/worker-provider-command-resolution
Open

aaroncoville wants to merge 2 commits into
mainfrom
upstream/worker-provider-command-resolution

Conversation

@aaroncoville

Copy link
Copy Markdown
Owner

What & why

A spawn request may name a provider without naming a command. In that case the launch
builder took the binary from the app-wide defaultCommand — typically claude — but took the
auto-mode flag from the requested provider. The two halves came from different CLIs, so a
codex-provider worker was launched as claude --dangerously-bypass-approvals-and-sandbox, which
claude rejects as an unknown option; the worker exited 1 within a second of spawning, before it
could do any work.

Root cause: the command and the flags were resolved from two different sources. The auto flag has
always been the provider's, but the command rung skipped the provider entirely and fell through to
the global default — so naming a provider changed the flags without changing the binary.

The fix resolves the command in the precedence the request already implies: an explicit command
first, then the named provider's preset binary, then the global default. custom has no binary of
its own and keeps falling back to the configured default; requests that name no provider are
unaffected.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

This is a CLI-argv bug with no UI, so the evidence is the resolved argv and the test that pins it,
captured as text.

Before

Spawn request: { provider: 'codex' } — no command, app default claude, auto mode on.

Resolved argv:

claude --dangerously-bypass-approvals-and-sandbox
error: unknown option '--dangerously-bypass-approvals-and-sandbox'

The worker exits 1 immediately.

The two new tests run against the unmodified source on main fail:

$ node --test test/worker-launch.test.cjs
not ok 15 - a provider-only request resolves that provider's binary, not the global default
  Expected values to be strictly equal:
  'claude' !== 'codex'
not ok 16 - a provider-only request with a model uses that provider's model flag
# tests 19
# pass 17
# fail 2

After

The same spawn request, with this change applied.

Resolved argv:

codex --dangerously-bypass-approvals-and-sandbox

The worker boots and runs. The same tests pass:

$ node --test test/worker-launch.test.cjs
# tests 19
# pass 19
# fail 0

How I tested it

  • OS: macOS 15 (Darwin 25.6.0), Node via the repo toolchain.
  • Steps:
    1. Measured the baseline on the branch point first: npm run test:focused on main →
      552 of 552 passing, npm run typecheck → 0 errors.
    2. On this branch: npm run typecheck → 0 errors.
    3. npm run test:focused → 557 of 557 passing, 0 failing (the 5 new tests are the delta;
      no pre-existing failures on either ref).
    4. npm run build → succeeds.
    5. Confirmed the tests are load-bearing by reverting only src/main/workerLaunch.ts and
      src/main/index.ts to main while keeping the new tests: 2 of them go red with
      'claude' !== 'codex' (transcript under Before). Restored, green again.

Discord (optional)

Discord:

Checklist

  • Before and after evidence is attached above, under both headings.
  • npm run typecheck passes.
  • npm run test:focused passes.
  • npm run build succeeds.
  • This PR is one change. Unrelated fixes belong in their own PR.
  • I read the diff myself before opening this, and there is no debug output,
    commented-out code, or unrelated formatting churn in it.
  • Any new UI derives from DESIGN.md / tokens.ts — no ad-hoc colors,
    spacing, or fonts. (No UI in this change.)
  • If I added art, it's my own or compatibly licensed, and listed in
    ATTRIBUTION.md. (No art in this change.)

github-actions Bot and others added 2 commits August 26, 2026 19:03
Co-authored-by: wall-sync <wall-sync@users.noreply.github.com>
…global default

A spawn request may name a `provider` without naming a `command`. In that case
buildWorkerLaunch took the command from the app-wide `defaultCommand` — typically
`claude` — and then appended the auto-mode flag belonging to the REQUESTED
provider. The two halves came from different CLIs, so the result was a command
line no binary accepts:

    claude --dangerously-bypass-approvals-and-sandbox [--model gpt-5.6-sol]
    error: unknown option '--dangerously-bypass-approvals-and-sandbox'

The worker exited 1 within a second of spawning, before it could do anything.

Root cause: the command and the flags were resolved from two different sources.
The auto flag has always been the provider's (`autoModeFlagForProvider`), but the
command rung skipped the provider entirely and fell through to the global
default, so naming a provider changed the flags without changing the binary.

Resolve the command in the same precedence the rest of the request implies:
an explicit `command` first, then the named provider's preset binary, then the
global default. `custom` has no binary of its own, so it keeps falling back to
the configured default. Requests that name no provider are unaffected.
@aaroncoville

Copy link
Copy Markdown
Owner Author

Note for review — one file in this diff is not part of the change.

docs/wall-data.json appears here only because this fork's main is one upstream commit behind: it is missing b9f34e71 ("wall: 5 new Founding Supporter(s) (HarnessMD#319)"), which touches that file. This branch was cut fresh from origin/main (upstream tip), so GitHub shows that upstream commit as part of the diff against the fork's base.

Against the real target it is clean:

$ git log origin/main..HEAD --oneline
b6ca0a23 fix: resolve a worker's engine binary from its own provider, not the global default

$ git diff origin/main...HEAD --stat
 src/main/index.ts           |  2 +-
 src/main/workerLaunch.ts    | 22 +++++++++++++++++--
 test/worker-launch.test.cjs | 53 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)

Exactly one commit, only the intended files, no fork-only files. The PR body above is written to the upstream template and can be reused verbatim when the upstream PR is opened — the only thing it still needs is the terminal captures under ### Before / ### After; the repro steps and argv are already there as text.

@github-actions

Copy link
Copy Markdown

🚫 This PR is missing its before/after evidence

Every pull request here has to show its work. Screenshots or a short screen recording, before the change and after it.

  • Before — no image or video under that heading
  • After — no image or video under that heading

How to fix it: edit the description, keep the ### Before and ### After headings from the template, and drag an image or video under each. GitHub uploads it inline. This check re-runs the moment you save.

A bug fix with no visible surface still needs it: show the failing behaviour, then the same steps passing. A terminal recording is fine.

Genuinely nothing to show — a CI tweak, a typo, a dependency bump? A maintainer can apply the no-visual-change label. Please don't ask unless it truly has no observable effect.

📖 CONTRIBUTING.md → Evidence is mandatory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant