🏷️ fix: Keep Redacted Child Labels in Activity Phase Prompts - #573
danny-avila wants to merge 1 commit into
Conversation
|
@codex review |
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. |
![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: 8d9124ae27
ℹ️ 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 childLabelsRedacted = | ||
| redaction != null && | ||
| redactionContexts.length > 0 && | ||
| redactionContexts.every((context) => | ||
| coversToolOutputRedaction( | ||
| hasToolOutputTracingConfig(this.langfuse, context.langfuse) | ||
| ? resolveToolOutputTracingConfig(this.langfuse, context.langfuse) | ||
| : undefined, | ||
| redaction | ||
| ) |
There was a problem hiding this comment.
childLabelsRedacted is inferred only from the agents' current configurations, but ActivityPhaseEntry.label is a host-supplied string with no guarantee or metadata proving it was produced by generateActivityLabel under that policy. When a host supplies a legacy, manual, or otherwise unredacted committed label and all contributing agents share the active policy, this becomes true and the label—including any copied sensitive tool output—is sent to the phase model and recorded as generation input. Require per-label redaction provenance rather than treating matching agent configuration as proof.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
| for (const toolName of required.redactedToolNames) { | ||
| if (!candidate.redactedToolNames.has(toolName)) { | ||
| return false; |
There was a problem hiding this comment.
With partial matching, set membership is not equivalent to redaction coverage: a candidate pattern such as run_ redacts every tool matched by a required pattern run_select_query, but this check returns false because the longer string is not literally in the candidate set. This occurs, for example, when the phase destination has the narrower partial rule and the contributing agent has the broader one; childLabelsRedacted remains false and a labels-only phase still returns {} despite the labels having been generated under a strictly stronger policy.
Useful? React with 👍 / 👎.
1 participant
What breaks
With any tool-output redaction policy active,
Run.generateActivityPhaseLabelnever produces a phase title for phases whose activities carry committed child labels, which is the normal case once activity labels are on. The model is not called and the host's parent header falls back to the last activity's label.A deployment that redacts a single tool it rarely uses (for example
redactedToolNames: ['run_select_query']) loses phase titles on every run, including runs that never touch that tool.Cause
buildActivityPhaseLabelPrompttreats any active policy as suppressing all free-form evidence:That gate drops committed child labels along with reasoning excerpts and assistant commentary. Hosts replace an activity's raw tool entries with its child label once the label commits, so a phase of labeled activities has no remaining evidence, the prompt is empty, and the early
if (userPrompt === '') return {}fires.Reasoning and commentary are rightly suppressed: they can quote output from an earlier call to a redacted tool. Child labels are different.
generateActivityLabelbuilds each one from redacted evidence under its agent's own resolved policy, replacing redacted outputs with the redaction text and dropping free-form prose, so a child label cannot carry output that policy hides.Change
A child label may stand in the phase prompt when the policy it was generated under covers the merged phase policy.
generateActivityPhaseLabelalready resolves each contributing agent's policy to build the merged one; it now also checks that every one of those policies covers the merge:coversToolOutputRedaction(candidate, required)is true whencandidateglobally disables tool-output tracing, or when it names every toolrequirednames and matches at least as broadly (anexactcandidate does not cover apartialrequirement). A context with no policy never covers an active one.buildActivityPhaseLabelPrompttakes the result aschildLabelsRedactedand uses it only for committed labels:Reasoning excerpts and assistant commentary stay gated on
freeFormSuppressedexactly as before. Raw tool entries are unchanged and still redact per tool.What stays suppressed
redactionContextsthen spans every agent, so one weaker agent anywhere suppresses them.Tests
activity-phase-label.test.ts: a run with a single run-level policy summarizes two labeled activities through the model, with the labels in the prompt and commentary absent; a multi-agent run where one contributing agent has no policy returns{}without a model call.activity-label-prompt.test.ts:childLabelsRedactedadmits labels but not reasoning or commentary; without it, a labels-only phase under an active policy still builds an empty prompt;coversToolOutputRedactioncases for a missing candidate, global disable in both directions, name subsets, and exact versus partial matching.Both new behavior tests fail without the source change. The 13 Langfuse and activity suites pass (239 tests), and
tsc --noEmitand ESLint are clean on the changed files.Built into a LibreChat checkout that sets a uniform
run_select_query/run_tools_with_bashredaction policy, this change turns theactivity-fold.spec.tse2e suite from 2 failed to 2 passed with the policy still in place.