fix(kiro): gate diagnostic request-body encoding behind debug flag - #236
fix(kiro): gate diagnostic request-body encoding behind debug flag#236luvs01 wants to merge 1 commit into
Conversation
|
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: Team 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5dc7a8c4d
ℹ️ 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".
| test("buildRequest does not encode the request body for diagnostics when debug is disabled", async () => { | ||
| const encode = spyOn(TextEncoder.prototype, "encode").mockImplementation(() => { | ||
| throw new Error("diagnostic encoding must stay behind the debug gate"); |
There was a problem hiding this comment.
Explicitly disable all provider-debug sources in this test
When the test suite is launched with the supported OCX_DEBUG=1 environment setting, this test enters the diagnostic branch and the mocked encoder throws; this is reproducible with OCX_DEBUG=1 bun test tests/kiro-stream.test.ts --test-name-pattern 'buildRequest does not encode'. The shared setup only deletes legacy OCX_DEBUG_FRAMES, while isDebugEnabled() also reads OCX_DEBUG and runtime overrides, so explicitly clear/reset those sources (or force the runtime debug setting off) before asserting that diagnostics are disabled.
AGENTS.md reference: AGENTS.md:L228-L230
Useful? React with 👍 / 👎.
|
Superseded by upstream lidge-jun/opencodex#3837, which carries this fix forward against current This PR's head dates from 2026-08-10 and targets The upstream PR keeps the same defect and the same fix, rebuilt on
Verified there: 418 Kiro tests pass, typecheck, privacy scan, and Closing this fork PR to keep a single tracked item for the work. |
Motivation
bodyBytesby callingnew TextEncoder().encode(body).lengthwhile building requests, which forced a full UTF-8 encoding (and allocation) of the upstream request body even when diagnostic logging was disabled, creating a resource-exhaustion risk.Description
isDebugEnabledand move the diagnosticbodyBytescomputation and thedebugProviderDiagnostic(...)call behind anif (isDebugEnabled())gate insrc/adapters/kiro.tsso expensive encoding only happens when provider debug is enabled.tests/kiro-stream.test.tsthat spies onTextEncoder.prototype.encodeand fails if it is called when diagnostics are disabled to prevent regressions.src/adapters/kiro.tsandtests/kiro-stream.test.tsand preserve existing runtime request shape and behavior when debug is enabled.Testing
bun test tests/kiro-stream.test.tsand the new regression (buildRequest does not encode the request body for diagnostics when debug is disabled) passed in the targeted runs.bun run typecheckwhich succeeded.bun run privacy:scanwhich succeeded.bun run test) which surfaced unrelated, environment-sensitive failures in management-auth tests (HTTP 403 responses) that are not caused by this change; the failures are outside the scope of this diagnostic-gating fix.Codex Task