Commit Graph

3 Commits

Author SHA1 Message Date
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
filip131311 cccda92d15 feat(config): ios.additionalDeviceSets — additive array of extra CoreSimulator device sets (#542)
## Summary

**Stacked on #534.** Adds the first *additive* configuration entry on
the new schema — `ios.additionalDeviceSets`, a list of extra
CoreSimulator device-set directories — delivering the config side of the
#401 discussion on the #534 machinery.

### The schema entry

One registry entry in `config-schema.ts`:

- key `ios.additionalDeviceSets`, scopes `project` + `global`, `parse:
asStringArray`, **`merge: "union"`**

Unlike `lens.agent` (project *shadows* global), this is additive: the
effective value is the **union of both scopes** — the global baseline
first, the project's extras appended, deduplicated. A repo can commit
the device sets its tooling needs to `.argent/config.json` and they
*extend* (never replace) the user's global list. `argent config
list/get/set/unset` handle the array key with zero command changes —
arrays already round-trip through the JSON value coercion.

### The runtime reader

`getAdditionalIosDeviceSets(options): string[]` in `config-access.ts` —
the surface iOS tooling should call. It reads each scope separately so
paths normalize against the base the author meant:

- `~` / `~/…` expands to the home directory (honoring the sandbox
`homeDir` override)
- relative **project** entries resolve against the **project root**,
relative **global** entries against **home**
- output order matches the `union` preset (global first), with
**post-normalization** duplicates dropped — `~/DeviceSets/ci` in global
and its absolute spelling in the project file collapse to one entry
- empty array when unset

Support refactor: `resolveHomeDir()` extracted in `paths.ts` (the
HOME/USERPROFILE + override logic previously inlined in
`argentHomeDir`/`configDir`).

### Out of scope (follow-up)

Actually pointing simctl at the extra sets. There is no central simctl
wrapper today (~25 call sites build `xcrun simctl …` argv inline, and
`ax-prefs.ts` hardcodes the default set's `CoreSimulator/Devices` path),
so consuming this reader in `list-devices`/boot/etc. is its own change —
the runtime-read-off-disk design here means that consumer needs no
CLI/env threading, matching how #534's description framed the split.

## Testing

- `configuration-core`: 5 new tests (82 total, green) — additive union
across scopes + per-scope file isolation, non-array rejection + entry
normalization, reader `~`/relative resolution per scope,
post-normalization dedup, empty-when-unset
- `argent-cli`: new real-filesystem CLI round-trip (274 total, green) —
`config set` an array at each scope, effective `get` shows the union
while `get --scope` shows only that scope's entries
- `tsc` build + `typecheck:tests` + eslint clean on touched packages
2026-07-31 10:27:25 +02:00
filip131311 8d2a499eaf Project-scope configuration with schema-driven merge + argent config (#534)
## Summary

Adds a general, **scoped** configuration system on top of the shared
`config.json`, plus a universal `argent config` command to manage it.
Motivated by the discussion on #401 (iOS device set): config that needs
to work naturally with the MCP server should be read off disk at runtime
like telemetry consent is — not threaded through CLI flags/env and a
kill-and-respawn dance. This lands the reusable machinery; the
device-set becomes a one-line schema entry + a runtime reader on top of
it.

Four pieces, matching the requirements:

### 1. Project-scope config files
`config.json` now resolves at two scopes:
- `~/.argent/config.json` (**global**, default)
- `<project-root>/.argent/config.json` (**project**, resolved by the
same `.argent`/`.git`/`package.json` marker walk as feature flags)

`configFilePath` / `readConfigObject` / `updateConfig` take an optional
`scope` (default `global`), so **every existing caller is unchanged**
(telemetry, notices, lens). Per-scope lock + atomic write are preserved.

### 2. Schema-driven merged reads
`getConfigValue(def)` reads each scope the value's schema entry allows,
merges them under the entry's policy, and falls back to the entry's
default. This is the reader consumers should call instead of poking at a
single global file. The existing **lens** getters (`getRememberedAgent`
etc.) are migrated onto it as the first live consumer — a repo can now
pin its lens agent, falling back to the user's global choice.

### 3. A registration file for configurations (`config-schema.ts`)
Adding a configuration is a one-liner: `key`, `description`, allowed
`scopes`, a `parse` validator, and a `merge` policy. Merge presets:
- `prioritize-local` (project wins) · `prioritize-global` (global wins)
- `prioritize-restrictive` (booleans/numbers — opt-out/stricter wins)
- `union` / `intersection` (arrays)
- …plus an escape hatch: pass your **own** `(inputs) => value` function
for a bespoke rule.

### 4. `argent config` — one universal command
`argent config <list | get | set | unset>` with `--scope global|project`
and `--json`. New configs are added as **schema entries**, not new
top-level `argent` commands, to avoid bloat.

**Telemetry stays backward-compatible** on its dedicated `argent
telemetry` command (which also drains/resets the live client). It's
surfaced *read-only* under `argent config` via a `manageCommand` guard —
so `argent config set telemetry.enabled` refuses and points at `argent
telemetry`, and a committed project file can never re-enable a global
opt-out (`prioritize-restrictive`, global-only).

## Testing

- **Unit / integration**: 71 configuration-core tests (merge presets +
custom fn, dotted-path + prototype-pollution guards, scope merge,
scope/validation/manage-command errors, `listConfig`, `coerceCliValue`)
and 11 real-filesystem CLI tests driving the actual `config()` entry
point across sandboxed HOME + project cwd. Full suites green:
configuration-core 71, argent-cli 268, telemetry 281, argent 60.
- **Live, on the built binary** (`node packages/argent/dist/cli.js …`):
set global + project (from a deep subdir) → effective get returns the
project value; per-scope gets return each file; both files land in the
right place; `argent telemetry disable/enable` writes `config.json` and
`argent config get telemetry.enabled` reflects it read-only; unknown-key
/ invalid-value / bad-scope errors exit non-zero with hints.
- `tsc --build` across the workspace, prettier, and eslint
(`--max-warnings 0`) all clean.

## Notes / follow-ups
- Config-file reads are not yet fingerprint-cached the way telemetry's
consent read is; a long-lived tool-server re-reads on each
`getConfigValue`. Fine for CLI/low-frequency use; a hot runtime consumer
(e.g. a future device-set reader) may want the same mtime/inode cache.
2026-07-22 12:47:42 +02:00