Commit Graph

3 Commits

Author SHA1 Message Date
Ignacy Łątka 5bc9c7be4c test: make four non-discriminating test sites fail on what they claim to pin (#1059)
Four tests could not fail on the thing they name: an assertion implied
by the one below
it, an assertion satisfied by the fixture's own filename, a clearance
poll with nothing
to clear, and `it.each` rows whose rendered name described only argv[0]
- two of them
stating the opposite of what the row asserts.

Each site now fails when its claim is broken, proven by mutation. Tests
only, no source
change, so no docs update.

Fixes #943
Fixes #942
Fixes #841
Fixes #861

<details>
<summary>#943 &mdash; capability: toolId/platform were substrings of the
path assertion</summary>

`toContain("demo-tool")` + `toContain("android")` are both substrings of
`tools/demo-tool/platforms/android.ts`, asserted on the next line.
Replaced by the prose
sentence, which names both outside the path.

Mutation - drop the sentence naming the tool and the platform:

```diff
-      `Tool '${opts.toolId}' is not yet implemented on ${opts.platform}. ` +
+      `Tool is not yet implemented. ` +
```

| | result |
|---|---|
| before | `Tests 7 passed (7)` - green |
| after | `AssertionError: expected 'Tool is not yet implemented. The
cros…' to contain 'Tool \'demo-tool\' is not yet impleme…'` |

The sibling "works without a hint" assertions are load-bearing and
untouched.
</details>

<details>
<summary>#942 &mdash; native-profiler: the wording assertion matched
inside missing_cpu.xml</summary>

`/missing|not found|unreadable/i` matched the fixture filename, so any
product copy
passed. The wording now has to land before the backticked path (a
negated character class
cannot cross into it), which no path can satisfy.

Mutation - reword the ENOENT copy:

```diff
-    if (code === "ENOENT") return `not found at \`${filePath}\``;
+    if (code === "ENOENT") return `absent at \`${filePath}\``;
```

| | result |
|---|---|
| before | `Tests 1 passed (1)` - green (also green with the state word
deleted outright) |
| after | `AssertionError: expected '# iOS Instruments Analysis…' to
match /-\s*\*\*cpu\*\*:[^…]*(?:missing\|not found\|unreadable)/i` |
</details>

<details>
<summary>#841 &mdash; vega-cli: the output-cap reaps had nothing to
observe</summary>

`expect(await waitForClear(SENTINEL_OVERFLOW)).toBe(0)` reads 0 just as
readily for a
launcher that never spawned a worker. The cap trips on the first write,
so there was no
window to see one in: the fake now spawns its sentinel worker **before**
the flood and
holds the flood - and so the reap - until the test writes a gate file,
having seen the
worker. Both the stdout and the stderr twin.

Gated rather than timed, so the observation is decided, not raced. Cost
is ~+110ms and
~+50ms on the two tests (165/177ms to 278/226ms).

Mutation - the flood branches' workers stop carrying the sentinel, reap
path untouched:

```diff
-  const worker = spawn("sleep", [secs], { stdio: "ignore" });
+  const worker = spawn("sleep", ["600.0"], { stdio: "ignore" });
   ...
-  spawnSync("sleep", [secs]);
+  spawnSync("sleep", ["600.0"]);
```

| | result |
|---|---|
| before | `Tests 12 passed (12)` - green, and `pgrep -f '^sleep
600\.0$'` empty (the reap works; nothing observed it) |
| after | both overflow tests `AssertionError: expected +0 to be 1` |

Cross-check that the pair pins the production reap: with `reapOnce()`
made a no-op in
`vega-cli.ts`, both fail `expected 2 to be +0`.

**Flake runs: 30 consecutive passes** (2 x 15, before and after the
comment pass),
`12 passed (12)` every time.
</details>

<details>
<summary>#861 &mdash; four it.each rows named only argv[0] (plus a fifth
site)</summary>

A single `%j` against a multi-element row renders only the first
element. Each row is now
one argv, nested, so `%j` names all of it.

`packages/argent-cli/test/tools-help.test.ts`

