-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(responses): accept complete external task inputs without weakening tool guards #3743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
a73bb16
fix(responses): preserve complete external task-input envelopes
invalid-email-address 79810cf
test(responses): cover external task input and retained rejection bou…
invalid-email-address e274094
docs(responses): record task-input implementation and proof boundary
invalid-email-address 815f112
test(responses): distinguish opaque fixtures from normalized plaintext
invalid-email-address b24ed35
Merge branch 'codex/release-244-auth-port-fixtures-07c0' into codex/r…
invalid-email-address b7e67d8
fix(responses): align stateful external-task guidance in raw replay
invalid-email-address File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
devlog/_plan/260906_release_244_followups/021_task_input_implementation.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # External task input implementation | ||
|
|
||
| The pure task-input leaf validates the complete external envelope before using the | ||
| existing input-content converter. It accepts text and URL-backed images, rejects | ||
| partial or opaque arrays as a whole, and preserves accepted content order. The | ||
| parser uses the result for both the continuation boundary and a user turn that | ||
| clears pending reasoning. Ordinary tool results, the core call-id guard and raw | ||
| passthrough handling remain unchanged. | ||
|
|
||
| Existing unpaired-result regressions remain in place. Added parser cases cover | ||
| shape/content controls, original image detail, frozen input, continuation and | ||
| reasoning separation; HTTP cases exercise accepted text/images and rejected | ||
| envelopes before upstream work. A passthrough case verifies the existing raw | ||
| orphan-output behavior alongside the new parsed user representation. | ||
|
|
||
| The implementation preserves Yrlan's contributor attribution from the public | ||
| issue and supplied proposal. Protocol/security review and hosted CI are recorded | ||
| on the fixing PR and source-bound cycle receipt. No local test suite, typecheck, | ||
| build or live Kiro request is part of this validation. | ||
|
|
||
| The first hosted run exposed two invalid HTTP test stimuli: short text in an | ||
| encrypted_content slot follows the existing plaintext normalization path before | ||
| the parser. The negative fixtures now use synthetic ciphertext-shaped content | ||
| with an explicit classifier check; a separate positive control retains plaintext | ||
| slot compatibility. The 400/no-upstream assertions and production logic are unchanged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # Stateful external-task guidance consistency | ||
|
|
||
| Parent PR #3743 recognizes a complete external task-input envelope as a user turn | ||
| and starts the parsed continuation boundary there. Its review identified the | ||
| remaining raw insertion predicate in collaboration.ts, which still recognizes | ||
| only ordinary user/assistant messages and agent_message. In a stateful delta, | ||
| generated guidance can therefore precede the task in parsed messages but follow | ||
| it in the stored raw input; reparsing changes the delivered order. | ||
|
|
||
| This C4 protocol/replay follow-up is a separate PABCD work-phase before Kiro | ||
| implementation resumes. The Kiro phase remains open with no code changes; the | ||
| goalplan gained an additional criterion and an explicit focus cursor, without | ||
| marking any unfinished task complete or weakening existing criteria. | ||
|
|
||
| Archetype: spec-satisfaction repair. Goal: the same conversational boundary in | ||
| parsed and raw stateful representations. Non-goals: new envelope forms, broader | ||
| tool-output repair, stateless insertion changes, auth changes or live Kiro. | ||
| Verifier: hosted ci.yml runtime/type/privacy gates and focused regression cases | ||
| in tests/codex-integration/multi-agent-compat.test.ts. No local test suite, | ||
| typecheck or build. Stop only after exact-head CI and independent review pass, | ||
| parent review is resolved and its verified head is ready for the Kiro cascade. | ||
|
|
||
| Resources inherit the authorized release loop: existing repository/GitHub access, | ||
| requested xai/grok-4.6 reviewers, no new credentials or purchases, no fixed model | ||
| cost cap, bounded processes and status waits. Main owns code/FSM/GitHub actions; | ||
| reviewers are read-only. Reclaim failed dispatches; no implicit phase movement. | ||
| Design and final source/CI evidence reside in this unit and the bound goalplan. | ||
|
|
||
| The complete implementation map is 010_raw_boundary.md. Apply the verified delta | ||
| to parent #3743, then refresh the saved Kiro branch from that parent before B. |
38 changes: 38 additions & 0 deletions
38
devlog/_plan/260906_stateful_task_guidance/010_raw_boundary.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Align the stateful raw conversation boundary | ||
|
|
||
| ## Exact diff map | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| - MODIFY src/server/responses/collaboration.ts: import the existing pure | ||
| externalTaskInputContent helper. In isConversationalItem, recognize a complete | ||
| external task envelope with helper(item) !== undefined, alongside existing | ||
| agent_message and user/assistant message handling. Do not duplicate its shape | ||
| validator or alter statefulRawInsertionIndex's replay-prefix/fallback logic. | ||
| - MODIFY tests/codex-integration/multi-agent-compat.test.ts near injectDeveloperMessage: | ||
| stateful external envelope alone and after a leading ordinary call result must | ||
| receive guidance before the external task in both parsed context and raw input. | ||
| Reparse the stored raw body and compare role/content order. Add an expanded | ||
| replay-prefix case so historical external inputs are not selected as the new | ||
| boundary. Keep ordinary stateful protocol, compaction and guidance-dedup tests. | ||
| - MODIFY docs-site/src/content/docs/guides/sub-agent-surface.md and | ||
| structure/04_transports-and-sidecars.md: distinguish unchanged payload content | ||
| from intentional generated-guidance placement; both representations use the | ||
| same complete-envelope boundary during stateful injection. | ||
|
|
||
| Before: parsed [developer, user] while raw [external-envelope, developer]. | ||
| After: parsed [developer, user], raw [developer, external-envelope], and reparsed | ||
| role/content order agrees. Leading protocol results remain before guidance; | ||
| historical replay-prefix items remain in place. | ||
|
|
||
| ## Activation and boundary proof | ||
| The new predicate executes only when stateful guidance inspects raw input. Tests | ||
| set previous_response_id, invoke the real injector and assert raw/parsed/reparsed | ||
| arrays. Ordinary tool outputs with call_id remain protocol items because the | ||
| shared helper rejects them. Invalid/partial/opaque envelopes retain their current | ||
| classification; the complete validator is already covered by parent regressions. | ||
|
|
||
| No persisted schema, configuration or role changes. Existing input shape -> shared | ||
| validation -> raw insertion index -> stored raw input -> later parser is the full | ||
| data flow. The helper remains pure and adds no optional subsystem dependency. | ||
| Review uses the actual diff; all runtime checks execute in GitHub Actions. | ||
|
|
||
| ## A audit amendment | ||
| Use the parse-time previous_response_id pattern from multi-agent-compat.test.ts:1075-1089 for envelope-alone and leading-result cases. The raw body must contain that field before parseRequest and retain it during reparse; do not copy the post-hoc parsed.previousResponseId assignment fixture at1029. For historical-prefix coverage use the1043-1072 pattern with explicit _replayPrefixLen and _continuationConversationMessageIndex, and put an old external envelope inside that preserved prefix. Assert parsed boundary before injection as well as raw/parsed/reparsed ordering. This closes the auditor's false-green fixture concern. | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
16 changes: 16 additions & 0 deletions
16
devlog/_plan/260906_stateful_task_guidance/011_implementation.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Implementation and verification boundary | ||
|
|
||
| The raw conversational-item predicate now reuses externalTaskInputContent, matching | ||
| the parsed continuation predicate without another envelope validator. Replay-prefix | ||
| skipping and the existing fallback remain unchanged. | ||
|
|
||
| Three new cases parse with previous_response_id already in the raw body, exercise | ||
| external input alone or after a real protocol result, preserve a historical external | ||
| envelope in the replay prefix, and compare raw/parsed/reparsed role-content order. | ||
| They retain the stateful field during reparse and assert the initial parsed boundary, | ||
| avoiding a fixture that could accidentally validate stateless behavior. | ||
|
|
||
| Apply this review fix to #3743. Source review and exact-head hosted CI are recorded | ||
| on that PR and in the cycle receipt; no local test suite or live Kiro request is run. | ||
| After verification, resolve the review and refresh the preserved Kiro branch before | ||
| its implementation cycle continues. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.