fix(chat): deliver provider transparency note via system-prompt channel, not the prompt#3683
fix(chat): deliver provider transparency note via system-prompt channel, not the prompt#3683djm204 wants to merge 1 commit into
Conversation
…el, not the prompt The runtime-status note (ADR-040) was appended directly onto the raw user-turn prompt during CLI session continuation, making it textually indistinguishable from injected content. Claude's own anti-injection training correctly flagged and refused to trust it, undermining the feature end-to-end. Route it through Claude Code CLI's --append-system-prompt instead, verified live to be trusted (even when contradicting the model's default self-report) and to survive --continue/--resume. Codex and Gemini have no equivalent flag, so they keep the prior append-to-prompt behavior via a new ICliProvider.supportsSystemPromptAddendum? capability check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
![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: d537279058
ℹ️ 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".
| const completeOptions = { | ||
| sessionContinue: shouldContinue, | ||
| ...(sessionId ? { sessionId } : {}), | ||
| ...(transparencyNote ? { systemPromptAddendum: transparencyNote } : {}), |
There was a problem hiding this comment.
When this engine is backed by the shared CachedCliLlmClient/operation router, the note added here never reaches CliLlmAdapter: CachedCliLlmClient.complete() rebuilds completionOptions with only signal and timeoutMs, and its invoke() request forwards only those two fields. In that context the provider/fallback transparency note is silently discarded instead of going through Claude's system-prompt channel or the append fallback, so cached callers of the ILlmClient API still produce answers without the runtime facts; please propagate systemPromptAddendum through that wrapper as well, accounting for cache/session keys as needed.
Useful? React with 👍 / 👎.
| provider: activeProvider, | ||
| ...(activeModel ? { model: activeModel } : {}), | ||
| content: prompt, | ||
| content: effectivePrompt, |
There was a problem hiding this comment.
When replay capture is enabled and the active provider supports the new system-prompt channel, effectivePrompt is deliberately only the user prompt, so the llm.request replay record omits the runtime-status addendum that was actually sent via --append-system-prompt. Audits or replays of Claude turns after a fallback will be missing the instruction that made the model trust the provider facts, while non-Claude attempts still record it because it is appended to stdin; record the addendum separately or include it in the replay payload.
Useful? React with 👍 / 👎.
| const completeOptions = { | ||
| sessionContinue: shouldContinue, | ||
| ...(sessionId ? { sessionId } : {}), | ||
| ...(transparencyNote ? { systemPromptAddendum: transparencyNote } : {}), |
There was a problem hiding this comment.
With this change the transparency note is no longer part of the first completeWithUsage argument, but the unchanged REST/WS integration tests still assert the second turn prompt contains the fallback text (tests/integration/chat/chat-routes.test.ts:300 and tests/integration/chat/ws-chat-server.test.ts:627-629). In CI those mocks will receive the note on the options object instead, so these tests fail until they inspect mock.calls[1][1].systemPromptAddendum (or equivalent) rather than the prompt.
Useful? React with 👍 / 👎.
1 participant
Summary
--append-system-promptflag instead of concatenating it into the prompt. Verified live before implementing: content delivered this way is trusted even when it contradicts the model's default self-report, and survives--continue/--resumesession continuation.codex exec --help/gemini --helpchecked), and Codex was already observed to accept the raw-appended note without flagging it — so those providers keep the prior append-to-prompt behavior via a newICliProvider.supportsSystemPromptAddendum?()capability check.systemPromptAddendumoption throughLlmCompletionOptions→AdapterLlmClient→CliLlmAdapter.execute()→ProviderOpts, following the same per-provider-capability pattern asextractUsage?/extractModel?.Test plan
packages/franken-orchestrator: full suite green (283 files / 4435 tests), typecheck clean, build cleanpackages/franken-types: full suite green (11 files / 130 tests), typecheck clean, build cleanclaude-provider.test.ts(--append-system-promptwiring +supportsSystemPromptAddendum),cli-provider.test.ts(ProviderOptsshape),cli-llm-adapter.test.ts(Claude routes via the flag with stdin untouched; Codex falls back to append-to-prompt),adapter-llm-client.test.ts(option passthrough),conversation-engine.test.ts+runtime-parity.test.ts(note delivered viasystemPromptAddendum, never in the prompt string)claudeCLI: two-turn session recalled a session-native fact from turn 1, and on turn 2 truthfully explained a fabricated codex→claude fallback frompriorProviderContextwith no injection-suspicion language and persona intact🤖 Generated with Claude Code