Skip to content

Commit 3308df1

Browse files
committed
fix(providers/codex): synchronous abort check at stream entry, plus review-pass fixes
`streamCodexEvents` now checks `abortSignal?.aborted` before entering the `for await` so a caller abort that lands between attempt setup and the first event surfaces immediately instead of waiting on the next event or end-of-stream. The existing between-events check is retained. Also from the same review pass: - Pi shim: wrap `mkdirSync`/`writeFileSync` in try/catch so EACCES/ ENOSPC surfaces as a classified "Pi shim setup failed at <dir>" instead of a raw node:fs error. - Codex retry path: `getLog().debug` before throwing the model-access error from a retry-attempt `startThread`; the outer query_error log only runs for retryable errors. - Docs: ai-assistants.md and configuration.md updated for Claude `~/.local/bin/claude` autodetect; ai-assistants.md gains a Codex autodetect bullet listing the five probed paths. - Tests: `homedir()` instead of `process.env.HOME ?? '/Users/test'` to match the implementation; Windows autodetect probe covered; config-over-autodetect precedence covered.
1 parent 442a117 commit 3308df1

6 files changed

Lines changed: 76 additions & 14 deletions

File tree

packages/docs-web/src/content/docs/getting-started/ai-assistants.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ See [Anthropic's setup guide](https://code.claude.com/docs/en/setup) for the ful
4343

4444
### Binary path configuration (compiled binaries only)
4545

46-
Compiled Archon binaries cannot auto-discover Claude Code at runtime. Supply the path via either:
46+
In compiled Archon binaries, if `claude` is not on the default install path Archon autodetects, supply the path via either:
4747

4848
1. **Environment variable** (highest precedence):
4949
```ini
@@ -55,8 +55,9 @@ Compiled Archon binaries cannot auto-discover Claude Code at runtime. Supply the
5555
claude:
5656
claudeBinaryPath: /absolute/path/to/claude
5757
```
58+
3. **Autodetect** (zero-config fallback): Archon probes `~/.local/bin/claude` (POSIX) and `%USERPROFILE%\.local\bin\claude.exe` (Windows), matching the native curl/PowerShell installer layouts.
5859

59-
If neither is set in a compiled binary, Archon throws with install instructions on first Claude query.
60+
If none of the three resolves in a compiled binary, Archon throws with install instructions on first Claude query.
6061

6162
The Claude Agent SDK accepts either the native compiled binary or a JS `cli.js`.
6263

@@ -171,6 +172,7 @@ In compiled Archon binaries, if `codex` is not on the default PATH Archon expect
171172
codexBinaryPath: /absolute/path/to/codex
172173
```
173174
3. **Vendor directory** (zero-config fallback): drop the native binary at `~/.archon/vendor/codex/codex` (or `codex.exe` on Windows).
175+
4. **Autodetect** (zero-config fallback): if the vendor directory is empty, Archon probes the common npm-global install layouts: `~/.npm-global/bin/codex` (POSIX), `/opt/homebrew/bin/codex` (macOS Apple Silicon), `/usr/local/bin/codex` (macOS Intel and Linux), `%APPDATA%\npm\codex.cmd` and `%USERPROFILE%\.npm-global\codex.cmd` (Windows). For other npm prefixes or custom layouts, set `CODEX_BIN_PATH` or the config path explicitly.
174176

175177
Dev mode (`bun run`) does not require any of the above — the SDK resolves `codex` via `node_modules`.
176178

packages/docs-web/src/content/docs/getting-started/configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Set these in your shell or `.env` file:
1414

1515
| Variable | Required | Description |
1616
|----------|----------|-------------|
17-
| `CLAUDE_BIN_PATH` | Yes (binary builds) | Absolute path to the Claude Code SDK's `cli.js`. Required in compiled Archon binaries unless `assistants.claude.claudeBinaryPath` is set. Dev mode (`bun run`) auto-resolves via `node_modules`. |
17+
| `CLAUDE_BIN_PATH` | No (binary builds autodetect `~/.local/bin/claude`) | Absolute path to the Claude Code binary or SDK `cli.js`. Overrides autodetection in compiled Archon binaries. Falls back to `assistants.claude.claudeBinaryPath`, then to the native-installer path. Dev mode (`bun run`) auto-resolves via `node_modules`. |
1818
| `CLAUDE_USE_GLOBAL_AUTH` | No | Set to `true` to use credentials from `claude /login` (default when no other Claude token is set) |
1919
| `CLAUDE_CODE_OAUTH_TOKEN` | No | OAuth token from `claude setup-token` (alternative to global auth) |
2020
| `CLAUDE_API_KEY` | No | Anthropic API key for pay-per-use (alternative to global auth) |

packages/providers/src/codex/binary-resolver.test.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* Must run in its own bun test invocation because it mocks @archon/paths
55
* with BUNDLED_IS_BINARY=true, which conflicts with other test files.
66
*/
7+
import { homedir } from 'node:os';
8+
79
import { describe, test, expect, mock, beforeEach, afterAll, spyOn } from 'bun:test';
810
import { createMockLogger } from '../test/mocks/logger';
911

@@ -89,8 +91,7 @@ describe('resolveCodexBinaryPath (binary mode)', () => {
8991

9092
test('autodetects npm global install at ~/.npm-global/bin/codex (POSIX)', async () => {
9193
if (process.platform === 'win32') return; // POSIX-only probe
92-
const home = process.env.HOME ?? '/Users/test';
93-
const expected = `${home}/.npm-global/bin/codex`;
94+
const expected = `${homedir()}/.npm-global/bin/codex`;
9495
fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation(
9596
(path: string) => path === expected
9697
);
@@ -103,6 +104,33 @@ describe('resolveCodexBinaryPath (binary mode)', () => {
103104
);
104105
});
105106

107+
test('autodetects npm global install at %APPDATA%\\npm\\codex.cmd (Windows)', async () => {
108+
if (process.platform !== 'win32') return; // Windows-only probe
109+
const appData = process.env.APPDATA ?? 'C:\\Users\\test\\AppData\\Roaming';
110+
const expected = `${appData}\\npm\\codex.cmd`;
111+
fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation(
112+
(path: string) => path === expected
113+
);
114+
115+
const result = await resolver.resolveCodexBinaryPath();
116+
expect(result).toBe(expected);
117+
expect(mockLogger.info).toHaveBeenCalledWith(
118+
{ binaryPath: expected, source: 'autodetect' },
119+
'codex.binary_resolved'
120+
);
121+
});
122+
123+
test('config codexBinaryPath takes precedence over autodetect', async () => {
124+
// Both the explicit config path AND a typical autodetect path are
125+
// present on disk; config must win. Mirrors the env-over-config and
126+
// env-over-autodetect tests above so the four-tier precedence
127+
// (env → config → vendor → autodetect) is fully covered.
128+
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
129+
130+
const result = await resolver.resolveCodexBinaryPath('/explicit/config/codex');
131+
expect(result).toBe('/explicit/config/codex');
132+
});
133+
106134
test('autodetects homebrew install on Apple Silicon', async () => {
107135
if (process.platform !== 'darwin' || process.arch !== 'arm64') {
108136
// `/opt/homebrew/bin/codex` is only probed on darwin-arm64; on other

packages/providers/src/codex/provider.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ async function* streamCodexEvents(
197197
const state: CodexStreamState = {};
198198
let accumulatedText = '';
199199

200+
if (abortSignal?.aborted) {
201+
getLog().info('query_aborted_before_stream');
202+
throw new Error('Query aborted');
203+
}
204+
200205
for await (const event of events) {
201206
if (abortSignal?.aborted) {
202207
getLog().info('query_aborted_between_events');
@@ -593,6 +598,7 @@ export class CodexProvider implements IAgentProvider {
593598
} catch (startError) {
594599
const err = startError as Error;
595600
if (isModelAccessError(err.message)) {
601+
getLog().debug({ attempt, errorClass: 'model_access' }, 'query_error_pre_retry');
596602
throw new Error(buildModelAccessMessage(requestOptions?.model));
597603
}
598604
throw new Error(`Codex query failed: ${err.message}`);

packages/providers/src/community/pi/provider.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
import { readFileSync } from 'node:fs';
2+
import { join } from 'node:path';
3+
import { tmpdir } from 'node:os';
4+
15
import { beforeEach, describe, expect, mock, test } from 'bun:test';
26
import type { AgentSessionEvent } from '@mariozechner/pi-coding-agent';
37

@@ -222,6 +226,21 @@ describe('PiProvider', () => {
222226
await consume(new PiProvider().sendQuery('hi', '/tmp'));
223227
expect(process.env.PI_PACKAGE_DIR).toBeDefined();
224228
expect(process.env.PI_PACKAGE_DIR).toContain('archon-pi-shim');
229+
230+
// Stub contents are load-bearing: Pi reads `version` to populate its
231+
// user-agent and `piConfig` (even when empty) to opt into the defaults
232+
// path instead of erroring on missing config. Asserting on shape so a
233+
// regression here surfaces in the test suite, not in a Pi runtime crash.
234+
const shimDir = process.env.PI_PACKAGE_DIR;
235+
expect(shimDir).toBe(join(tmpdir(), 'archon-pi-shim'));
236+
const stub = JSON.parse(readFileSync(join(shimDir!, 'package.json'), 'utf8')) as {
237+
name: string;
238+
version: string;
239+
piConfig: Record<string, unknown>;
240+
};
241+
expect(stub.name).toBe('archon-pi-shim');
242+
expect(stub.version).toBe('0.0.0');
243+
expect(stub.piConfig).toEqual({});
225244
});
226245

227246
test('throws when no model is configured', async () => {

packages/providers/src/community/pi/provider.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,24 @@ function ensurePiPackageDirShim(): void {
5252
const shimDir = join(tmpdir(), 'archon-pi-shim');
5353
const shimPkgJson = join(shimDir, 'package.json');
5454
if (!existsSync(shimPkgJson)) {
55-
mkdirSync(shimDir, { recursive: true });
5655
// `piConfig: {}` is explicit so Pi's defaults (`name: 'pi'`,
5756
// `configDir: '.pi'`) kick in — matches Pi's standalone behavior.
58-
writeFileSync(
59-
shimPkgJson,
60-
JSON.stringify({
61-
name: 'archon-pi-shim',
62-
version: '0.0.0',
63-
piConfig: {},
64-
})
65-
);
57+
try {
58+
mkdirSync(shimDir, { recursive: true });
59+
writeFileSync(
60+
shimPkgJson,
61+
JSON.stringify({
62+
name: 'archon-pi-shim',
63+
version: '0.0.0',
64+
piConfig: {},
65+
})
66+
);
67+
} catch (error) {
68+
// Surface as a classified error so the executor's catch sees a known
69+
// shape instead of a raw EACCES/ENOSPC from node:fs.
70+
const err = error as NodeJS.ErrnoException;
71+
throw new Error(`Pi shim setup failed at ${shimDir}: ${err.message}`);
72+
}
6673
}
6774
process.env.PI_PACKAGE_DIR = shimDir;
6875
}

0 commit comments

Comments
 (0)