-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(responses): recover encrypted output rejections without masking failures #3753
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
14 commits
Select commit
Hold shift + click to select a range
ff5105c
docs: lock current-dev opaque recovery carry boundaries
invalid-email-address fd17378
docs: preserve missing-content-type Responses preflight parity
invalid-email-address 3b8cf8a
fix(responses): recover exact encrypted output rejection once
invalid-email-address de0d22f
test(responses): cover opaque recovery and headerless streaming
invalid-email-address 840e4c0
test(responses): guard preflight opt-in and safe failed tails
invalid-email-address b73809f
docs: record bounded opaque recovery integration evidence
invalid-email-address 73a69e6
docs: plan opaque preflight transport and inspection finality repair
invalid-email-address 15b6d14
docs: preserve original preflight read rejection without cancellation
invalid-email-address 3e1e611
fix(responses): preserve preflight resets and tee failure outcomes
invalid-email-address 812f7af
docs: align native error delivery and account outcome semantics
invalid-email-address f0cdcb2
test(responses): preserve preflight read resets and client aborts
invalid-email-address 4112efc
docs: include semantic failure usage marker parity
invalid-email-address b86021b
fix(responses): align semantic failure accounting across relay modes
invalid-email-address cd6d4d3
fix(types): infer the configured stream reader result
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Opaque preflight transport and terminal outcomes | ||
|
|
||
| Class C4. Mandatory parent-PR review repair under the existing authorized release | ||
| chain; work phase opaque-transport-finality, criterion c-2. Parent #3753 remains | ||
| open/draft at b73809f7e, child #3754 remains open/draft at f5c88beb9 with its parent | ||
| base restored. No parent merge occurred. The original #3535 was briefly closed | ||
| by an out-of-order follow-up, immediately reopened, and its comment corrected. | ||
| No completion, approval or release gate is waived. | ||
|
|
||
| Public review references: PRRT_kwDOS-0Gi86fqEUo (preflight read failure escapes) | ||
| and PRRT_kwDOS-0Gi86fqEUq (tee EOF reports incomplete despite failed client tail). | ||
| The earlier full CI and independent reviews did not cover these paths. The | ||
| unfinished combo cycle is preserved and must consume the repaired parent before | ||
| its final verification. All execution remains hosted; no local suite/typecheck/ | ||
| build or live Kiro request. | ||
|
|
||
| Implementation is one bounded failure-contract unit in 010_failure_boundaries.md. | ||
| Update the existing parent PR, run exact-head CI, cascade its commit into #3754, | ||
| and require fresh composed CI and review before bottom-up integration. Do not | ||
| close an original or retarget a child as a side effect of an unverified merge: | ||
| verify each preceding command and actual merged state before dependent actions. |
67 changes: 67 additions & 0 deletions
67
devlog/_plan/260906_opaque_transport_finality/010_failure_boundaries.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,67 @@ | ||
| # Preserve preflight read failures and inspection finality | ||
|
|
||
| ## Current ownership | ||
|
|
||
| Core selects native encrypted-output candidates and awaits combo-stream-preflight | ||
| before exposing headers. The preflight owns a bounded retained prefix and one | ||
| reader; replayBufferedResponse already emits that prefix and forwards later read | ||
| errors. Client relays own synthetic failed tails. consumeForInspection owns the | ||
| independent tee terminal callback used by native account health. The shared SSE | ||
| inspector reports real terminals and exposes parsed payload callbacks. | ||
|
|
||
| ## Planned change | ||
|
|
||
| - src/server/responses/combo-stream-preflight.ts: native-only replayReadErrors | ||
| option, default false. Catch only reader.read rejection; opted-in callers get | ||
| an accepted reconstructed stream retaining the bounded prefix and the errored | ||
| reader. Never cancel that errored reader: its original rejection must survive | ||
| the replay into relay/inspection. Default combo callers preserve their prior throw behavior. Do not retry | ||
| or classify a read reset as a decrypt rejection, swallow it, or grow buffers. | ||
| - src/server/responses/core.ts: enable that option only on the native opaque | ||
| preflight. After its await, caller abort takes the existing cancellation cleanup | ||
| path before any replay/rebuild. Other read failures reach the normal mid-stream | ||
| relay and inspection path, not a connect-phase error classifier. | ||
| - src/server/relay.ts: reuse a bounded/redacted bare-error message helper at the | ||
| client boundary and within consumeForInspection's parsed-payload callback. | ||
| Keep that evidence local to this reader rather than borrowing stale log state. | ||
| At clean EOF without a real terminal, a witnessed bare error reports failed | ||
| using the shared terminal HTTP mapper; an error-free EOF remains incomplete. | ||
| Preserve the caller's parsed-payload callback. Real terminals and cancellation | ||
| retain precedence; no extra terminal callback or healthy-account reset. | ||
|
|
||
| ## Rejected alternatives and scope | ||
|
|
||
| A blanket core catch mapped as a connect error can misclassify an already-started | ||
| response's account outcome. Globally replaying all preflight errors changes combo | ||
| behavior. Reporting failure at the first bare error would override a later real | ||
| terminal. Borrowing the client relay's mutable state revives tee scheduling races. | ||
| Use the existing preflight/relay ownership and callback seams instead; no new | ||
| public inspector method, provider policy or retry budget. | ||
|
|
||
| ## Verification | ||
|
|
||
| Existing native request fixtures add created-then-reset and created-then-caller- | ||
| abort cases: no uncaught handleResponses rejection, no sanitize resend, normal | ||
| failed stream or 499 cancellation and appropriate attempt/terminal metadata. | ||
| Run tee/eager variants where selected by the existing harness. Preflight tests | ||
| prove default read-error behavior is unchanged and native opt-in preserves prefix | ||
| and exact failure. Inspection/account-health fixtures cover flat/nested bare | ||
| errors at EOF, prior failure/avoidance not cleared, real-terminal precedence, | ||
| error-free EOF compatibility and cancellation neutrality. Existing redaction, | ||
| byte bounds, no-persistence and one-shot recovery tests remain. | ||
|
|
||
| Independent plan/source/final review; exact parent and cascaded child hosted | ||
| Linux/macOS/gates CI. Final Windows six-shard and release gates remain mandatory. | ||
|
|
||
| ## Usage-marker parity amendment | ||
|
|
||
| Source review confirms the account-health blocker is closed by failed EOF. The | ||
| existing eager callback still labels every synthetic failure as streamAborted, | ||
| though a clean EOF after an explicit upstream error is a semantic failure, not a | ||
| body-read reset (PersistedUsageAttempt documents that distinction). Criterion c-2 | ||
| also requires usage outcome parity, so include this small related correction: | ||
| relay-eager passes optional upstream_error provenance only for that clean-EOF tail; | ||
| core records its semantic failed status without streamAborted. Ordinary reset | ||
| callbacks retain their one-argument shape, 502 and streamAborted. Add request-level | ||
| tee/eager assertions for repeated bare errors versus actual reset; do not infer | ||
| this marker from a stale log message or change real-terminal precedence. |
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
20 changes: 20 additions & 0 deletions
20
devlog/_plan/260906_release_244_followups/041_opaque_recovery_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,20 @@ | ||
| # Opaque recovery implementation evidence | ||
|
|
||
| Source 3b8cf8a8f carries PR3535 with a narrowly scoped preflight opt-in. The default | ||
| combo event classifier is unchanged; only a matched bare error supplied by the | ||
| native decrypt caller is replayable. Headerless streaming is an explicit option | ||
| under the existing core condition. Client-reader error evidence is redacted and | ||
| bounded before a failed tail is synthesized; real terminals remain authoritative. | ||
|
|
||
| Independent plan audit accepted the scoped seam. Independent source/security | ||
| review passed: exact 502 gate, one sanitized rebuild, raw-body object identity, | ||
| no replay after visible output, cancellation and current rewrite ordering remain. | ||
| The source contributor is credited in the carry commit and PR. | ||
|
|
||
| Regression commits cover native function and agent-message history, repeated | ||
| flat/nested errors, both relay shapes, unrelated errors and default combo byte | ||
| preservation, output commitment, missing-header and wrong-media-type controls, | ||
| and bounded synthesized-message redaction. The headerless fixture uses bytes | ||
| and asserts the absence of Content-Type because a string body supplies text/plain. | ||
| No local test suite, typecheck, build or live Kiro request was run. Final evidence | ||
| comes from hosted CI on the complete PR head and a fresh independent review. |
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
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.