[WRONG BRANCH] fix(codebuddy): stage system prompts in a private file instead of process argv - #479
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a9cd6b7db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (effort) args.push("--effort", effort); | ||
| const system = buildSystemPrompt(parsed); | ||
| if (system) args.push("--append-system-prompt", system); | ||
| if (systemPromptFile) args.push("--system-prompt-file", systemPromptFile); |
There was a problem hiding this comment.
For every turn containing a system or developer prompt, this changes the previous append behavior into replacement behavior: the Claude-compatible CLI distinguishes --system-prompt-file, which replaces its default system prompt, from --append-system-prompt-file, which appends file contents (CLI flag reference). Replacing the vendor prompt can remove the CLI's baseline behavioral and protocol instructions, so pass the staged file through the append-file variant instead.
Useful? React with 👍 / 👎.
| if (system) { | ||
| try { | ||
| promptDir = await mkdtemp(join(tmpdir(), "ocx-codebuddy-prompt-")); | ||
| promptFile = join(promptDir, "system-prompt.txt"); | ||
| await writeFile(promptFile, system, { encoding: "utf8", mode: 0o600, flag: "wx" }); |
There was a problem hiding this comment.
When the temporary directory is unavailable or unwritable, a request with a system prompt now emits system_prompt_staging_failed before runCodingAgentTurn can perform its pre-abort, canonical-destination, credential, or CLI checks. Consequently, even an already-cancelled request or invalid provider configuration gets the wrong error and unnecessarily writes request-derived content to disk; move staging into a post-preflight callback or perform it after those checks.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| } finally { | ||
| if (promptDir) await rm(promptDir, { recursive: true, force: true }).catch(() => {}); |
There was a problem hiding this comment.
If recursive removal encounters a transient EPERM or EBUSY—notably on Windows—the empty catch silently leaves the plaintext system/developer prompt in the temporary directory indefinitely. Since this change's privacy guarantee depends on deleting that file after the child exits, use bounded removal retries and retain a cleanup mechanism or diagnostic when deletion still fails rather than discarding the error.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Its title has been prefixed with |
github-actions
Bot
changed the title
1 participant
Motivation
/proc/<pid>/cmdlineinspection.Description
argvand instead stage the folded system+developer prompt in an exclusive temp file and pass its path to the CLI via--system-prompt-filefrombuildArgs.mkdtemp+writeFile(..., { mode: 0o600, flag: "wx" }), fail closed with a clear error if staging fails, and always remove the private temp directory after the turn completes.systemPromptFileparameter throughcreateCodeBuddyAdapter/runTurnsorunCodingAgentTurnreceives args that reference the file rather than the prompt text.tests/providers/codebuddy-adapter.test.tsthat assert the staged file contains the prompt, that the childargvdoes not contain prompt text, that file permissions are restrictive on Unix, and that the file is deleted after the turn.Testing
bun test tests/providers/codebuddy-adapter.test.tsand the CodeBuddy-focused suite passed (23 tests, 0 failures).bun run typecheckandbun run privacy:scan, both of which completed successfully.bun run test; the run showed many passing tests but ultimately aborted due to a parallel-worker panic in this environment unrelated to the CodeBuddy change; the focused CodeBuddy tests remained green.Codex Task