Skip to content

fix(cloud-agent-sdk): validate question entries and reset retry and cleared-transcript state - #8531

Merged
iscekic merged 2 commits into
mainfrom
drain/validate-question-entries
Oct 22, 2026
Merged

iscekic merged 2 commits into
mainfrom
drain/validate-question-entries

Conversation

@iscekic

@iscekic iscekic commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — five regression tests fail on main and pass on the branch; the package suite runs 1751 tests green; and the extension sidepanel built from this branch replayed a fixture cloud-agent stream carrying one question entry that does not match the shared shape, where main logged an uncaught render error and ended with an empty panel, while the branch rendered the question that precedes that entry and its Answer action submitted.

Change

  • packages/cloud-agent-sdk/src/schemas.ts, normalizer.ts: question.asked entries are validated against the shared QuestionInfo shape and the normalized event publishes the parsed list instead of casting unknown[]. Answers are positional (answerQuestion(requestId, answers: string[][])), so the list keeps its valid prefix and stops at the first entry that fails the shape; an entry that fails in first position makes the payload carry no questions, which is the same result as the existing non-array fallback, so the pending request still resolves and the card reports the interrupted state. An explicitly empty list stays empty.
  • session-manager.ts clearFailedMessage: the service result is honoured. When the service reports that the pruned entry held the terminal error and has undone it, the mirrored error state is cleared, matching the durable-seed path in the same file.
  • session-manager.ts clearTranscript: resets the omitted-item count and the initial-page accounting baseline to zero, the same reset the non-preserved branch of clearAllAtoms performs, so a cleared view neither reports omitted history nor caches a stale count for the next open.

Verification

  • pnpm --filter @kilocode/cloud-agent-sdk exec jest: 35 suites, 1751 tests pass. typecheck, oxlint and oxfmt --list-different are clean for the package.
  • Five regression tests fail against main and pass on the branch: valid-prefix/truncation/undefined-list normalization, extra-field stripping, retry clearing the terminal error, and the cleared view resetting the omitted accounting (including the page written to the transcript cache).
  • End-to-end on the real UI: the extension sidepanel, built from this branch's bundle, replayed a fixture cloud-agent stream whose question.asked carries a valid question, a malformed entry, then a second valid question. On main the sidepanel recorded an uncaught render error while the card mounted and ended with an empty panel. On the branch the card renders the first question, does not offer the question after the malformed entry, and the Answer action submits. The capture is attached in a comment on this pull request.
  • The two manager-state findings have no in-repo app trigger: no app calls clearTranscript, and the retry residue needs a delivery-failure replay that arrives after the re-send starts, which the UI cannot force. Those are verified at the manager level with the real manager, jotai store, page-application and cache-write paths; the service-state result contract is covered by its own suite.
  • Merge order with fix(cloud-agent-sdk): bound session event payloads #8350, which bounds the same questions array: whichever lands second keeps the bound inside the new list schema. The branch is currently conflict-free with main.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Malformed entry policy: question answers are positional, aligned to the CLI's question indices. How should the normalizer handle a questions array where some entries fail the shared shape?
    Answer: Truncate at the first malformed entry.
    Confidence: 0.60. Precedent: executive decisions under the "is this reasonable?" rule. Truncating keeps answers aligned by position; dropping only the bad entries would silently pair an answer with the wrong CLI question.
  2. Question: When no entry of a non-empty questions array is usable, should the event still be published?
    Answer: Keep the event with the question list undefined.
    Confidence: 0.65. Precedent: the same "is this reasonable?" rule. This matches the existing handling of a non-array payload, so the pending request still resolves and the card shows as interrupted.

@iscekic iscekic added drain Work item from the findings drain janitor:features-workflows Janitor finding family: features-workflows labels Oct 22, 2026
@iscekic

iscekic commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Verified on the extension sidepanel built from this branch, driven by a fixture cloud-agent stream whose question payload carries a valid question, an entry that does not match the shared question shape, then a second valid question.

Observed on this branch, from the live DOM at capture time:

  • the card renders the first question and its option;
  • the question that follows the invalid entry is not offered, because the list is truncated there;
  • the Answer action submits the selection and the question card closes.

The same stream on the base revision ended with an uncaught render error while the card mounted and an empty sidepanel body; the capture here is supplementary to those DOM assertions.

Extension sidepanel showing the validated question card

@kilo-code-bot

kilo-code-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit types the validated question entries with the shared QuestionInfo and extracts a questionOptionSchema helper; the runtime behavior is unchanged and the mirror still matches packages/app-shared/src/opencode.gen.ts field-for-field, so no defects were found in the changed code.

Assumptions: [INFERENCE] I did not execute the jest suite or tsgo (read-only review). The exactOptionalPropertyTypes rationale in the new toQuestionInfo comment is not enabled in packages/cloud-agent-sdk/tsconfig.json; it is enabled in apps/extension/tsconfig.json, which typechecks the SDK source through the package export map, so the conditional spreads remain the correct construction for that consumer. [INFERENCE] The pre-existing truncate-at-first-malformed-entry and QuestionInfo[] | undefined policies were reviewed in the previous pass and are unchanged here.

Files Reviewed (1 file)
  • packages/cloud-agent-sdk/src/schemas.ts
Previous Review Summary (commit 1875dcd)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 1875dcd)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the question.asked validation/truncation schema, the retry-driven errorAtom clear, and the cleared-transcript omitted-history reset; no defects found in the changed code.

Assumptions:

  • [INFERENCE] I did not execute the jest suites (read-only review). The PR reports 1751 passing tests; the new tests match the code paths I traced.
  • [INFERENCE] When a questions list is truncated at the first malformed entry, the client submits a shorter answers array than the CLI's question count. Whether the CLI rejects or partially accepts that is unverified; this is the deliberate, documented tradeoff (positional alignment preserved) rather than a defect.
  • [INFERENCE] The hand-written questionInfoSchema mirrors the generated QuestionInfo in @kilocode/app-shared/opencode and matches it today (fields, option shape, optional flags). Future drift in the generated type is a maintainability risk, not a current finding.
  • The unbounded size of the questions array is unchanged by this PR and is scoped to fix(cloud-agent-sdk): bound session event payloads #8350 as noted in the description.
Files Reviewed (5 files)
  • packages/cloud-agent-sdk/src/schemas.ts
  • packages/cloud-agent-sdk/src/normalizer.ts
  • packages/cloud-agent-sdk/src/session-manager.ts
  • packages/cloud-agent-sdk/src/normalizer.test.ts
  • packages/cloud-agent-sdk/src/session-manager.test.ts

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 35.5K · Output: 4.7K · Cached: 465.8K

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 22, 2026
@iscekic
iscekic enabled auto-merge October 22, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain janitor:features-workflows Janitor finding family: features-workflows merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants