Skip to content

fix(settings): keep agent environment values with spaces when the field loses focus - #31772

Merged
AmoabaKelvin merged 3 commits into
mainfrom
keep-agent-env-quoted-values
Oct 22, 2026
Merged

AmoabaKelvin merged 3 commits into
mainfrom
keep-agent-env-quoted-values

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​37 $\color{#cf222e}{\Huge{\mathbf{−}}}$​19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​18
Prod 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​58 $\color{#cf222e}{\Huge{\mathbf{−}}}$​67 $\color{#cf222e}{\Huge{\mathbf{−}}}$​9

ELI5

The Environment box in Settings → Agents showed A="two words" as A=two words. Clicking into the box and back out read that text back and saved A=two, so part of a saved value was lost without any edit. The box now shows quotes where a value needs them, reads them back the same way, and does not save at all when you only click in and out.

What Changed

  • stringifyAgentDefaultEnvDraft quotes a name or value when it is empty or contains whitespace or a quote (A='two words' B=keep). Plain values stay bare.
  • parseAgentDefaultEnvDraft now uses the shared tokenizer that custom commands already use (tokenizeCustomCommandTemplate, 'literal' backslash mode), in place of its own whitespace splitter. Quotes group words, and backslashes stay literal, so TOOL=C:\tools\bin typed without quotes still saves as typed.
  • A draft with an unclosed quote is not saved. The field shows "Close the quote to save the environment." in the same slot as the existing too-large error.
  • Blur and Enter skip the save when the draft equals the saved text.
  • The field's custom destructive classes are gone because Input already styles aria-invalid.

Why

The owner of the bug is the format/parse pair in agent-default-env-draft.ts. The formatter dropped quoting that the parser never understood, so the two were not inverses and every blur re-saved a cut value. With one grammar on both sides, any saved record survives format → parse unchanged. Skipping unchanged blurs also stops a settings write on every click through the field.

I chose literal backslashes over POSIX escapes because unquoted Windows paths are the common case in this field, and the parser already accepted them that way. The quoting reuses quotePosixShellDoubleQuotedApostrophe (no backslashes needed). Only the settings text changes; the stored agentDefaultEnv record and the wire format stay the same, so mixed client/server versions and remote hosts are unaffected. The remote host editor uses one field per variable and mobile never edits this text.

Linked Issue

No GitHub issue. Linear: STA-11496 (found in review of #31315).

Visual Proof

Saved env {"A":"two words","B":"keep"}. Click into Environment, click out without editing. The caption shows the saved record read from the settings store.

BEFORE (main): saved value becomes {"A":"two","B":"keep"}.

agent-env-before.mp4

AFTER (this branch): the field shows A='two words' B=keep and the saved value is unchanged.

agent-env-after.mp4

AFTER, unclosed quote: not saved, inline error.

agent-env-unclosed-quote.png

Testing

  • agent-default-env-draft.test.ts: format → parse returns the same record for spaces, apostrophes, double quotes, both quotes, =, empty strings, tabs and Windows paths. It fails on main. Also covers bare backslashes and the unclosed-quote error.

  • Hidden dev builds of main and this branch on macOS, driven over CDP with real clicks, as in the videos.

  • pnpm run check:code-quality:changed, pnpm run check:react-doctor:changed, i18n tests, verify:localization-catalogs, verify:localization-extraction, verify:localization-coverage.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Brennan's #31315 also edits AgentDefaultEnvInput. The overlap here is the error-state rename and the skip-unchanged guard, so the conflict should be small.

Values with spaces were shown unquoted and re-split on blur, cutting A="two words" to A=two.
The localization audit reads an `error` property as UI copy; these are codes.
@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 22, 2026 22:00
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6acc903b-d9dd-4e7f-8dc0-be505d87c193
📥 Commits

Reviewing files that changed from the base of the PR and between a8dff64 and 5cab361.

📒 Files selected for processing: 10
  • src/renderer/src/components/settings/AgentLaunchDefaultsEditor.tsx
  • src/renderer/src/components/settings/agent-default-env-draft.test.ts
  • src/renderer/src/components/settings/agent-default-env-draft.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/zh.json
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AmoabaKelvin
AmoabaKelvin merged commit 494ecd2 into main Oct 22, 2026
58 checks passed
@AmoabaKelvin
AmoabaKelvin deleted the keep-agent-env-quoted-values branch October 22, 2026 22:35
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