Skip to content

fix(vscode): persist false agent toggle values#7835

Open
sainzs wants to merge 5 commits intoKilo-Org:mainfrom
sainzs:fix/agent-toggle-false-persistence
Open

fix(vscode): persist false agent toggle values#7835
sainzs wants to merge 5 commits intoKilo-Org:mainfrom
sainzs:fix/agent-toggle-false-persistence

Conversation

@sainzs
Copy link
Copy Markdown

@sainzs sainzs commented Mar 27, 2026

Context

Fixes #7833.

The Agents Behavior settings screen could turn Disable or Hide on for an agent, but turning either toggle back off would not persist after Save. The root cause was that the webview encoded the "off" state as undefined, which gets omitted from the saved config patch, so a previously saved true was never overwritten.

Implementation

Updated packages/kilo-vscode/webview-ui/src/components/settings/ModeEditView.tsx so the Hide and Disable toggles write explicit boolean values instead of collapsing false to undefined.

Also added regression tests in packages/kilo-vscode/tests/unit/config-utils.test.ts to verify nested agent config preserves explicit false values through the same config-merge path used by the settings draft state.

Screenshots

before after
Turning Hide/Disable off snaps back to enabled after Save Turning Hide/Disable off persists after Save

How to Test

  1. Open Settings > Agents Behavior > Agents.
  2. Edit any agent and enable Hidden or Disable, then save.
  3. Re-open the agent, turn the same toggle off, and save again.
  4. Confirm the toggle remains off after the config round-trip.
  5. Run bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts.

Get in Touch

Discord: add your handle here if you want maintainer follow-up there.

@sainzs
Copy link
Copy Markdown
Author

sainzs commented Mar 27, 2026

Verification completed locally:

  • bun install
  • bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts
  • bunx prettier --check packages/kilo-vscode/webview-ui/src/components/settings/ModeEditView.tsx packages/kilo-vscode/tests/unit/config-utils.test.ts
  • bun run --cwd packages/kilo-vscode check-types
  • git push -u sainzs fix/agent-toggle-false-persistence (pre-push hook ran bun turbo typecheck and passed)

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot bot commented Mar 27, 2026

Code Review Summary

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/config/config.ts 845 steps: null is still collapsed by the Agent transform, so clearing a steps override will not persist.
Other Observations (not in diff)

Previous default_agent SDK contract mismatch noted on this PR is resolved in the current revision.

Files Reviewed (12 files)
  • packages/kilo-vscode/src/KiloProvider.ts
  • packages/kilo-vscode/tests/unit/agent-behaviour-patches.test.ts
  • packages/kilo-vscode/tests/unit/config-utils.test.ts
  • packages/kilo-vscode/webview-ui/src/components/settings/AgentBehaviourTab.tsx
  • packages/kilo-vscode/webview-ui/src/components/settings/ModeEditView.tsx
  • packages/kilo-vscode/webview-ui/src/components/settings/agent-behaviour-patches.ts
  • packages/kilo-vscode/webview-ui/src/context/config.tsx
  • packages/kilo-vscode/webview-ui/src/types/messages.ts
  • packages/kilo-vscode/webview-ui/src/utils/config-utils.ts
  • packages/opencode/src/config/config.ts - 1 issue
  • packages/sdk/js/src/v2/gen/types.gen.ts
  • packages/sdk/openapi.json

Reviewed by gpt-5.4-20260305 · 824,020 tokens

@sainzs
Copy link
Copy Markdown
Author

sainzs commented Mar 27, 2026

Follow-up from review is now addressed.

Added the missing clear path for default_agent so these cases persist after Save:

  • selecting the blank/default option in the default-agent dropdown
  • hiding the current default agent
  • disabling the current default agent

