Add Codex plan/spark provider support - #11
Conversation
|
Implementation note: this Codex integration was inspired by fast-agent, especially its Codex auth/model handling and Responses transport design. References: |
|
Hey @iqdoctor @kevincodex1, first of all great work on this PR the auth.json fallback works perfectly on Windows too. Quick question since you're the ones who implemented the Codex auth flow: I'm using openclaude with my ChatGPT Business plan Codex subscription via
Thanks! |
|
The algorithm was ported from |
|
Thanks. This reply was prepared by the Codex session that shipped the Codex patches in this PR. For the
So the short version is: for the |
|
Thanks a lot for the clarification really appreciate it. It’s especially helpful to know the Codex flow was ported from fast-agent and that you’ve already been using subscription-based Codex keys there successfully for a long time. That gives me a lot more confidence in the setup. Also, thanks again for the great work on this PR. |
…spark Add Codex plan/spark provider support
…spark Add Codex plan/spark provider support
…spark Add Codex plan/spark provider support
The summary line included Low: L but Phase 3 drops non-measurable findings and exclusions remove micro-optimizations. Low findings (measurable but not user-visible) would never survive the filter, so remove Low from the severity categories and summary line. fix(bughunter-security): align log-forging exclusion with A9 criteria Exclusion Twigpine#11 blocked all log spoofing/forging, but A9 says to flag log injection when it enables audit-trail forgery. Narrowed the exclusion to allow concrete audit-trail attacks through while still excluding generic non-exploitable logging suggestions.
Reword exclusion Twigpine#11 to require concrete evidence of a log-entry or structured-field forgery path, not merely unsanitized user input.
…bughunter-perf with robust fallback prompts (#1621) * feat(bughunter): split into /bughunter, /bughunter-security, /bughunter-perf Replace the single /bughunter command with three siblings that share a common prefix: /bughunter — general bug hunt (existing prompt, untouched) /bughunter-security — OWASP-aligned, exploit-driven, confidence ≥ 8 /bughunter-perf — hot-path complexity, sync I/O, leaks, N+1 Both new subcommands are prompt commands built with createMovedToPluginCommand so they migrate to the bughunter marketplace plugin unchanged once it ships. While the marketplace is private they inline the full audit prompt (frontmatter + !`git ...` blocks) just like the existing /bughunter. All three stay in the public COMMANDS list (not INTERNAL_ONLY_COMMANDS) so non-ant users can invoke them. clearCommandMemoizationCaches() now also flushes the zero-arg COMMANDS() and builtInCommandNames() memos so tests can switch USER_TYPE mid-run without poisoning the cache. Adds regression tests in src/commands.test.ts covering: - bughunter stays public for non-ant users - bughunter-security and bughunter-perf are in the public list Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore(bughunter): remove orphan index.js after .js → .ts rename The bughunter command directory was renamed from a single .js file to index.ts in the previous commit, but git tracked them as separate paths so the old .js was left in the tree. Drop it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(bughunter): enhance fallback prompts for robustness in non-git environments - Add graceful error handling to all git commands in fallback prompts (|| echo fallbacks) - Add explicit non-git fallback guidance in Phase 1 for all three commands - /bughunter: search for entry points, core business logic, recently modified files - /bughunter-security: search for auth/middleware, validation, DB, config, upload code - /bughunter-perf: search for handlers, loops, data access, serialization, build configs - Improve context labels to clarify git context may be empty * fix(bughunter): address CodeRabbit feedback - Fix test isolation: restore USER_TYPE/IS_DEMO env vars in finally blocks - Add non-git fallback test cases for all three bughunter commands - Fix bash pipeline issue: replace if/then/else subshells with simple git commands + static fallback text in template - Fix output format contradiction: remove LOW confidence from scoring (Phase 3 drops LOW, so scoring only includes Critical/Medium) * fix(test): correct case and prefix in git fallback assertions for bughunter-security and bughunter-perf tests * fix(test): add missing opening parenthesis in bughunter test assertions * fix(bughunter): complete non-git fallback and propagate allowedTools - Fix git commands in all three prompts to always succeed with fallback text (using || echo) - Modify createMovedToPluginCommand to accept allowedTools parameter - Add allowedTools to all three bughunter commands so slash-command turn grants declared tools - Parse allowed-tools from frontmatter at command creation time * fix(bughunter): complete non-git fallback and allowedTools propagation - Fix git commands in prompts to always succeed with fallback text (using || echo) - Modify createMovedToPluginCommand to accept allowedTools parameter - Add allowedTools to all three bughunter commands so slash-command turn grants declared tools - Fix RECENTLY COMMITTED FILES command to avoid command substitution (permission check rejects ) - Update tests to accept shell tool's '(Bash completed with no output)' for empty results - Use runWithCwdOverride and additionalWorkingDirectories for proper test isolation * fix(bughunter): prevent shell injection via user-provided args The user-provided scope was interpolated into the prompt template BEFORE executeShellCommandsInPrompt() ran, so any !command or ```! block syntax in the args would be interpreted and executed as shell commands. Fix: parse frontmatter from the raw template and run shell execution first (with {{ARGS}} still in place — inert to shell patterns), then replace {{ARGS}} with the user scope on the processed output. This ensures args are never fed through the shell command parser. * refactor(bughunter): use createGetAppStateWithAllowedTools helper Replaces duplicate inline getAppState overrides across all three bughunter commands (bughunter, bughunter-security, bughunter-perf) with the shared helper from src/utils/forkedAgent.ts. This: - Eliminates ~30 lines of duplicated permission context modification - Merges allowedTools with existing alwaysAllowRules.command (vs overwrite) * fix(bughunter): address jatmn review - String.replace special patterns + test isolation - Replace '{{ARGS}}' with a replacer function () => scope instead of the plain string 'scope'. JavaScript's String.replace treats $&, $', $', , 32855 specially even in string replacements, so a scope like 'src/auth $&' would render as 'src/auth {{ARGS}}' instead of literal text. The replacer function bypasses all special patterns. - Restore USER_TYPE and IS_DEMO env vars in the injection regression test's finally block, matching the isolation pattern used by all other bughunter tests. * fix(bughunter): make fallback prompt generation work on Windows Wrap executeShellCommandsInPrompt() in a try/catch in all three bughunter commands. On platforms where bash is unavailable (e.g. Windows without Git Bash), the bash-specific shell syntax (2>/dev/null, | head -N) would cause executeShellCommandsInPrompt to throw MalformedCommandError, preventing the prompt from being generated at all. The catch handler replaces the !`command` inline patterns with a static placeholder, allowing the LLM to still receive the full audit instructions and non-git search strategies in Phase 1. * fix(bughunter-perf): remove Low severity contradiction The summary line included Low: L but Phase 3 drops non-measurable findings and exclusions remove micro-optimizations. Low findings (measurable but not user-visible) would never survive the filter, so remove Low from the severity categories and summary line. fix(bughunter-security): align log-forging exclusion with A9 criteria Exclusion #11 blocked all log spoofing/forging, but A9 says to flag log injection when it enables audit-trail forgery. Narrowed the exclusion to allow concrete audit-trail attacks through while still excluding generic non-exploitable logging suggestions. * fix(bughunter-security): tighten log-forging exclusion threshold Reword exclusion #11 to require concrete evidence of a log-entry or structured-field forgery path, not merely unsanitized user input. * fix(bughunter): preserve fallback text on Windows/no-bash path Replace generic '(Shell execution unavailable)' placeholder with a regex that extracts the || echo "..." fallback text from each shell command. This ensures the prompt shows meaningful messages like '(If empty: not a git repository or git unavailable)' even when bash is unavailable (e.g. Windows without Git Bash), matching what Linux users see from working shell execution. Also make injection test assertion platform-agnostic — accept either bash output or the static echo fallback text. * refactor(test): extract duplicate mockContext into createMockToolContext helper The three non-git fallback tests each had an identical ~42-line mockContext object. Moved it to a shared createMockToolContext(cwd, commands) helper and a FULL_GIT_COMMANDS constant. Also updated the injection test to use the same helper. Net -89 lines. * fix(createMovedToPluginCommand): only grant allowedTools when fallback prompt runs The ant (USER_TYPE === 'ant') branch returns a plugin-install notice that doesn't need Read/Glob/Grep/Bash tools, but allowedTools was statically attached to the command object. This caused processSlashCommand to grant turn-scoped permissions for tools that were never used. Changed to a getter that returns undefined in the ant branch, so the plugin-install notice runs without unnecessary tool permissions. * fix(bughunter): simplify shell commands to single git commands, narrow catch to surface interruptions * fix(bughunter): surface permission-denied/aborted shell preprocessing, fix Windows cleanup * fix(dragDropPaths.test): resolve package.json relative to test file, not process.cwd() * fix(commands.test): restore original cwd in rmRetry, guarantee env/cache cleanup on rm failure * fix(bughunter): bound diff to 400 lines, swap HEAD~10 for git log -10 Address both P2 reviewer findings on feat/bughunter-command-v3-new. (1) Fresh-repo HEAD~10 lookup stripped every snippet. In a one-commit repo, `git diff --name-only HEAD~10..HEAD --diff-filter=AM` exits 128 (HEAD~10 doesn't resolve). The shell-execution catch then ran the outer "strip all snippets" fallback, leaving git status / diff --cached / diff HEAD empty even though those commands would have produced useful context. Switched to `git log -10 --name-only --diff-filter=AM`, which works at any history depth and yields the same file list. Applied to bughunter, bughunter-security, and bughunter-perf. (2) Diff cap removed in 73d0bcb. The prompt label still advertised "first 400 lines" but the snippet was just `git diff HEAD -- .`, and a 900-line diff was injected verbatim. Added a new `lineLimits` option to `executeShellCommandsInPrompt` that bounds output by command prefix. The cap is applied to stdout *before* processToolResultBlock, so the persistence + empty-content guard flows run once on the bounded payload, and large diffs no longer hit the 30k Bash result cap and spill into the prompt. Each bughunter command passes `{ lineLimits: { 'git diff HEAD -- .': 400 } }`. Allowed-tools frontmatter is unchanged — no compound `| head -400` that the permission parser might reject. Tests: - `executeShellCommandsInPrompt applies per-prefix line limits` + `does not truncate below the cap` (new unit tests in promptShellExecution.test.ts). - `bughunter keeps git context populated in a fresh single-commit repo` (regression for finding 1, uses real one-commit git repo). - `bughunter diff block is bounded to 400 lines` (regression for finding 2, builds 1000-line diff and asserts ≤400 lines). - `FULL_GIT_COMMANDS` and the injection test now include `git log -10 --name-only --diff-filter=AM` in place of the removed HEAD~10 form. * fix(bughunter): keep recent-files path-only, cover all three siblings Two review follow-ups on the previous P2 commit. (P3) `git log -10 --name-only` defaulted to --pretty=fuller, so the "RECENTLY COMMITTED FILES" block injected commit hash, author, date, and message lines into the prompt under a files-only heading — that extra metadata crowded out the scoped file list the command was trying to provide. Added `--pretty=format:` to suppress the commit header on all three commands (bughunter, bughunter-security, bughunter-perf). Verified locally: the previous form emitted ~7 header lines per commit; the new form emits just the file paths. (P2) The fresh-repo and 400-line cap regression tests only exercised /bughunter, so a sibling could regress back to the old shallow-history failure or lose the diff cap without this suite failing. Parameterized both tests over {bughunter, bughunter-security, bughunter-perf} via a BUGHUNTER_SIBLINGS const; each command now runs both regressions in its own tmp dir (six new test cases total). Typecheck clean, 27 tests pass. * fix(promptShellExecution): granular snippet fallback, restore rich error for other callers - Add granularFallback option to executeShellCommandsInPrompt. When enabled, a failing shell snippet is blanked in place and the rest of the snippets keep their output. Permission denials and interrupted ShellError still rethrow as MalformedCommandError, never swallowed. - Restore the formatted MalformedCommandError wrapping in the default path. Previously a no-op that rethrew the raw ShellError, which made processSlashCommand render only 'ShellError: Shell command failed' for /commit, /security-review, /commit-push-pr, loaded skills, and plugin commands. Now includes the failing pattern and formatted stdout/stderr. - /bughunter, /bughunter-security, /bughunter-perf opt into granularFallback and drop the catch-and-strip-all pattern. A failing 'git log -10' on a zero-commit repo no longer discards git status output. - Tests cover per-snippet blanking, default-path rich error wrapping, and that permission denials still surface under granularFallback. Co-Authored-By: Claude <noreply@anthropic.com> * fix(promptShellExecution): preserve trailing newline in applyLineLimit truncation --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
codexplanandcodexsparkmodel aliases/responsesbackend instead of/chat/completionsCODEX_API_KEYor the Codex CLIauth.jsonfallback (CODEX_AUTH_JSON_PATH/CODEX_HOME/~/.codex/auth.json)codexprofileImplementation notes
codexplanmaps togpt-5.4withreasoning.effort=highcodexsparkmaps togpt-5.3-codex-spark/responsescalls withstore:false, then the shim translates Codex SSE events back into Anthropic-style events for the rest of OpenClaudetool_use/ usertool_resulthistory into Responsesfunction_call/function_call_outputitemsValidation
bun run test:providerbun run buildbun run smokeenv CLAUDE_CODE_USE_OPENAI=1 OPENAI_MODEL=codexplan bun run doctor:runtime./node_modules/.bin/tsc --noEmit --target ES2022 --module ESNext --moduleResolution bundler --strict --esModuleInterop --skipLibCheck --resolveJsonModule --baseUrl . src/services/api/providerConfig.ts src/services/api/codexShim.ts src/services/api/openaiShim.ts src/services/api/codexShim.test.tsCaveat
bun run typecheckis already failing in this repo for many unrelated upstream issues (missing declarations, macro globals, missing generated modules, ES2023 array helpers, etc.). This PR does not make that baseline green; the targeted Codex files typecheck cleanly.