```
- prints usage for "--help" without contacting the tool-server   <- duplicate
- prints usage for "--json" without contacting the tool-server   <- false: a bare --json
                                                                    prints no usage and
                                                                    does hit the server
+ prints usage for ["--help","--json"] without contacting the tool-server
+ prints usage for ["--json","--help"] without contacting the tool-server
```

`packages/argent/test/installer-help.test.ts`

```
- treats "--foo" as a help request      <- rendered twice; a bare --foo is the opposite
+ treats ["--foo","--help"] as a help request
+ treats ["--foo","help"] as a help request
- does not treat "--metro-port" as a help request
+ does not treat ["--metro-port","8082"] as a help request
```

Swept the class across every `it.each` array-literal table in the repo
(154 sites). One
more reproduces it -
`packages/argent-cli/test/telemetry-command.test.ts`, fixed here:

```
- prints usage for status and writes nothing    <- rendered twice
- prints usage for disable and writes nothing   <- false: a bare `disable` writes config
+ prints usage for ["status","--help"] and writes nothing
+ prints usage for ["status","-h"] and writes nothing
+ prints usage for ["disable","-h"] and writes nothing
```

Left alone: ~64 sites with a discriminating label column (`_label`
first, house style) and
16 whose first column is a real discriminator - names unique and
truthful in both groups.
`providers-command.test.ts:440` drops a third "shape" column from its
name but stays
unique and true, so it is a legibility nit, not this defect.
</details>

<details>
<summary>Checks run</summary>