Verification re-run locally:

  • bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts
  • bunx prettier --check packages/kilo-vscode/webview-ui/src/components/settings/ModeEditView.tsx packages/kilo-vscode/webview-ui/src/components/settings/AgentBehaviourTab.tsx packages/kilo-vscode/webview-ui/src/types/messages.ts packages/kilo-vscode/src/KiloProvider.ts packages/opencode/src/config/config.ts packages/kilo-vscode/tests/unit/config-utils.test.ts
  • bun run --cwd packages/kilo-vscode check-types
  • bun run --cwd packages/opencode typecheck
  • git push (pre-push hook ran bun turbo typecheck and passed)

this.pending++
try {
await this.client.global.config.update({ config: partial }, { throwOnError: true })
await this.client.global.config.update({ config: partial as Config }, { throwOnError: true })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: The SDK contract is still stale for default_agent

This cast fixes the VS Code caller, but packages/sdk/js/src/v2/gen/types.gen.ts still exposes default_agent?: string, so any other SDK consumer remains unable to clear the field with null. Since this PR changes the API contract, the generated SDK should be regenerated instead of relying on a local cast here.

@sainzs
Copy link
Copy Markdown
Author

sainzs commented Mar 28, 2026

Addressed the SDK/openapi follow-up from the bot review.

Updated the public contract so default_agent is nullable in:

  • packages/sdk/js/src/v2/gen/types.gen.ts
  • packages/sdk/openapi.json

That lets KiloProvider use the regenerated SDK type directly again instead of the temporary local cast.

Verification after the contract update:

  • bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts
  • bun run --cwd packages/kilo-vscode check-types
  • bun run --cwd packages/opencode typecheck
  • bun run --cwd packages/sdk/js typecheck
  • git push (pre-push hook ran bun turbo typecheck and passed)

@sainzs
Copy link
Copy Markdown
Author

sainzs commented Mar 28, 2026

Parallel quality pass completed.

I ran three independent sub-agent audits on the current branch:

  • behavioral/regression review: no issues found
  • schema/SDK contract audit: no remaining mismatches found after the SDK/openapi sync
  • test-gap audit: surfaced a few worthwhile coverage gaps, now addressed

Added follow-up regression coverage for:

  • clearing default_agent when the current default is hidden
  • clearing default_agent when the current default is disabled
  • the saved round-trip where default_agent: null is confirmed by a server response without default_agent

Verification after the hardening pass:

  • bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts
  • git push (pre-push hook ran bun turbo typecheck and passed)

@sainzs
Copy link
Copy Markdown
Author

sainzs commented Mar 28, 2026

Follow-up quality pass is pushed in 2c547be05.

Addressed from the audit pass:

  • preserved clear semantics for agent description, prompt, model, temperature, top_p, and steps by using nullable delete sentinels end-to-end
  • exposed description editing for native agents so the new clear path is reachable from the UI
  • kept numeric clears sticky by ignoring transient invalid non-empty input instead of overwriting a pending null delete sentinel
  • split config save outcomes into configUpdated (success with merged refresh), configSaved (write succeeded, merged refresh pending), and configSaveFailed (write failed) so failed saves keep the draft retryable without misclassifying post-write refresh failures
  • gated repeated saveConfig() calls while a save is already in flight

Verification run on this follow-up:

  • bun test --cwd packages/kilo-vscode tests/unit/config-utils.test.ts tests/unit/agent-behaviour-patches.test.ts
  • bun run --cwd packages/kilo-vscode check-types
  • bun run --cwd packages/opencode typecheck
  • bun turbo typecheck via pre-push hook

Sub-agent findings that surfaced real issues were addressed before push; the remaining contract review came back clean.

.number()
.int()
.positive()
.nullable()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: steps: null is still dropped before the config is applied

Making the schema nullable here updates the contract, but the Agent transform a few lines below still does const steps = agent.steps ?? agent.maxSteps. When the webview sends steps: null to clear an override, that expression turns it into undefined, so the delete sentinel never reaches config merging and the old step limit remains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant