mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
stack/721-base-legacy
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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) |
||
|
|
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 |
||
|
|
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. |