- `npm run format`, `npm run lint` (the latter needs `npm ci` inside
`packages/docs` locally)
- `npx tsc --noEmit -p
packages/{tool-server,argent-cli,argent}/tsconfig.test.json`
- `packages/tool-server`: `npx vitest run --shard=1/6` .. `6/6` - 6163
passed, 1 skipped
- `npx vitest run --root packages/argent-cli` (619), `--root
packages/argent` (80)
- `vega-cli-timeout.test.ts` x30 consecutive, zero flakes
</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Improved test parameterization so command-line help cases display
complete argument lists accurately.
* Strengthened error-message and profiler warning assertions to verify
exact wording and placement.
* Improved subprocess timeout and output-limit tests by synchronizing
worker startup and cleanup checks.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-09 12:38:37 +02:00
Ignacy Łątka 2e19c97de0 fix(cli): accept --help after a telemetry subcommand (#959)
Found while differentially regression-testing the 0.22.1 release against
a bundle built from `v0.22.0`.

## The problem

`argent telemetry status --help` exits 2 with `Error: Unknown flag:
--help`. So do `-h`, and the same after `enable` / `disable`. On 0.22.0
all of them exited 0.

```
$ argent telemetry status --help
0.22.0   telemetry: / state: enabled / device id: not created     (rc=0)
0.22.1   Error: Unknown flag: --help                              (rc=2)
```

`telemetry` is the only subcommand that behaves this way. Checked
against every sibling on both versions — `server status|logs|start`,
`config list|get|set`, `flow list|run`, `secrets`, `flags`, `enable`,
`link`, `unlink` all return 0 on both. And the top-level help tells the
user to run exactly this: "Run `argent <command> --help` for
command-specific help."

## Cause

`TELEMETRY_OPTIONS` declares only `scope`, and
`parseCommandArgs(args.slice(1), …)` reads everything after the
subcommand — so `--help` arrives as an unknown flag. The `case "--help"`
arm in the switch only fires when help is `args[0]`, which is why bare
`argent telemetry --help` still works.

Introduced when `telemetry` was converted to the declarative parser
(#952, extended by #953).

## The fix

Hoist the help check above the parser, matching what `config.ts:220` and
`flags.ts:92` already do; `server`, `link` and `lens` instead declare
`help` in their option spec. Telemetry did neither. Ordering of
`telemetryInit` / `telemetryShutdown` around the help path is unchanged,
so a help invocation behaves exactly as `argent telemetry --help` did
before.

## Tests

Six cases added to `telemetry-command.test.ts` covering `--help` / `-h`,
bare and after each subcommand, asserting usage is printed and no config
file is written.

Mutation-checked: with the source change reverted, 4 of the 6 fail (the
2 bare-help cases pass either way, since those already worked) — so the
test pins the actual defect rather than the behaviour around it.

`@argent/cli`: 498 passed, 3 skipped. `tsc --build` clean, prettier
clean.

## Docs

No docs update needed — this restores previously documented behaviour
rather than changing a documented surface.

Co-authored-by: Ignacy Łątka <f.kaminski2000@gmail.com>
2026-08-25 17:05:57 +02:00
filip131311 c2324f052d feat(telemetry): project-scope opt-out (restrictive merge) + init --local --no-telemetry (#952)
## Why

Teams that must not send usage data to a third party (a
strict-compliance customer asked) had no way to turn telemetry off for a
repository. `telemetry.enabled` was global-only
(`~/.argent/config.json`), so a committed local install (`init --local`)
still defaulted every teammate to **on** — `--no-telemetry` only opted
out the machine that ran `init`.

## What

- **`telemetry.enabled` accepts the project scope.** `consent.ts` now
reads both `~/.argent/config.json` and
`<project-root>/.argent/config.json` (root resolved from `cwd`, per-file
cache keyed by path) and merges **restrictively**: `false` in either
document wins. A committed project opt-out holds on every clone, and a
project file can never re-enable a global opt-out. Env overrides
(`DO_NOT_TRACK`, `ARGENT_TELEMETRY`) still outrank both.
- **`argent telemetry enable|disable --scope global|project`** (default
`global`). `status` names the document that decided (`config.json
(project)`, `(global)`, `(project and global)`). `enable` at one scope
while the other still opts out reports that telemetry stays disabled
instead of claiming success. Unknown scope / argument → exit 2, nothing
written.
- **`argent init --local --no-telemetry`** also writes
`telemetry.enabled: false` to `.argent/config.json`, and the "commit
these files" checklist lists it. Global mode is unchanged.
- `argent config set/unset telemetry.enabled` stays delegated to `argent
telemetry` (`manageCommand`), so the lifecycle work (draining the live
client) is still done by the dedicated command.

## Tests

Unit: `telemetry` (+8 project-scope cases: single-scope opt-out, both
restrictive directions, both-off, env precedence, live re-read of the
project file, malformed project file), `configuration-core` (schema
scopes), new `argent-cli/test/telemetry-command.test.ts` (real-fs, 6
cases), `argent-installer` — all green; eslint, prettier, knip,
`docusaurus build` green.

Live, from a real `npm pack` tarball of this branch (sandboxed `HOME`):

1. Fresh project → `init --local --yes --no-telemetry --from <tgz>`:
writes `.argent/config.json` `{telemetry:{enabled:false}}` + global
opt-out; summary lists `.argent/config.json` to commit.
2. `telemetry status` → `disabled`, source `config.json (project and
global)`. `telemetry enable` (global) → "stays disabled: config.json
(project) still opts out"; from `~` (no project) → `enabled`. `enable
--scope project` → enabled; `disable --scope project` → disabled.
`--scope team` → exit 2.
3. **Teammate simulation**: copied only the committed files to a fresh
clone with a fresh `HOME` (no global opt-out), bundle built with a dummy
OTLP token, `ARGENT_TELEMETRY_DEBUG=1`, drove `argent mcp` (initialize →
tools/list → `list-devices` call). Control clone without
`.argent/config.json`: tool-server emitted `toolserver:start`,
`tool:invoke`, `tool:complete`, `toolserver:stop`. Clone with the
committed opt-out: **0 events**, and the first-run telemetry notice is
not printed.

## Caveat (documented)

The project file is resolved from the working directory of the process.
Editors start the MCP server in the workspace root, so it and the
tool-server it launches read it. The tool-server is shared by every
editor on the machine and reads the project file of the workspace that
started it; the global opt-out or `DO_NOT_TRACK` still covers the
machine-wide case.

## Docs

Updated in this PR: `reference/configuration.mdx` (scopes + merge
wording), `reference/telemetry.mdx` (new "Opt out for a whole
repository" section), `reference/cli.mdx` (`init`, `telemetry` rows),
`fundamentals/installation.mdx` (`--no-telemetry` in local mode).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu
2026-08-25 11:24:02 +02:00