Repository navigation
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe latest commit Files Reviewed (3 files)
The other seven PR files are unchanged since the previous review and were verified current, not re-analyzed. Previous Review Summaries (7 snapshots, latest commit 140304a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 140304a)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe current PR head has regressed two previously-fixed behaviors in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit 8e6e504)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe current PR head has regressed two previously-fixed behaviors in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (10 files)
The latest commit ( Fix these issues in Kilo Cloud Previous review (commit 5b8063b)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (11 files)
Fix these issues in Kilo Cloud Previous review (commit 6fe4ac6)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new failure-history router, dialog panel, and shared taxonomy are correct and access-scoped; the only finding is a conditional blob-URL/anchor cleanup leak in the download handler. Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (11 files)
Fix these issues in Kilo Cloud Previous review (commit 972b045)Status: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit Files Reviewed (3 changed files)
The other seven PR files are unchanged since the prior review and were not re-analyzed. Previous review (commit 138d911)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe commit that was meant to fix the two prior findings does not: the new failure-history error panel is unreachable because the Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 changed files)
The other eight files from the prior review are unchanged since f135bcd and were not re-analyzed. Fix these issues in Kilo Cloud Previous review (commit f135bcd)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryAuthorization and runtime behavior look correct; the notable gaps are a test that does not actually exercise the organization-membership denial it claims to cover, and a transient-error path that silently hides the failure panel. Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (10 files)
Reviewed by deepseek-flash-latest · Input: 55.2K · Output: 6.2K · Cached: 553.2K Review guidance: REVIEW.md from base branch |
f135bcd to
138d911
Compare
972b045 to
6fe4ac6
Compare
5b8063b to
8e6e504
Compare
90a5d82 to
9f4c956
Compare
Users only see a one-line safe error for failed Cloud Agent sessions and cannot inspect which runs failed, when, or why (Kilo-Org#6167). - Add shared user-visible failure message maps to the worker-utils failure taxonomy; the cloud-agent-next projection now reuses them - Add a cloudAgentNextFailures tRPC router scoped to the calling user: per-session failure history (run attempts, codes, timestamps) and a JSON/CSV diagnostics export for support tickets - Show a failure history section with a diagnostics download in the Session Info dialog; expired diagnostics are hidden Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
- Fix the router test fixture: one cli_sessions_v2 row per cloud agent session id, since the table has a unique index on that column - Fix the summary test expectation and the CSV row count - Accept legacy agent_ and control-plane workspace_ session ids in the failure panel gate, so worktree chats are covered - Escape leading formula characters in the CSV export - Select and emit wrapper_run_id in the history and the CSV export - Report the history run limit and label the retention window from session creation; rename the download loading copy
9f4c956 to
140304a
Compare
…e-history # Conflicts: # services/cloud-agent-next/src/session/safe-failure-projection.ts
maphew
left a comment
maphew
left a comment
There was a problem hiding this comment.
Addressed both findings: the error panel is evaluated when the query has no cached history, and the diagnostics download now removes the anchor and revokes the object URL in finally so a throw cannot leak either.
Summary
Closes #6167 (the unresolved part: users have no way to scan and analyze their own threads).
Today a failed Cloud Agent session shows one safe error line. The user cannot see which runs failed, when, or why, and support intake depends on the user manually collecting session IDs. This PR gives the calling user read access to their own failure records and a support-ticket export.
What changes
cloudAgentNextFailurestRPC router (apps/web/src/routers/cloud-agent-next-failures-router.ts), mounted at the root router:getSessionFailureHistory: per-session history with session identifiers, the setup failure (stage, code, responsibility, reason, safe diagnostic), and run attempts (status, timestamps, failure fields,wrapperRunId). The list is capped at the latest 50 runs; the cap is part of the output.exportSessionDiagnostics: the same data as a JSON document or CSV, withcloud_agent_session_id,kilo_session_id, and timestamps so users can paste identifiers into tickets. Expired diagnostics (error_expires_at) are withheld, matching the admin router rule. CSV cells that start with a formula character are guarded against spreadsheet formula injection.agent_and control-planeworkspace_session ids, so worktree chats are included.@kilocode/worker-utils/cloud-agent-failureso the web router reuses one source of truth; projection strings are unchanged (177 service unit tests pass). The setup-failure stage/code unions now share one type.Access rule
A caller can read failure data for a session only when they own the
cli_sessions_v2row (bykilo_user_id), and for organization rows only while they are still a current member of that live (not deleted) organization, enforced byqueryAccessibleCloudAgentSession. Becausecloud_agent_session_idis unique acrosscli_sessions_v2, one session has exactly one owning row. Tests cover both denial directions.Scope decisions
STE note
User-facing copy, commit messages, and this PR use Simplified Technical English: short sentences, active voice, one term per concept.
Verification
tsc --noEmit:@kilocode/worker-utils,services/cloud-agent-nextpass.tsgo --noEmit:apps/webpasses.oxlint+oxfmton changed files: clean.services/cloud-agent-nextunit tests for the projection: 177 pass.cloud-agent-next-failures-router.test.ts: scoping in both directions, timestamp normalization from raw Postgres text, diagnostic expiry, JSON/CSV export, CSV formula guard,wrapperRunId) and a pure unit test for the UI summary helper. The jest suites need the test Postgres and could not run in the authoring sandbox (no database); they run in CI.Review
An adversarial review pass found fixture and copy defects (blocker: fixture violated the unique index on
cloud_agent_session_id; blocker: two wrong test expectations; major:workspace_sessions invisible in the panel; major: CSV formula injection; minor: wrapper_run_id missing, run-cap labeling). All are fixed in 0a1c98b.Follow-ups (not in this PR)