Skip to content

Delete the adapters' test-only paths - #506

Merged
SaladDay merged 4 commits into
aos/cutoverfrom
aos/adapter-cleanup
Oct 7, 2026
Merged

SaladDay merged 4 commits into
aos/cutoverfrom
aos/adapter-cleanup

Conversation

@SaladDay

@SaladDay SaladDay commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

This lane deletes the Codex and Claude SDK adapters' test-only paths (the adapter part of main's J audit) and one unset override.

Codex

  • Deletes the Session-without-Executor branches: the bare Cancel/cancelNativeWork path and the s.executor guards. Production builds every Turn with its Executor. Deletes their bare-Session tests.
  • Folds Prepared into Executor, which drops watchOwner, and inlines startNative into StartTurn.
  • Fixes a crash already on the base: a preparation rejected before start returned a nil *Executor inside a non-nil interface, and dispatch's cleanup panicked on it. PrepareExecutor now returns a nil interface, and a regression test covers it.
  • Deletes OAC_RUNTIME_CODEX_HARNESS_BIN, which nothing sets (audit 40-C6).

Claude

  • Deletes the test-only prepare and startRequest.Input. Tests now use the production prepareConfiguration path.
  • Renames the Test*Factory* tests after what they check.

Test fixes

  • drainEnvelopes now stops when the output channel closes, instead of looping forever.
  • The fake Codex server's cleanup waits for its goroutine. The test now passes 200 of 200 race runs.

Net change: 39 files, +193/−766.

Local checks:

  • gofmt is clean.
  • go vet passes for linux, darwin and windows.
  • The darwin and windows builds pass.
  • Focused tests pass for agent/codex, agent/claudesdk, agent and the dispatch Codex/Claude tests.
  • -race -count=50 passes on the changed concurrent tests, and -count=200 on the timing test the lane fixed.
  • make check-names check-docs check-ci pass.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Every Codex Turn is a Session built by Executor.StartTurn, so the
Session-without-Executor branches ran only in tests: Cancel's
cancelNativeWork path and the executor guards in emitTerminalFailure,
beginOperation/endOperation and onServerRequest. Delete them with the
executor marker field, the bare-Session cancellation tests that encoded
their semantics, and the Session part of the RPC close test.

Prepared only lived until newExecutor took it on the same call, so fold
it into Executor: newExecutor prepares the base Session and plan
directly, preparation failure releases them in place, and the owner
watcher that only ran before the immediate transfer goes with
TestPreparedCloseWaitsForOwnerCleanup. Inline startNative into
StartTurn and delete session_run.go. The subagent cancellation, blocked
steering and readiness cancellation tests now run on the Turn path.

Delete Claude's test-only prepare and startRequest.Input, which only
prepare set; executor_prepare never carries input. Its tests call
prepareConfiguration, the factory's option path, and the Test*Factory*
tests are named after the Turn behavior they check.
PrepareExecutor returned newExecutor's nil *Executor as a non-nil
agent.Executor when preparation was rejected before any resource
existed, such as a read-only or structured-output request. The daemon
then kept it as the native Executor and its cleanup called Close on a
nil receiver. Return a nil interface, and test read-only rejection
through the factory.
No deployment, image or script sets OAC_RUNTIME_CODEX_HARNESS_BIN, so
the local Environment declaration never read a value from it.
The Turn Cancel can send turn/interrupt before settlement closes the
client, and settlement does not wait for that reply. The observation
fixture's server could then persist history after the test removed its
temporary home. Cleanup now waits for the server goroutine.
@SaladDay
SaladDay merged commit 34cb1fa into aos/cutover Oct 7, 2026
1 check passed
@SaladDay
SaladDay deleted the aos/adapter-cleanup branch October 7, 2026 17:07
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