mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
flow-visual-settle
8 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) |
||
|
filip131311
|
fc09612f30 |
feat(config): recordings.directory — configurable save location for screen recordings (#601)
## Summary Adds a `recordings.directory` configuration entry (built on the #534 config system) that lets users choose where finished screen recordings (mp4) are saved, instead of the fixed `<project>/.argent/recordings/`. - **Schema entry** (`configuration-core`): scopes `project` + `global`, `prioritize-local` merge (project wins when both are set). Picked up automatically by `argent config set/get/unset/list`. - **Value semantics**: absolute path, `~`-prefixed (expands to home), or relative — relative anchors at the project root (home dir when not in a project). - **Client-side wiring** (`argent-tools-client` `durableSaveTarget`): the wire `saveDir` hint stays `.argent/recordings` (wire contract unchanged → old/new client-server interop). Past the existing client allowlist, the recordings hint is remapped to the configured directory. ## Security model unchanged The allowlist exists because `saveDir` arrives from a possibly-hostile remote tool-server. That gate still runs first and is unchanged: a server can only *select* the sanctioned recordings category, never name a destination. The configured directory comes from the client's own trusted config files. Exclusive never-overwrite writes (`name (2).mp4`), the durable download size caps, and the symlink real-path guard all remain; for a configured dir the realpath check degenerates to "the dir resolves to itself" (a user-chosen symlink is legitimate, matching how the default base may be symlinked). A test asserts hostile wire values (`.git`, `..`, absolute) stay rejected even with config set. ## `argent link` interaction Materialization always runs on the **client** (co-located → copy from `hostPath`; remote → download over `/artifacts/:id`), and the config is read there at materialize time — so with a remote tool-server the mp4 lands on the client host per the *client's* config, on both paths. ## Testing - New unit tests: 10 in `argent-tools-client` (absolute/`~`/relative resolution, local-vs-global precedence, both materialization paths, hostile wire values with config set) + 4 in `configuration-core` (scope writes, trim, precedence, validation). - Suites green: tools-client 188, configuration-core 87, tool-server screen-recording 62, CLI config 16; full workspace vitest 4727 pass (3 pre-existing `scripts/*.test.mjs` node:test files don't parse under root vitest — unrelated). - **Live E2E** on an iPhone 15 Pro simulator via the branch tool-server: real recording captured, then materialized through the branch client with configs written by the real `argent config` CLI in a sandboxed HOME/project. 7 scenarios green: default location, global-only (`~` path), project-only (relative), both-set → project wins (co-located **and** remote download with Bearer auth), unset fallbacks, `argent config list` rendering. Remote-downloaded mp4 ffprobe-verified (byte-exact size, 4.17 s duration); name-collision suffixing observed live. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
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 |
||
|
Ignacy Łątka
|
2251c7cf46 |
feat(screen-recording): screen-recording-start/stop tools with per-call stop reminders (#517)
## What Adds a `screen-recording-start` / `screen-recording-stop` tool pair (naming follows `native-profiler-start/stop`) that records the device screen to an mp4 and returns it as a downloadable artifact. Capture runs through **simulator-server** — the same backend `screenshot` and every interaction tool already drive. simulator-server publishes the device screen as an MJPEG stream (`/stream.mjpeg`), so recording needs no platform-specific recorder: the tool-server subscribes to that stream, paces the frames onto a fixed 30 fps timeline, and pipes them into a single ffmpeg process that encodes — and watermarks — straight to the final mp4. The stream is multi-subscriber, so a preview window can stay open on the same device while recording. - **One path for every device simulator-server drives** — iOS simulators, Android emulators, and physical Android devices. - **Native resolution, constant 30 fps** — h264, `+faststart`, no post-processing pass. - Session state lives in a per-device `ScreenRecordingSession` blueprint (mirrors the native-profiler session): time-limit cap with auto-finalize, unexpected-exit recovery (a capture that ended on its own is still retrievable via stop), always-startable state reset on stop, and dispose-on-shutdown cleanup through the registry. ## Why the frames are paced host-side A device only emits a frame when the screen **changes**. Pointing ffmpeg straight at the stream therefore produces a video whose length is the sum of the moments something moved — a still screen collapses to a fraction of a second — and an ffmpeg blocked on a silent socket does not answer a stop signal promptly either (measured: 12 s late). Re-emitting the last frame on a wall-clock schedule fixes both: **video duration always equals real elapsed time**, and stop is a stdin EOF that finalizes in ~60 ms. Duplicate frames cost almost nothing once encoded (a 10 s recording of a static screen is ~1 MB). ## Finishing pass folded into capture The Argent watermark (logo + `By @swmansion`, bottom-left, sized relative to the frame) is stamped by the same ffmpeg invocation that encodes the capture, so there is no second encode and no temp intermediate. Its colour is chosen **per pixel** from the background luma underneath it — the white logo over dark areas, a runtime-derived near-black copy over light areas — so it keeps maximum contrast against whatever is behind it. It sits at 20 % opacity and is opt-out via `argent disable video-watermark` (read when the recording starts). Frame geometry comes from the first JPEG's SOF header, so no `ffprobe` is needed. ffmpeg is now the recorder rather than an optional finisher, so a missing ffmpeg fails `screen-recording-start` up front with an install hint instead of silently degrading the output at stop time. ## Reminder plumbing While a recording is running — or ended on its own but was never retrieved — **every** tool result carries a `note` telling the agent the recording is still going and how to stop it. It rides the same `note` field as the update reminder but is per-call and unsuppressed; both notes compose when they coincide. The note text switches to "already ended, retrieve the file" once the cap fires or the capture dies. ## Skill New `argent-screen-recording` skill (+ routing stanza in `rules/argent.md`): documents the start → interact → stop workflow and instructs the agent to schedule its own reminder/wakeup for the expected end of the recording immediately after starting one (harness reminder tool if available, otherwise a background `sleep` whose completion notification pulls it back). Scores 10/10 on `scripts/grade-skills.mjs`. ## Verification - Unit: 45 tests across `screen-recording.test.ts`, `mjpeg-stream.test.ts`, `watermark.test.ts`, `http-recording-note.test.ts`; full tool-server suite **2808 tests / 280 files green**; `tsc --noEmit -p tsconfig.test.json`, root `tsc --build`, eslint, prettier all clean. - E2E on a live tool-server (`POST /tools/...`), driving real interactions between start and stop: - **iOS sim** (iPhone 16 Pro Max, iOS 18.5): 1320×2868 h264, `r_frame_rate=30/1`, 338 frames, container duration 11.27 s vs reported `durationMs` 11314 — **within 50 ms of wall clock**. Extracted frames show real screen content (mid app-switcher animation) with the watermark rendered light-over-dark. - **Android emulator** (Pixel 3a, API 34): 1080×2220 h264, 30 fps, 353 frames, duration 11.77 s vs `durationMs` 11793 — within 30 ms. 512 KB for a mostly-static home screen. - **Static screen** (no interaction at all, 10 s): 300 frames, duration exactly 10.000 s — the case the previous `recordVideo` implementation reduced to ~0.03 s. - **Long silence then activity** (15 s untouched, then an app launch): stream survived, no warning, frames after the silence show the new screen. This caught a real bug during review — the connect timeout was also acting as a socket *inactivity* timeout and tore the stream down at 10 s of stillness; fixed and covered by a regression test that fails without the fix. - **Time-limit cap**: `timeLimitSeconds: 5` self-finalized at 4.97 s / 149 frames, the per-call note flipped to "already ended (it hit its 5s time limit)", and a stop issued 11 s later still returned the video with the matching warning. ## Not supported (capability-gated with clear errors) tvOS simulators, physical iPhones, Chromium apps, Vega, and remote (`remote:`-prefixed) simulators — none of them expose a readable frame stream. --------- Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com> |
||
|
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. |
||
|
Stanisław Chmiela
|
a7775e4ba6 |
feat(tool-server): add tool-server event log (#367)
We want to display a nicely parsed and rendered log of events that happened over a span of an Argent session. Current log file is not designed to be machine-readable, so adding a JSONL-formatted log of events (gated by `tool-server-event-log`). I eventually want this 😋 https://x.com/j3nnings/status/2069072177584849192 When enabled, the tool server writes Bunyan-compatible records to `ARGENT_EVENT_LOG` or `~/.argent/tool-server-events.jsonl`. The log captures tool-server lifecycle events and registry lifecycle events, including tool invocations and others. To make messages nicer, added `interaction` object to tool definition. This allows tools to define a nicer message in log (print "Screenshot capture completed." instead of "Tool `screenshot` completed.") Adding these to the existing tools is a follow up PR matter I think. |
||
|
Ignacy Łątka
|
a92c859d07 |
feat(argent-lens): argent lens — bind a claude session to the preview window (#416)
## What Adds `argent lens` — open Argent Lens bound to a fresh `claude` session. ## Why The Lens preview window and the agent communicate through a shared, process-wide tool-server that carries **no per-agent identity** (only a coarse telemetry header). So when the user requests changes in the window, there's no reliable way to route them to the *specific* agent that opened it. `argent lens` sidesteps the problem by inverting ownership: the human launches Lens, and Lens spawns (and therefore owns) exactly one `claude` terminal. The binding is 1:1 by construction — no identity plumbing required. ## How it works 1. `argent lens` ensures a tool-server and **begins a CLI Lens session**, which opens the preview window up front (no `await_user_selection` needed) and keeps it open across rounds. 2. It spawns a tracked `claude` terminal in the current directory — a TypeScript port of the macOS applet's `osascript` spawn/track mechanism — seeded to propose variants without blocking. 3. A watcher polls `GET /preview/outcome`; on each submitted round it types a one-line feedback summary into that terminal, queuing it to the agent as its next prompt. Requesting changes therefore "talks back" to the bound agent instead of closing the window. macOS only (drives Terminal/iTerm via `osascript`); off-darwin it exits with a clear message. ## Changes - **CLI** `lens-terminal.ts` — `osascript` spawn/track + a new write-into-running-session path (the "outputting" channel; injects input into the running TUI, which a tty write cannot do). - **CLI** `lens.ts` — the command + feedback formatter. - **tool-server** — a `cliSession` flag in the variant store (opens the window, suppresses auto-close on submit); `POST /preview/cli-session` and `GET /preview/outcome` routes. - **UI** — relabels the submit action to "Request changes" while a CLI session is active. Behind the existing `argent-lens` feature flag. ## Verification - Build + typecheck across the workspace; ESLint (`--max-warnings 0`) + Prettier clean. - 34 new unit tests; full `@argent/cli` suite **118 passing**, tool-server variant/preview suites **46 passing**. - **E2E (observed on this machine):** - **tool-server HTTP** — `cli-session` toggles `cliSession` true/false; submitted feedback round-trips through `/preview/outcome`. - **terminal injection** — spawned a window running `cat`, confirmed `writeToSession` lands text in the running program's **stdin** (both iTerm and Terminal), with embedded newlines flattened. - **full relay** — the real `watchAndRelay` loop against a real tool-server typed two distinct submission rounds' feedback into a stand-in agent terminal (dedup on `completedAt` verified). - `argent lens --help` and the feature-flag gate (exit 1 with guidance, no spawn). Note: a real end-to-end with a live `claude` agent was not run (it would drive a paid interactive session); the agent-facing channel is proven by injecting into a stand-in process's stdin via the exact same code path. --------- Co-authored-by: Filip131311 <contact@filipkaminski.com> |
||
|
Paweł Fornagiel
|
92cc69a75c |
feat: extract configuration-core and migrate auto-screenshot to feature flag (#295)
## Summary - Introduces `@argent/configuration-core` as the shared source of truth for feature-flag storage, the registry, and `isFlagEnabled`, so MCP and CLI can import the same primitives without coupling runtime code to CLI-only modules. - Replaces `ARGENT_AUTO_SCREENSHOT` with the off-by-default `disable-auto-screenshot` flag (auto-screenshot stays on unless enabled via `argent enable disable-auto-screenshot`); renames `ARGENT_AUTO_SHUTDOWN` to `ARGENT_TOOL_SERVER_SHUTDOWN_ON_MCP_EXIT`. - Keeps `@argent/cli` as the command layer (`enable` / `disable` / `flags`) with backward-compat re-exports, colored flag state output (picocolors), and `--help` listings of available flags. --------- Co-authored-by: Filip131311 <contact@filipkaminski.com> Co-authored-by: Cursor <cursoragent@cursor.com> |