Repository navigation
Conversation
…place) Baseline test suite for specs/PR-3c.md, written before the implementation. Adds compile-only todo!() stubs for the interface contract's not-yet-existing items in provider/vscode/settings.rs (SettingsConflict, SettingsState with its v2 camelCase/deny_unknown_fields serde shape and explicit-null-preserving override_before, Removal, edit_settings, remove_settings, read_state) plus the jsonc-parser dependency (AC10, cst+serde_json features). SettingsState's serde impl and the some_or_explicit_null deserializer are real, not stubs, so fixtures that round-trip it are meaningful now. settings_tests.rs adds the pure-function baseline (fresh apply, byte-identical comment/CRLF/trailing-comma preservation, override replaced in place vs. appended, ignore-array creation/extension, hand-edited vs. user-held override restore, null override, conflicts, v1-sidecar upgrade) plus plan-level tests for the mode-tightening and no-pairing-in-sidecar criteria the old json_merge sidecar can't meet. Rewritten against the v2 sidecar (ledger, AC9): hand_edited_override_is_replaced_on_reapply_and_not_restored_on_removal and user_override_from_before_the_first_apply_survives_a_hand_edit_and_returns_on_removal now call edit_settings/remove_settings directly instead of going through plan/json_merge's removed owned_keys. comment_and_trailing_comma_conflicts is renamed comments_and_trailing_commas_are_preserved and now expects success. managed_settings_carries_both_keys is dropped (ledger, AC9): nothing but this test used managed_settings once plan is rewired to edit_settings/remove_settings. `cargo test -p agentdesktop-agent --lib --no-run` succeeds. Running the suite: 26 fail (19 on a todo!() stub, 7 on the old json_merge implementation not yet matching the new criteria: comment/mode-tightening/sidecar-redaction/no-sidecar removal on a commented file), 17 pass unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rmatting
VS Code's user settings.json is JSONC and commonly commented. Under
`copilotChat: githubModels` the daemon used to rewrite the whole file
through json_merge (keys sorted, formatting lost) and refused a file with
comments or trailing commas as a conflict, which blocked every managed file
on the device.
The file is now edited in place with jsonc-parser's lossless syntax tree:
only `github.copilot.advanced.debug.overrideCapiUrl` and the two
`settingsSync.ignoredSettings` entries change. New properties are inserted
at the top of the object, so no line the user wrote changes (appending after
the last property would add a comma to it); entries added to an existing
ignore array are appended. Comments, trailing commas, key order, indentation
and line endings stay. Removal takes out exactly what the apply added and
restores the user's own override in place (a same-line comment kept, an
explicit null kept as null); a file the daemon only added keys to returns to
its earlier bytes; a created file is deleted when only `{}` remains, kept
when a comment remains.
Parsing accepts VS Code's JSONC only (comments, trailing commas); a missing
comma, single quotes and other JSON5 forms are conflicts, as are a non-object
root, a non-array `settingsSync.ignoredSettings` and a managed key that
appears twice. A conflict logs the reason with the path.
An unchanged re-apply writes nothing, except that a file looser than 0600
(VS Code saves it 664) is rewritten with the same bytes at 0600; the sidecar
is written only when its content changes.
The sidecar (`.settings.json.agentdesktop`, v2) no longer holds a copy of the
user's settings: `{version: 2, created, overrideBefore?, addedIgnored,
ignoredCreated}`. It holds no URL or pairing. The earlier whole-document
sidecar is read once and upgraded; an unreadable sidecar is a warning and the
sidecar-less rules apply (an override equal to this daemon's own URL is never
recorded as the user's value; an ignore list holding only our two entries
counts as created by the daemon).
json_merge: `MergeOptions::owned_keys` (only this file used it) is removed;
the other callers are unchanged. New workspace dependency
`jsonc-parser 0.34` (MIT, features `cst` and `serde_json`); `json5` stays for
read-only discovery.
README: the in-place edit, what counts as a conflict and how to fix it, the
tightening of an unchanged looser file.
Tests: baseline b31cef8 (settings module; three test changes in the ledger,
none weakening a criterion). The Docker test starts from a commented file
with a trailing comma and asserts the comment survives apply and removal.
Agent lib: 246 tests; the VS Code Docker tests pass (3/3).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e order, BOM, recreated list/file) Review round 1 on pr3c-v1 (conformance, adversarial, maintainer; no blocker or major). - An override pointing at any agentdesktop loopback CAPI route (`http://<loopback>:<port>/vscode-copilot-capi/<pairing>`) is the daemon's, never the user's value: without a sidecar, a URL an earlier listen address or pairing left behind was recorded as `overrideBefore` (putting a pairing into the sidecar and writing a dead URL back on removal); sidecar-less removal now takes such a URL out too (adversarial m2). - The sidecar is written before `settings.json`, so an apply that stops between the two writes cannot lose the record of the user's own override (adversarial m3). - A leading UTF-8 byte-order mark is kept and edited around instead of being a conflict (adversarial m4; Windows PowerShell 5.1 writes one). - A file or ignore list the user deleted and a re-apply recreated counts as the daemon's, so removal takes it out again (adversarial n1, n2; maintainer n6). - Removal of an unparseable file without a sidecar logs its debug line again; a removal with nothing to change records `unchanged` like the other managed files (maintainer m2, m3; conformance). - Clean-ups: removal tuple simplified, unused binding gone, sidecar mode uses the file-mode constant, the Docker helper parses with VS Code's JSONC options, stale process comments and spec references removed from the settings tests (four comment-only baseline test changes, ledger). - README: "adds" instead of "merges"; the byte-order mark; one exception to "returns to its earlier content": a file written on a single line is expanded to one property per line when keys are added and stays expanded (VS Code writes one property per line). Corrections to the pr3c-v1 message: "no line the user wrote changes" holds for files with one property per line, not for a single-line object; only an unparseable sidecar is a warning, a sidecar the daemon cannot read at all (permissions) is an error like any unreadable managed file. Tests by author: baseline b31cef8 (test author) 45 tests in settings_tests.rs, 7 changed in the ledger (none weakening a criterion); writer 4 new tests (earlier daemon URL, byte-order mark, recreated list and file, duplicated ignore list). Agent lib: 250 tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…f ours was there
Delta review 1 on pr3c-v2 (one major, minors).
The removal path recorded `unchanged` for every existing settings.json,
so a user daemon without `programs.vscode` counted "1 unchanged" in every
plan summary, and it recorded nothing when a sidecar existed but the file
was gone. It now matches the other managed files: `unchanged` only when a
sidecar existed (the file unchanged, or the file absent; the sidecar is
removed in both cases), nothing otherwise. The pr3c-v2 message's "records
`unchanged` like the other managed files" was true only after this change.
Writer test `removal_records_unchanged_only_when_something_of_ours_was_there`.
Doc comments updated for the loopback-URL rule and for which sidecar
failures are warnings (cannot be parsed) and which are errors (cannot be
read); a leftover block removed; two stale test section comments reworded.
Left as is, with the reason: the "Added with the implementation (not part of
the spec-derived baseline)" labels on writer tests match the earlier PRs of
the series; a comment-only settings.json gains `{}` on apply (VS Code never
writes such a file, and the text before `{}` is kept); the baseline commit's
stub failures are intentional in the stacked review history, and whether the
series is squashed is decided with the integration PR.
Agent lib: 251 tests (writer +1).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… BOM on deletion)
The module doc states the single-line exception and the byte-order mark;
a non-UTF-8 file without a sidecar logs the same debug line as an
unparseable one; a created file emptied to `{}` with a byte-order mark is
deleted like one without. No behaviour change otherwise; agent lib 251
tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
Superseded: landed upstream in agentdesktop-dev#135 (in-place JSONC edit of VS Code settings.json). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on this fork so the diff is this PR only (base:
copilot/pr3b-vscode-github-models). Seventh change in the series; James gets one integration PR fromcopilot/integrationwhen the series is ready.Why. Under
copilotChat: githubModelsthe daemon rewrote the whole usersettings.json(keys sorted, formatting lost) and treated a file with comments or trailing commas as a conflict, which blocks every managed file on the device. VS Code's settings file is JSONC and commonly commented, so on real developer machines that was the common case. The project's stated behaviour is to merge managed values while preserving unrelated user settings; for this file that means leaving everything the user wrote as it is.What it does. The file is edited in place with
jsonc-parser's lossless syntax tree (MIT, dprint; featurescst,serde_json; workspace dependency).github.copilot.advanced.debug.overrideCapiUrland the twosettingsSync.ignoredSettingsentries change. New properties are inserted at the top of the object, so no line the user wrote changes; entries added to an existing ignore list are appended. Comments, trailing commas, key order, indentation, line endings and a leading byte-order mark stay. One exception: a single-line object is expanded to one property per line (VS Code always writes one per line).cmpidentical). A user's own override is restored in place, keeping a same-line comment; an explicitnullstaysnull.settings.json..settings.json.agentdesktopno longer copies the user's settings:{version: 2, created, overrideBefore?, addedIgnored, ignoredCreated}, no URL, no pairing. A PR 3b sidecar is read once and upgraded. An override pointing at any agentdesktop loopback CAPI route (for example one left by an earlier pairing) is never recorded as the user's value, and sidecar-less removal takes it out.json_merge'sowned_keys(only this file used it) is removed; the other callers are unchanged. Removal recordsunchangedonly when a sidecar existed, as for the other managed files.Process. Spec reviewed (Opus, 16 findings taken) before the test author; baseline tests first (b31cef8, 45 tests in
settings_tests.rs); a spike confirmed the crate's round-trip behaviour before the spec froze. Three review lenses on two models: no blocker or major; the adversarial minors (a stale loopback URL recorded as the user's value, write order, byte-order mark, recreated list or file) and the doc corrections went into the second commit. Delta 1 found one major (the removal path recordedunchangedfor every existing file, so users without the program saw "1 unchanged"), fixed in the third commit; delta 2 passed with nits, fixed in the fourth; delta 3 was a clean pass (stop rule: two consecutive passes without a blocker or major) Test guard: 7 baseline tests changed, all listed (comments, one assertion loosened from "rewritten" to "not deleted", one search target), none weakening a criterion; writer added 5 tests. Gate: 251 agent + 18 core tests, clippy, fmt, schema, frontend.Lab (Linux VM, user mode, VS Code 1.139, Solo Enterprise for AgentGateway standalone, controller-pushed config). First run by an independent agent on the second commit, every item passed: a PR 3b sidecar upgraded in place with the user's override kept and restored; a commented fixture with a block comment, same-line comments and a trailing comma got exactly two added lines (
diff2a3,4), mode 644 to 600; a chat turn answered through the gateway (llm.client="vscode-copilot", 200) with the pairing in no log; switching to own models restored the fixture byte for byte; after VS Code saved the file 664 with a new inode, a daemon restart tightened it to 600 without changing a byte; a file with a syntax error stops the daemon at startup with the conflict message and stays unchanged. Repeat on the third commit withlab/checks/pr3c-settings.sh: 17 of 17. Docker suite for VS Code: 3 of 3, starting from a commented file.Not covered: a WebSocket
/responsesturn in this run (the GitHub models are policy-restricted on the lab seat, the turn ran on Auto; the tunnel itself is unchanged since PR 3b); macOS and Windows beyond unit tests.Follow-ups (local issue tracker): one JSONC parser for discovery too (
json5stays for now); an in-place mode for the other managed files if needed; the Claude Code sidecar rewritten on every delivery (issue 006).🤖 Generated with Claude Code