Skip to content

Commit fa212c7

Browse files
committed
devlog: finalize overnight PR review — 3 merged, 6 rebuild, 1 keep-open
Sol reviewed all 10 PRs. Results: - Merged: #286 (500→400), #296 (Cursor Router), #302 (Kiro hardening) - Rebuild-on-dev: #255, #283, #293, #301, #298, #299 - Keep open: #249 (Cloudflare tunnel) Typecheck + full test suite pass (3574/3574) after all merges.
1 parent 49e586d commit fa212c7

1 file changed

Lines changed: 22 additions & 24 deletions

File tree

devlog/_plan/260723_overnight_pr_review/080_pr302_kiro_hardening.md

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,29 @@
22

33
- **Author:** mushikingh
44
- **Branch:** feat/kiro-integration-hardening → dev
5-
- **CI:** enforce-target pass, missing full CI run
6-
- **Decision:** MERGE (last, highest risk)
7-
- **Risk:** High (8 commits, touches core bridge.ts, types.ts, responses.ts, state.ts, parser.ts, schema.ts)
5+
- **Size:** 41 files, 3,324 additions, 11 commits
6+
- **Sol Review:** Schrodinger — VERDICT: GO-WITH-FIXES (blockers=0)
7+
- **Decision:** MERGE_AND_IMPROVE ✓ (committed 49e586d9)
88

9-
## Changes (8 commits)
9+
## Sol Review Summary
1010

11-
1. Provider-aware terminal state: `OcxProviderContinuationState` type.
12-
2. `OcxMessagePhase` type for phase-aware text deltas.
13-
3. `incomplete` event type in bridge SSE handling.
14-
4. `adapterFailureFromEvent()` for structured error extraction.
15-
5. `endTurn` field in response snapshots.
16-
6. `retryable` flag on error/incomplete responses.
17-
7. Non-streaming (`buildResponseJSON`) gets same phase/incomplete support.
18-
8. Tests: bridge.test.ts (+87 lines), responses-parser.test.ts (+15), responses-state.test.ts (+43).
11+
### Issues (all medium/low — no blockers)
12+
1. Medium — Structured adapter status lost across Responses→Claude bridge
13+
(Kiro 401 becomes Anthropic overloaded_error instead of authentication_error)
14+
2. Medium — Kiro persists store:false conversation contents to disk
15+
(file mode 0600, no token leak, but caller prompts may contain secrets)
16+
3. Low — OcxProviderContinuationState type broader than implementation
17+
4. Low — Incomplete continuation behavior internally inconsistent
1918

20-
## Files Modified
19+
### Strengths noted by Sol
20+
- end_turn is additive, optional, backwards compatible
21+
- buildResponseJSON mirrors streaming changes correctly
22+
- Test coverage "unusually strong"
23+
- No credential/token exposure found
24+
- Security: redaction, bounded error bodies, 0600 permissions
2125

22-
- `src/bridge.ts` — major: incomplete event, phase-aware messages, structured errors
23-
- `src/responses/parser.ts`, `schema.ts`, `state.ts` — minor extensions
24-
- `src/server/responses.ts` — non-streaming JSON builder updates
25-
- `src/types.ts` — new types (OcxMessagePhase, OcxProviderContinuationState)
26-
- Tests: 3 test files updated
27-
28-
## Review Notes
29-
30-
- Well-structured incremental commits.
31-
- Backwards compatible: new fields are optional, `incomplete` is additive.
32-
- Merge LAST to minimize conflict surface with #283 and #286 which also touch responses.ts.
26+
## Post-merge improvements needed (future commits)
27+
1. Coherent error status/type/code across Responses and Claude bridges
28+
2. Memory-only continuation for store:false (no disk persistence)
29+
3. Tighter OcxProviderContinuationState type
30+
4. Formalize incomplete response chaining behavior

0 commit comments

Comments
 (0)