fix(cloud-agent-sdk): treat unknown CLI capabilities as supported - #6662
Conversation
Surface: the mobile app (apps/mobile) and the cloud-agent SDK (packages/cloud-agent-sdk). A capability gate that depends on CLI support must default to YES. Today it defaults to NO, so a feature disappears until the CLI advertises it. Evidence: - `packages/cloud-agent-sdk/src/session-manager.ts:880` creates `supportsAttachmentsAtom` as `atom(false)`. - `recomputeSupportsAttachments` (`:1468`-`:1485`) sets `true` for `cloud-agent`, and `currentCapabilities?.attachments === true` for `remote`. Every other remote state (absent, false, mid-reconnect) sets `false`. - `apps/mobile/src/components/agents/session-detail-content.tsx:2298` passes that atom to `attachmentsEnabled`, so the paperclip is absent until the CLI reports the capability. Requirements: - Optimistic default: while the CLI capability is unknown, the gate reports supported. - Downgrade only on an explicit negative. A heartbeat or `sessions.list` row that says `attachments === false` sets the gate to false. - A `read-only` session stays unsupported. - Apply the same rule to every gate in this file that reads a CLI capability. Attachments is one example, not the whole set. - The downgrade must still take effect as soon as the CLI reports it. Do not lose the reconciliation. Proof: unit tests for unknown -> true, explicit false -> false, `cloud-agent` -> true, `read-only` -> false. Then one live proof on the platform you choose: open a remote session whose CLI has not yet reported capabilities, and show the attachmen
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only incremental change since 55d11fd is a test-only refactor in Files Reviewed (1 file)
Previous Review Summaries (6 snapshots, latest commit 55d11fd)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 55d11fd)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review (55d11fd) is a test-only refactor in Files Reviewed (1 file)
Previously reported findings remain addressed: the spawn dispatch now re-runs file and clone admission against the refreshed live instance before committing (85073ce), and the SDK send guard requires Previous review (commit be77704)Status: No Issues Found | Recommendation: Merge Executive SummaryThe two follow-up fixes are correct and consistent with the existing gates: the spawn dispatch re-runs file and clone admission against the refreshed live instance before committing, and the SDK send guard now requires Files Reviewed (4 files)
Previous review (commit 4d93db2)Status: No Issues Found | Recommendation: Merge Executive SummaryThe net PR is scoped to the CLI-capability change: unknown Files Reviewed (18 files)
Previous review (commit e948963)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the only files changed after Files Reviewed (2 files)
Previous review (commit 7caa120)Status: 1 Issues Found | Recommendation: Address before merge Executive SummaryThe incremental commits add a mobile-only Overview
Issue Details (click to expand)CRITICAL
Fix these issues in Kilo Cloud Files Reviewed (22 files)
Previous review (commit fac3728)Status: No Issues Found | Recommendation: Merge Executive SummaryThis PR flips the CLI capability gates from fail-closed ( Files Reviewed (17 files)
NotesThe optimistic default is intentional per the PR requirements: files can be admitted while Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
|
kilo-review — independent audit of the published diff. Status: 1 Issues
|
…ent model) (kwf kwf-fix-review-af0e/gr2)
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
The /container/ entry in the /api/towns/:townId/* skip list let every Town Container control-plane route (agents/start, agents/:id/stop, agents/:id/message, agents/:id/status, agents/:id/stream-ticket, health, pty) bypass kiloAuthMiddleware, adminAuditMiddleware and townAuthMiddleware. The handlers proxy straight to the container control server and check no authorization of their own, so any principal that clears Cloudflare Access could drive another tenant's container by supplying its townId. CF Access authenticates the caller but does not enforce town ownership. Drop the skip and return the middleware response instead of awaiting it, so an unauthenticated caller gets the middleware 401 instead of a dropped response. Update the container route comment and the two integration tests that asserted an unauthenticated request reached the body validator.
The gastown auth and Durable Object fixes, the session-ingest test repair, and the security-auto-analysis integration config came from an unrelated backend gate repair. They do not belong to a cloud-agent-sdk capability change. Reverts those trees to the branch merge base (8e59fe6). The same gastown fix is present in #6689 and #6580.
…y-default-yes-a3fb
The file and clone checks ran against the press-time row before the refetch resolved the live row. A rebooted host can come back on a new connectionId and report an explicit refusal the press-time row did not, so the spawn could use a row that now refuses the file payload or the clone source. Both checks now run again against the live row before the spawn commits; the attempt was already admitted, so a refusal fails it and re-arms the abandon guard.
The send guard checked the session type and the CLI capability, but not the consumer's declaration that it can deliver remote attachment parts. The UI gate disables the attachment control without that declaration, so a caller that supplied attachmentParts could still have them forwarded. The guard now requires config.supportsRemoteAttachmentParts, the same condition the gate uses.
The two new stubs returned Promise.resolve from a plain arrow, which the repo's promise rules reject (promise-function-async and prefer-await-to-then). An async arrow satisfies both without the disable comment the older stubs needed.
An async arrow trips require-await in the mobile lint config, and a bare Promise.resolve arrow trips promise-function-async and prefer-await-to-then. The file's established single-line stub with the disable comment satisfies all three; the refreshed list is hoisted to a const so the stub stays on one line.
…y-default-yes-a3fb
|
Audit finding ( |
2 participants
Changelog for users
Changelog for maintainers
capabilities.attachmentsis unknown; only an explicitfalsedowngrades it.cloud-agentstays supported andread-onlystays unsupported; sending attachment parts to a non-remote or explicitly incapable session is still rejected.cliCapabilitySupported(value)helper (value !== false) backs both the gate and the send-time check.onResolvedandonTransportCapabilitiesChangeboth recompute, so a heartbeat orsessions.listnegative applies immediately.sessionClone, and share-to-CLI admission.truefor unknown session ids.recomputeSupportsAttachmentsand the send guard in the session manager, then the mobile!== falseconversions.E2E proof
Owner request
E2E proof
/home/igor_kilocode_ai/.local/share/kwf/sections/app-cli-capability-default-yes-a3fb/e2e-mobile-app/device.logOpen findings (not fixed here)
attachmentsfield): the composer attachment (paperclip) control is present.: reported skip, so nothing proves it (No mobile device on this linux host: my orient runcapabilities.attachments: false(or open a session whose row says false): the paperclip disappears and sending a file is refused with the 'can't receive files' error.: reported skip, so nothing proves it (Same device absence ('ANDROID CLAIMED