Files
filip131311 81de9f66a6 fix(config): say what a rejected value should have been, and prune the group it left behind (#650)
Fixes #643.

## Before / after

```
$ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project
Error: Invalid value for config key "ios.additionalDeviceSets".        # ← the whole message
```
```
$ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project
Error: Invalid value for config key "ios.additionalDeviceSets": expected an array of strings.
Did you mean: argent config set ios.additionalDeviceSets '["~/Library/Developer/Radon/Devices"]' --scope project
Run `argent config list` to see each key's expected value.
```
```
$ argent config unset ios.additionalDeviceSets --scope project
$ cat .argent/config.json
{ "ios": {} }        →        {}
```

## Symptom 1 — the message

The issue names three gaps: no expected type, no example, no pointer to
`config list`. All three are
closed, plus one the issue didn't ask for and I think is the actual fix.

**The suggestion carries the user's own value.** A generic `Example: …
'["~/DeviceSets/ci"]'` would
hand someone a path they have never heard of and expect them to spot the
difference. Where the
correction is mechanical — a single item offered to a list-valued key —
the error offers back what
they typed, wrapped, shell-quoted, and **on the scope they asked for**.
Pasting it works; verified
live that it lands in project scope, not global.

Nothing is coerced. The issue floated accepting a bare string and
storing it wrapped; I didn't,
because `config set` persists, gets committed, and is read by other
machines, so a value silently
stored in a shape the user didn't type is expensive to debug and
`coerceCliValue` already has one
silent fallback. Showing the corrected command costs the user one
keystroke and keeps the store
honest. If a reviewer prefers coercion, this is the smaller change to
build on.

**The expected shape comes from the validator, not a per-key string** —
`asStringArray` *is* "an
array of strings", so swapping a key's `parse` swaps its description
with it and the two cannot
drift. A bespoke validator can override via a new optional `expected`,
and degrades to today's exact
message if it says nothing.

**`config list` states it too.** The issue notes `list` doesn't close
the gap either — its
description describes the *elements* but never says the value is a list.
It now shows
`value: an array of strings, e.g. ["~/DeviceSets/ci"]`, and `list
--json` carries both fields.

`telemetry.enabled` deliberately gets no example: `setConfigValue`
refuses it before validating
(it's managed by `argent telemetry`), so a "here's how to set it" hint
would point at a command you
may not run. A schema guard test keeps that honest as keys are added —
and asserts every example
**actually round-trips through its own validator**, so an example can
never ship that reproduces the
error it exists to fix.

## Symptom 2 — the empty group

Deleting a key now removes any container the delete emptied, stopping at
the first ancestor that
still holds something.

**This is file hygiene, not a behaviour change.** I traced every raw
reader of the document —
`readConfigObject`, and telemetry's `consent.ts` and `notice.ts`, which
each do their own read — and
an empty parent is already indistinguishable from an absent one:
`getAtPath` returns `undefined`
either way, before `parse`, so the merge layer never sees a container at
all. The only observable
difference is the bytes on disk, which is exactly the complaint.

Guards worth noting:

- **The root always survives** — an emptied config is `{}`, never a
deleted file. `.argent/` is
itself a project-root marker, so removing the file wouldn't restore the
prior state anyway, and
`updateConfig` is a shared publish path with a lock/rename protocol that
has no defined meaning for
  file removal.
- **A no-op unset still doesn't touch the file.** That fast path exists
so an unset that removes
nothing can't materialise a project config and dirty git status; pruning
lives inside the mutate
callback, which the fast path returns before reaching. So a group
someone deliberately left empty
by hand is never tidied away — pinned by a test asserting the file stays
byte-identical.
- Siblings at any level stop the unwind, including an empty array value,
which is preserved.

**Two existing tests asserted the old behaviour and now assert the new
one** —
`config-access.test.ts:47` and `config.test.ts:109` (the latter is
`clearRememberedAgent`'s
sibling-preservation test, so it doubles as proof siblings survive).

## Known same-class case, not fixed here

`resetFirstRunNotice` (`packages/telemetry/src/notice.ts`) does its own
inline delete and still
leaves `{"notices":{}}`. It doesn't go through `deleteAtPath`, so it
doesn't inherit this. It's a
one-line change to route it through the same function — happy to do it
here or as a follow-up, but
it pulls the telemetry package into a config PR so I left it out by
default.

## Verification

Live against the built CLI: the reported case; pasting the suggestion
back and confirming it lands in
**project** scope; unset leaving `{}`; a sibling key surviving; a
non-array key correctly getting the
generic example and *no* bogus "Did you mean"; `config list` output;
`telemetry.enabled` still
delegating unchanged.

configuration-core 103 passed, argent-cli 285 passed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-08-03 14:56:40 +02:00

118 lines
3.7 KiB
TypeScript

import { describe, it, expect, beforeEach, afterEach } from "vitest";
import * as fs from "node:fs";
import * as path from "node:path";
import * as os from "node:os";
import { argentHomeDir, configFilePath } from "../src/paths.js";
import { readConfigObject, updateConfig } from "../src/config.js";
import {
getRememberedAgent,
setRememberedAgent,
clearRememberedAgent,
} from "../src/config-access.js";
// Redirect the `~/.argent` home into a tmp dir by mutating process.env.HOME
// (consumed by os.homedir() via argentHomeDir).
let tmpHome: string;
let originalHome: string | undefined;
let originalUserProfile: string | undefined;
beforeEach(() => {
tmpHome = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-config-home-")));
originalHome = process.env.HOME;
originalUserProfile = process.env.USERPROFILE;
process.env.HOME = tmpHome;
process.env.USERPROFILE = tmpHome;
});
afterEach(() => {
if (originalHome === undefined) delete process.env.HOME;
else process.env.HOME = originalHome;
if (originalUserProfile === undefined) delete process.env.USERPROFILE;
else process.env.USERPROFILE = originalUserProfile;
fs.rmSync(tmpHome, { recursive: true, force: true });
});
function readConfigFile(): unknown {
return JSON.parse(fs.readFileSync(configFilePath(), "utf8"));
}
describe("paths", () => {
it("locates config.json under the argent home", () => {
expect(configFilePath()).toBe(path.join(argentHomeDir(), "config.json"));
expect(argentHomeDir()).toBe(path.join(tmpHome, ".argent"));
});
});
describe("readConfigObject / updateConfig", () => {
it("returns an empty object when the file is missing", () => {
expect(readConfigObject()).toEqual({});
});
it("returns an empty object when the file is malformed", () => {
fs.mkdirSync(argentHomeDir(), { recursive: true });
fs.writeFileSync(configFilePath(), "not json{", "utf8");
expect(readConfigObject()).toEqual({});
});
it("creates and persists a new key", () => {
updateConfig((c) => {
c.telemetry = { enabled: false };
});
expect(readConfigFile()).toEqual({ telemetry: { enabled: false } });
});
it("merges rather than clobbering keys it does not touch", () => {
updateConfig((c) => {
c.telemetry = { enabled: false };
});
updateConfig((c) => {
c.notices = { firstRun: true };
});
expect(readConfigFile()).toEqual({
telemetry: { enabled: false },
notices: { firstRun: true },
});
});
});
describe("remembered agent (lens config)", () => {
it("returns null when nothing is stored", () => {
expect(getRememberedAgent()).toBeNull();
});
it("persists and reads back the chosen agent", () => {
setRememberedAgent("claude");
expect(getRememberedAgent()).toBe("claude");
expect(readConfigFile()).toEqual({ lens: { agent: "claude" } });
});
it("overwrites a previously remembered agent", () => {
setRememberedAgent("claude");
setRememberedAgent("codex");
expect(getRememberedAgent()).toBe("codex");
});
it("treats a blank stored agent as none", () => {
updateConfig((c) => {
c.lens = { agent: " " };
});
expect(getRememberedAgent()).toBeNull();
});
it("clears the remembered agent without dropping sibling keys", () => {
updateConfig((c) => {
c.telemetry = { enabled: true };
});
setRememberedAgent("claude");
clearRememberedAgent();
expect(getRememberedAgent()).toBeNull();
// Siblings survive; the emptied `lens` container does not.
expect(readConfigFile()).toEqual({ telemetry: { enabled: true } });
});
it("clearing when nothing is stored is a no-op that doesn't throw", () => {
expect(() => clearRememberedAgent()).not.toThrow();
expect(getRememberedAgent()).toBeNull();
});
});