diff --git a/.agents/skills/verify-nicely/SKILL.md b/.agents/skills/verify-nicely/SKILL.md index 5bb02d2d..b203f05c 100644 --- a/.agents/skills/verify-nicely/SKILL.md +++ b/.agents/skills/verify-nicely/SKILL.md @@ -17,13 +17,27 @@ Copy this checklist and tick each step: - [ ] 5. Evidence: the transcripts outlive the cleanup, and the report quotes them ``` +On Linux, a run also needs `dbus-run-session` and `gnome-keyring-daemon`. +Launch fails with the missing program's name; it never uses the caller's D-Bus session. +The run starts and unlocks its own keyring in one private session, shared by every later step. +Cleanup stops that keyring and its bus before removing scratch files. +The session also stops when its owned state or runtime marker disappears, and expires after six hours. +Start a new run after expiry; the wrapper never falls back to the user's session. +The Linux integration test follows this real start, auth and cleanup path. +It skips with the missing program's name when either prerequisite is absent. +[The lifecycle test](../../../cmd/ncly/keyring_lifecycle_linux_test.go) requires a normal private stop to record exit 0 without a shell diagnostic. +It also checks daemon termination and private runtime removal. +The macOS keychain path is unchanged. + ## Launch ```bash run="$(just verify)" ``` -`just verify` runs `scripts/verify.sh start`. It builds `ncly` from the checkout as it is, uncommitted changes included, into `$run/build/`. It makes a scratch home in `$run/scratch/home/`, with its own XDG folders, and installs the Go fixture `hello` there as an external extension, in `.local/share/nicely/extensions/`, with its manifest, catalogs, and skill beside it. It writes the wrapper `$run/scratch/bin/ncly`, which sets `HOME`, the XDG folders, and a `PATH` without any folder that holds an `ncly-` file, and unsets `NCLY_CONFIG`, which would point the config elsewhere, so every later command reaches only the scratch folder. On macOS, the auth feature points `HOME` back at the real home, as [auth](features/auth.md) says. Its last line on stdout is the run folder, under `${XDG_CACHE_HOME:-$HOME/.cache}/nicely/verify/`. Done when that folder holds `scratch/bin/ncly`. +`just verify` runs `scripts/verify.sh start`. It builds `ncly` from the checkout as it is, uncommitted changes included, into `$run/build/`. It makes a scratch home in `$run/scratch/home/`, with its own XDG folders, and installs the Go fixture `hello` there as an external extension, in `.local/share/nicely/extensions/`, with its manifest, catalogs, and skill beside it. It writes the wrapper `$run/scratch/bin/ncly`, which sets `HOME`, the XDG folders, and a `PATH` without any folder that holds an `ncly-` file, and unsets `NCLY_CONFIG`, which would point the config elsewhere, so every later command reaches only the scratch folder. On Linux, the wrapper replaces the caller's bus address and runtime directory with the private session of this run. +A missing or stopped private session fails the call, with no fallback. +On macOS, the auth feature points `HOME` back at the real home, as [auth](features/auth.md) says. Its last line on stdout is the run folder, under `${XDG_CACHE_HOME:-$HOME/.cache}/nicely/verify/`. Done when that folder holds `scratch/bin/ncly`. ## Doctor @@ -49,13 +63,17 @@ just verify cleanup "$run" ``` `cleanup` makes scratch and build files writable, including Go's module cache, before removal. +On Linux, it first stops the private keyring and D-Bus session, after any fake-key cleanup. +Its evidence records that stop. A stop failure keeps scratch files and fails cleanup. +If the private session already ended, cleanup discards its private keyring with the scratch folder. It keeps `$run/evidence/` and refuses a folder that `start` did not create. When the run prepared `auth`, cleanup first removes its fake key and checks that the keychain reports `AUTH_MISSING`. Both calls remain in `$run/evidence/cleanup.txt`. Its stderr says `logged the fake key of verifynicely out of the keychain` only after removal. Otherwise, stderr says `found no fake key`. When the key may remain, cleanup keeps scratch and exits 1. -Run cleanup again once the keychain answers. +On macOS, run cleanup again once the login keychain answers. +On Linux, rerun cleanup to discard the stopped private keyring and its scratch files. Run cleanup after a failed drive too. Done when cleanup lists the evidence files and both `$run/scratch` and `$run/build` are gone. diff --git a/.agents/skills/verify-nicely/features/README.md b/.agents/skills/verify-nicely/features/README.md index f2a9dfcc..2845e78e 100644 --- a/.agents/skills/verify-nicely/features/README.md +++ b/.agents/skills/verify-nicely/features/README.md @@ -35,7 +35,7 @@ Each feature file starts with an H1 title and one paragraph about what the user ## Features - `shared-domains.md` covers named roots, shared discovery, help, describe, completion, skills, config ownership, doctor, and grants -- `hello.md` covers the Go fixture installed as an external extension: discovery, help, answers, errors, reports, describe, its skill, and doctor +- `hello.md` covers the installed external Go fixture extension: discovery, help, answers, errors, reports, describe, its skill, doctor, and bounded handler cleanup of private staging - `auth.md` covers `ncly auth` against the real keychain, with a test service and a fake key: status and its grants, login, logout, grant, revoke, the masked field and the confirmations in a terminal, doctor, and the cleanup of the fake key - `terminal.md` covers `ncly` on a real terminal inside tmux, whose server does not answer: the version and the root help answer at once - `agent.md` covers `just install` in the scratch home and the installed `agent` extension, with a counted stub Codex: discovery, help, its skill, the Codex check of doctor, the dry run of `ncly agent run`, its depth limit, and one recorded run with its saved answer diff --git a/.agents/skills/verify-nicely/features/auth.md b/.agents/skills/verify-nicely/features/auth.md index 632f7ca4..944b9a9d 100644 --- a/.agents/skills/verify-nicely/features/auth.md +++ b/.agents/skills/verify-nicely/features/auth.md @@ -1,6 +1,8 @@ # auth -`ncly auth` keeps the keys of extensions. `ncly auth status` lists each service that an installed extension declares, says whether its key comes from its environment variable or from the OS keychain, without printing any key, and which extensions hold its grant. `ncly auth login` stores a key in the keychain, and `ncly auth logout` removes it. In a terminal, `login` asks for the key in a masked field, and a replacement or a logout asks for a typed `[y/N]` confirmation. `ncly auth grant` lets an extension receive a key once a human confirms it in a terminal, and `ncly auth revoke` takes the grant back. This feature drives it against the real keychain, with the test service `verifynicely`, whose variable is `VERIFYNICELY_API_KEY`, and a fake key that is no human's key. Its fixture `verifykeys` declares that service, and `ncly verifykeys digest` answers the SHA-256 of the key that it receives, never the key. It never touches the key of a real service. +`ncly auth` keeps the keys of extensions. `ncly auth status` lists each service that an installed extension declares, says whether its key comes from its environment variable or from the OS keychain, without printing any key, and which extensions hold its grant. `ncly auth login` stores a key in the keychain, and `ncly auth logout` removes it. In a terminal, `login` asks for the key in a masked field, and a replacement or a logout asks for a typed `[y/N]` confirmation. `ncly auth grant` lets an extension receive a key once a human confirms it in a terminal, and `ncly auth revoke` takes the grant back. On Linux, this feature uses the real Secret Service in the run's private session. +On macOS, this feature uses the login keychain. +This feature drives the keychain with the test service `verifynicely`, whose variable is `VERIFYNICELY_API_KEY`, and a fake key that is no human's key. Its fixture `verifykeys` declares that service, and `ncly verifykeys digest` answers the SHA-256 of the key that it receives, never the key. It never touches the key of a real service. ## Sub-features @@ -42,58 +44,18 @@ ## Driving it with verify.sh -Preconditions: +Follow these guides in order: -- A new run from `just verify`, whose Doctor step passed. On macOS, an agent runs it as Gotchas says -- `tmux -V` answers, for the steps in a terminal, as [terminal.md](terminal.md) describes `just verify tmux` -- The keychain may hold the fake key of an earlier run under `verifynicely`, left by a release without logout or by a cleanup that could not finish -- The fake key of this run: `fake=verify-nicely-fake-$(date +%s)` - -- **No default keychain.** On macOS, before the step Fixture, run `just verify step "$run" auth auth status --json`. Exit 0 with `"keychain":"unavailable"`: the scratch home holds no keychain. Then `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --json` exits 78 with `KEYRING_UNAVAILABLE`, and no window opens. -- **Fixture.** Run `just verify prepare "$run" auth`, with `--mac-keychain` on macOS. Exit 0, and stderr names the service `verifynicely` and its variable `VERIFYNICELY_API_KEY`, and on macOS that the wrapper keeps the real `HOME`. -- **Help.** Run `just verify step "$run" auth auth --help`. Exit 0, and `COMMANDS` lists `grant`, `login`, `logout`, `revoke`, and `status`. -- **Grant dry run.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --dry-run --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"dry_run":true,"pending_checks":[{"id":"auth.confirmation","message":"Whether a human at a terminal confirms the grant of the key of verifynicely to verifykeys."}],"plan":[{"key":"verifynicely","action":"grant","domain":"verifykeys","effects":{"user_writes":true,"network":false,"paid":false}}]}`. -- **Grant without a terminal.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --json`. Exit 78, stdout is empty, and stderr ends with an answer whose first error is `TERMINAL_REQUIRED`, with the hint ``Run `ncly auth grant verifynicely verifykeys` in a terminal`` and the `next` `["ncly","auth","grant","verifynicely","verifykeys"]`. -- **No grant to revoke.** Run `just verify step "$run" auth auth revoke verifynicely verifykeys --force --json`. Exit 2, and stderr ends with an answer whose first error is `NOT_FOUND`, with the hint `ncly auth status` and the same command in `next`. -- **Missing key.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, stderr is empty, and stdout is one line with `"ok":false`, `"keychain":"available"`, and one entry, whose `grants` is `[{"domain":"verifykeys","granted":false,"optional":false}]`. Without the fake key of an earlier run, the entry is `{"service":"verifynicely","configured":false,"source":"none","code":"AUTH_MISSING", ...}`, whose `hint` is `ncly auth login verifynicely` and whose `next` is `["ncly","auth","login","verifynicely"]`: a missing key reports its own code alone, without its grant. With it, the entry is `{"service":"verifynicely","configured":true,"source":"keychain","code":"KEY_NOT_GRANTED", ...}`, as in the step Key from the keychain: report which one you saw. -- **Missing key as text.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status`. Exit 78. The first line is `keychain available`, then an empty line. Without the fake key of an earlier run, the line of `verifynicely` is `verifynicely none verifykeys: not granted`. Two lines follow it, in the column of `none`: `fail[AUTH_MISSING] No key for verifynicely in VERIFYNICELY_API_KEY or the keychain.`, then `hint: ncly auth login verifynicely`. With the key, the line is `verifynicely keychain verifykeys: not granted`, and the two lines are `fail[KEY_NOT_GRANTED] The key of verifynicely is not granted to verifykeys.`, then `hint: ncly auth grant verifynicely verifykeys`. -- **Key from the variable.** Run `VERIFYNICELY_API_KEY="$fake" just verify step "$run" auth auth status --json`. Exit 78, and stdout is `{"ok":false,"contract_version":1,"keychain":"available","services":[{"service":"verifynicely","configured":true,"source":"env","code":"KEY_NOT_GRANTED","message":"The key of verifynicely is not granted to verifykeys.","hint":"ncly auth grant verifynicely verifykeys","next":["ncly","auth","grant","verifynicely","verifykeys"],"grants":[{"domain":"verifykeys","granted":false,"optional":false}]}]}`: the key is there, and no human granted it to the fixture yet. -- **Login dry run.** Run `just verify step "$run" auth auth login verifynicely --dry-run --json`. Exit 0, and stdout holds `"dry_run":true`, the plan `{"key":"verifynicely","action":"login","effects":{"user_writes":true,"network":false,"paid":false}}`, and the pending check `auth.keychain`. -- **Login.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"action":"login","service":"verifynicely"}`. -- **Key from the keychain.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and stdout is the one of the step Key from the variable, with `"source":"keychain"` instead of `"source":"env"`. -- **Doctor without the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor auth --json`. Exit 78, stderr is empty, and stdout is `{"ok":false,"contract_version":1,"checks":[{"id":"auth.keychain","component":"auth","status":"pass","message":"The keychain answers."},{"id":"auth.grants","component":"auth","status":"pass","message":"Every grant is in use."},{"id":"auth.verifynicely","component":"auth","status":"fail","code":"KEY_NOT_GRANTED","message":"The key of verifynicely is not granted to verifykeys.","hint":"ncly auth grant verifynicely verifykeys","next":["ncly","auth","grant","verifynicely","verifykeys"]}]}`. -- **Fixture without a grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 78, stdout is empty, and stderr is one line, an answer whose first error is `KEY_NOT_GRANTED`, with the hint `ncly auth grant verifynicely verifykeys` and the same command in `next`: the keychain holds the fake key, and still it does not reach the fixture. -- **Grant in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' y auth grant verifynicely verifykeys`. Exit 0. Before Enter, the terminal shows `? Grant the key of verifynicely to verifykeys?`, then ` Path ` followed by the fixture's path, which ends in `/scratch/home/.local/share/nicely/extensions/ncly-verifykeys`, then ` [y/N] y`. After Enter, it also shows `Granted the key of verifynicely to verifykeys.`, and no `Next time:` line. -- **Fixture with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"sha256":""}`. Compute the digest of the fake key without printing the key, with `printf '%s' "$fake" | sha256sum`, or `shasum -a 256` on macOS, and check that the two match: the fixture received the fake key from the keychain. -- **Status with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"keychain":"available","services":[{"service":"verifynicely","configured":true,"source":"keychain","grants":[{"domain":"verifykeys","granted":true,"optional":false}]}]}`. Then run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status`. Exit 0, and the line of `verifynicely` is `verifynicely keychain verifykeys: granted`, with no line under it. -- **Doctor with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor auth --json`. Exit 0, and stdout is the one of the step Doctor without the grant, with `"ok":true` and the last check `{"id":"auth.verifynicely","component":"auth","status":"pass","message":"The key comes from the keychain."}`. -- **Grant again.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --json`. Exit 0 without a terminal, and stdout is `{"ok":true,"contract_version":1,"action":"unchanged","domain":"verifykeys","path":"","service":"verifynicely"}`, where `` is the path that the step Grant in a terminal showed. -- **Revoke.** Run `just verify step "$run" auth auth revoke verifynicely verifykeys --force --json`. Exit 0, and stdout holds `"action":"revoke"`. Then run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 78, with `KEY_NOT_GRANTED` again. -- **Replacement.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --json`. Exit 2, and stderr ends with an answer whose first error is `CONFIRMATION_REQUIRED`, with the hint `ncly auth login verifynicely --stdin --force`. -- **Login as text.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force`. Exit 0, and stdout is `Stored the key of verifynicely in the keychain.` -- **Logout dry run.** Run `just verify step "$run" auth auth logout verifynicely --dry-run --json`. Exit 0, and stdout holds what the step Login dry run names, with `"action":"logout"`. -- **Logout without --force.** Run `just verify step "$run" auth auth logout verifynicely --json`. Exit 2, and stderr ends with an answer whose first error is `CONFIRMATION_REQUIRED`, with the hint `ncly auth logout verifynicely --force` and the same command in `next`. -- **Logout.** Run `just verify step "$run" auth auth logout verifynicely --force --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"action":"logout","service":"verifynicely"}`. -- **Key gone.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and the entry of `verifynicely` has `"source":"none"` and `"code":"AUTH_MISSING"`, with no other code. -- **Login in a terminal.** Run `just verify tmux "$run" auth --answer 'verifynicely key' "$fake" auth login verifynicely`. Exit 0. Before Enter, the terminal shows the masked field: `┃ verifynicely key`, `┃ It goes to the keychain and never shows.`, `┃ > ` followed by one `*` per character of the fake key, and `enter submit`. After Enter, the field is gone, and the terminal shows only `Stored the key of verifynicely in the keychain.`, with no `Next time:` line. -- **Key from the terminal.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and stdout is the one of the step Key from the keychain: the key comes from the keychain again, and the step Revoke took its grant back. -- **Replacement in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' n auth login verifynicely`. Exit 2. The terminal shows `? The keychain already holds a key for verifynicely. Replace it? [y/N] n`, then `error[CONFIRMATION_REQUIRED]: The key of verifynicely stays as it was.`, and no masked field. -- **Ctrl-C at the field.** Run `just verify tmux "$run" auth --answer 'verifynicely key' '^C' auth login verifynicely --force`. Exit 130. The terminal shows only `error[INTERRUPTED]: Stopped by Ctrl-C. Finished work is kept.` and its hint, because the field erased itself. -- **Logout in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' y auth logout verifynicely`. Exit 0. The terminal shows `? Remove the key of verifynicely from the keychain? [y/N] y`, then `Removed the key of verifynicely from the keychain.` -- **Doctor without the key.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor --json`. Exit 78, stderr is empty, and stdout lists `core.config`, then `auth.keychain` and `auth.grants` as a `pass`, then `{"id":"auth.verifynicely","component":"auth","status":"fail","code":"AUTH_MISSING", ...}` with the hint and `next` of the status entry, then the `extensions` checks, each a `pass`. -- **Nothing to remove.** Run `just verify step "$run" auth auth logout verifynicely --force --json`. Exit 2, and stderr ends with an answer whose first error is `NOT_FOUND`, with the hint and `next` `ncly auth status`. -- **Missing service.** Run `just verify step "$run" auth auth login`. Exit 2, stdout is empty, and stderr starts with `error[USAGE_INVALID]: The argument is missing.`, with the hint `ncly auth login `. -- **Login from the form.** Run `just verify tmux "$run" auth --answer 'Service of the key' verifynicely --answer 'verifynicely key' "$fake" auth login`. Exit 0. Before the first Enter, the terminal shows the form for missing values: `┃ service`, `┃ Service of the key, such as deepgram`, and `┃ > verifynicely`. Before the second, it shows the masked field of the step Login in a terminal. After it, both are gone, and the terminal shows `Stored the key of verifynicely in the keychain.`, then `Next time:` and ` ncly auth login verifynicely`, never the key. -- **Login for the cleanup.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force --json`. Exit 0, so the cleanup has a key to log out. -- **Proof.** Run `grep -c "$fake" "$run/evidence/auth.txt"` after the steps. It prints `0` and exits 1, grep's code for no match: no transcript holds the fake key, not even the one that the terminal steps typed. -- **Cleanup.** Run `just verify cleanup "$run"`, as SKILL.md says. Exit 0, stderr says `logged the fake key of verifynicely out of the keychain`, and `$run/evidence/cleanup.txt` holds an `ncly auth logout verifynicely --force --json` that exits 0, then an `ncly auth status --json` with `"keychain":"available"`, whose entry of `verifynicely` has `"source":"none"` and `"code":"AUTH_MISSING"`. +1. [Prepare and inspect the key](auth/prepare.md) +2. [Grant and remove the key](auth/grant.md) +3. [Terminal steps and cleanup](auth/terminal.md) ## Gotchas - Set `VERIFYNICELY_API_KEY` on the step itself, as written: the wrapper passes the caller's environment to `ncly`, so a value left in the shell changes the result of every later step - Quote each `--answer` as written: the shell would expand the brackets of `[y/N]` otherwise -- `"keychain":"unavailable"` on Linux means that no Secret Service answers, such as gnome-keyring outside a desktop session: report it, because the steps that expect `available` then fail with `KEYRING_UNAVAILABLE` +- `"keychain":"unavailable"` on Linux means the run's private Secret Service failed: stop and report it. Never use the user's session instead - On macOS, `prepare` needs `--mac-keychain`. Run the steps in the desktop session, because the login keychain refuses an SSH session. [Desktop session](../../../../docs/qa/protocol.md#desktop-session) in docs/qa says how, and when to call a human - `ncly auth status` and `ncly doctor auth` fail the stored key with `KEY_NOT_GRANTED` until the step Grant in a terminal, and again after the step Revoke: a key that no extension may receive is a problem to fix, and exit 78 is the expected result there - `just verify step` has no terminal, and `grant` takes no `--force`, so only the `tmux` step grants the key. Never print the fake key to compare digests: pipe it straight into `sha256sum` -- The cleanup logs the fake key out after a failed run too. When it cannot, because the keychain does not answer, it keeps the scratch folder and exits 1: run `just verify cleanup "$run"` again once the keychain answers, and report it if it still fails +- Cleanup removes the fake key after a failed run too. If it cannot confirm removal, cleanup keeps scratch and exits 1. On macOS, retry when the login keychain answers. On Linux, retry to discard the stopped private keyring. Report a repeated failure diff --git a/.agents/skills/verify-nicely/features/auth/grant.md b/.agents/skills/verify-nicely/features/auth/grant.md new file mode 100644 index 00000000..15c3966a --- /dev/null +++ b/.agents/skills/verify-nicely/features/auth/grant.md @@ -0,0 +1,16 @@ +# Grant and remove the key + +Part of [auth](../auth.md). Follow the three guides in order. + +- **Grant in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' y auth grant verifynicely verifykeys`. Exit 0. Before Enter, the terminal shows `? Grant the key of verifynicely to verifykeys?`, then ` Path ` followed by the fixture's path, which ends in `/scratch/home/.local/share/nicely/extensions/ncly-verifykeys`, then ` [y/N] y`. After Enter, it also shows `Granted the key of verifynicely to verifykeys.`, and no `Next time:` line. +- **Fixture with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"sha256":""}`. Compute the digest of the fake key without printing the key, with `printf '%s' "$fake" | sha256sum`, or `shasum -a 256` on macOS, and check that the two match: the fixture received the fake key from the keychain. +- **Status with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"keychain":"available","services":[{"service":"verifynicely","configured":true,"source":"keychain","grants":[{"domain":"verifykeys","granted":true,"optional":false}]}]}`. Then run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status`. Exit 0, and the line of `verifynicely` is `verifynicely keychain verifykeys: granted`, with no line under it. +- **Doctor with the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor auth --json`. Exit 0, and stdout is the one of the step Doctor without the grant, with `"ok":true` and the last check `{"id":"auth.verifynicely","component":"auth","status":"pass","message":"The key comes from the keychain."}`. +- **Grant again.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --json`. Exit 0 without a terminal, and stdout is `{"ok":true,"contract_version":1,"action":"unchanged","domain":"verifykeys","path":"","service":"verifynicely"}`, where `` is the path that the step Grant in a terminal showed. +- **Revoke.** Run `just verify step "$run" auth auth revoke verifynicely verifykeys --force --json`. Exit 0, and stdout holds `"action":"revoke"`. Then run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 78, with `KEY_NOT_GRANTED` again. +- **Replacement.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --json`. Exit 2, and stderr ends with an answer whose first error is `CONFIRMATION_REQUIRED`, with the hint `ncly auth login verifynicely --stdin --force`. +- **Login as text.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force`. Exit 0, and stdout is `Stored the key of verifynicely in the keychain.` +- **Logout dry run.** Run `just verify step "$run" auth auth logout verifynicely --dry-run --json`. Exit 0, and stdout holds what the step Login dry run names, with `"action":"logout"`. +- **Logout without --force.** Run `just verify step "$run" auth auth logout verifynicely --json`. Exit 2, and stderr ends with an answer whose first error is `CONFIRMATION_REQUIRED`, with the hint `ncly auth logout verifynicely --force` and the same command in `next`. +- **Logout.** Run `just verify step "$run" auth auth logout verifynicely --force --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"action":"logout","service":"verifynicely"}`. +- **Key gone.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and the entry of `verifynicely` has `"source":"none"` and `"code":"AUTH_MISSING"`, with no other code. diff --git a/.agents/skills/verify-nicely/features/auth/prepare.md b/.agents/skills/verify-nicely/features/auth/prepare.md new file mode 100644 index 00000000..a3fdf4ff --- /dev/null +++ b/.agents/skills/verify-nicely/features/auth/prepare.md @@ -0,0 +1,26 @@ +# Prepare and inspect the key + +Part of [auth](../auth.md). Follow the three guides in order. + + +Preconditions: + +- A new run from `just verify`, whose Doctor step passed. On macOS, an agent runs it as Gotchas says +- `tmux -V` answers, for the steps in a terminal, as [terminal.md](terminal.md) describes `just verify tmux` +- On macOS, the login keychain can hold `verifynicely` from an earlier run. Linux starts with a new private keyring +- The fake key of this run: `fake=verify-nicely-fake-$(date +%s)` + +- **No default keychain.** On macOS, before the step Fixture, run `just verify step "$run" auth auth status --json`. Exit 0 with `"keychain":"unavailable"`: the scratch home holds no keychain. Then `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --json` exits 78 with `KEYRING_UNAVAILABLE`, and no window opens. +- **Fixture.** Run `just verify prepare "$run" auth`, with `--mac-keychain` on macOS. Exit 0, and stderr names the service `verifynicely` and its variable `VERIFYNICELY_API_KEY`, and on macOS that the wrapper keeps the real `HOME`. +- **Help.** Run `just verify step "$run" auth auth --help`. Exit 0, and `COMMANDS` lists `grant`, `login`, `logout`, `revoke`, and `status`. +- **Grant dry run.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --dry-run --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"dry_run":true,"pending_checks":[{"id":"auth.confirmation","message":"Whether a human at a terminal confirms the grant of the key of verifynicely to verifykeys."}],"plan":[{"key":"verifynicely","action":"grant","domain":"verifykeys","effects":{"user_writes":true,"network":false,"paid":false}}]}`. +- **Grant without a terminal.** Run `just verify step "$run" auth auth grant verifynicely verifykeys --json`. Exit 78, stdout is empty, and stderr ends with an answer whose first error is `TERMINAL_REQUIRED`, with the hint ``Run `ncly auth grant verifynicely verifykeys` in a terminal`` and the `next` `["ncly","auth","grant","verifynicely","verifykeys"]`. +- **No grant to revoke.** Run `just verify step "$run" auth auth revoke verifynicely verifykeys --force --json`. Exit 2, and stderr ends with an answer whose first error is `NOT_FOUND`, with the hint `ncly auth status` and the same command in `next`. +- **Missing key.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, stderr is empty, and stdout is one line with `"ok":false`, `"keychain":"available"`, and one entry, whose `grants` is `[{"domain":"verifykeys","granted":false,"optional":false}]`. Without the fake key of an earlier run, the entry is `{"service":"verifynicely","configured":false,"source":"none","code":"AUTH_MISSING", ...}`, whose `hint` is `ncly auth login verifynicely` and whose `next` is `["ncly","auth","login","verifynicely"]`: a missing key reports its own code alone, without its grant. With it, the entry is `{"service":"verifynicely","configured":true,"source":"keychain","code":"KEY_NOT_GRANTED", ...}`, as in the step Key from the keychain: report which one you saw. +- **Missing key as text.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status`. Exit 78. The first line is `keychain available`, then an empty line. Without the fake key of an earlier run, the line of `verifynicely` is `verifynicely none verifykeys: not granted`. Two lines follow it, in the column of `none`: `fail[AUTH_MISSING] No key for verifynicely in VERIFYNICELY_API_KEY or the keychain.`, then `hint: ncly auth login verifynicely`. With the key, the line is `verifynicely keychain verifykeys: not granted`, and the two lines are `fail[KEY_NOT_GRANTED] The key of verifynicely is not granted to verifykeys.`, then `hint: ncly auth grant verifynicely verifykeys`. +- **Key from the variable.** Run `VERIFYNICELY_API_KEY="$fake" just verify step "$run" auth auth status --json`. Exit 78, and stdout is `{"ok":false,"contract_version":1,"keychain":"available","services":[{"service":"verifynicely","configured":true,"source":"env","code":"KEY_NOT_GRANTED","message":"The key of verifynicely is not granted to verifykeys.","hint":"ncly auth grant verifynicely verifykeys","next":["ncly","auth","grant","verifynicely","verifykeys"],"grants":[{"domain":"verifykeys","granted":false,"optional":false}]}]}`: the key is there, and no human granted it to the fixture yet. +- **Login dry run.** Run `just verify step "$run" auth auth login verifynicely --dry-run --json`. Exit 0, and stdout holds `"dry_run":true`, the plan `{"key":"verifynicely","action":"login","effects":{"user_writes":true,"network":false,"paid":false}}`, and the pending check `auth.keychain`. +- **Login.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"action":"login","service":"verifynicely"}`. +- **Key from the keychain.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and stdout is the one of the step Key from the variable, with `"source":"keychain"` instead of `"source":"env"`. +- **Doctor without the grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor auth --json`. Exit 78, stderr is empty, and stdout is `{"ok":false,"contract_version":1,"checks":[{"id":"auth.keychain","component":"auth","status":"pass","message":"The keychain answers."},{"id":"auth.grants","component":"auth","status":"pass","message":"Every grant is in use."},{"id":"auth.verifynicely","component":"auth","status":"fail","code":"KEY_NOT_GRANTED","message":"The key of verifynicely is not granted to verifykeys.","hint":"ncly auth grant verifynicely verifykeys","next":["ncly","auth","grant","verifynicely","verifykeys"]}]}`. +- **Fixture without a grant.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth verifykeys digest --json`. Exit 78, stdout is empty, and stderr is one line, an answer whose first error is `KEY_NOT_GRANTED`, with the hint `ncly auth grant verifynicely verifykeys` and the same command in `next`: the keychain holds the fake key, and still it does not reach the fixture. diff --git a/.agents/skills/verify-nicely/features/auth/terminal.md b/.agents/skills/verify-nicely/features/auth/terminal.md new file mode 100644 index 00000000..392f7348 --- /dev/null +++ b/.agents/skills/verify-nicely/features/auth/terminal.md @@ -0,0 +1,16 @@ +# Terminal steps and cleanup + +Part of [auth](../auth.md). Follow the three guides in order. + +- **Login in a terminal.** Run `just verify tmux "$run" auth --answer 'verifynicely key' "$fake" auth login verifynicely`. Exit 0. Before Enter, the terminal shows the masked field: `┃ verifynicely key`, `┃ It goes to the keychain and never shows.`, `┃ > ` followed by one `*` per character of the fake key, and `enter submit`. After Enter, the field is gone, and the terminal shows only `Stored the key of verifynicely in the keychain.`, with no `Next time:` line. +- **Key from the terminal.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth auth status --json`. Exit 78, and stdout is the one of the step Key from the keychain: the key comes from the keychain again, and the step Revoke took its grant back. +- **Replacement in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' n auth login verifynicely`. Exit 2. The terminal shows `? The keychain already holds a key for verifynicely. Replace it? [y/N] n`, then `error[CONFIRMATION_REQUIRED]: The key of verifynicely stays as it was.`, and no masked field. +- **Ctrl-C at the field.** Run `just verify tmux "$run" auth --answer 'verifynicely key' '^C' auth login verifynicely --force`. Exit 130. The terminal shows only `error[INTERRUPTED]: Stopped by Ctrl-C. Finished work is kept.` and its hint, because the field erased itself. +- **Logout in a terminal.** Run `just verify tmux "$run" auth --answer '[y/N]' y auth logout verifynicely`. Exit 0. The terminal shows `? Remove the key of verifynicely from the keychain? [y/N] y`, then `Removed the key of verifynicely from the keychain.` +- **Doctor without the key.** Run `VERIFYNICELY_API_KEY= just verify step "$run" auth doctor --json`. Exit 78, stderr is empty, and stdout lists `core.config`, then `auth.keychain` and `auth.grants` as a `pass`, then `{"id":"auth.verifynicely","component":"auth","status":"fail","code":"AUTH_MISSING", ...}` with the hint and `next` of the status entry, then the `extensions` checks, each a `pass`. +- **Nothing to remove.** Run `just verify step "$run" auth auth logout verifynicely --force --json`. Exit 2, and stderr ends with an answer whose first error is `NOT_FOUND`, with the hint and `next` `ncly auth status`. +- **Missing service.** Run `just verify step "$run" auth auth login`. Exit 2, stdout is empty, and stderr starts with `error[USAGE_INVALID]: The argument is missing.`, with the hint `ncly auth login `. +- **Login from the form.** Run `just verify tmux "$run" auth --answer 'Service of the key' verifynicely --answer 'verifynicely key' "$fake" auth login`. Exit 0. Before the first Enter, the terminal shows the form for missing values: `┃ service`, `┃ Service of the key, such as deepgram`, and `┃ > verifynicely`. Before the second, it shows the masked field of the step Login in a terminal. After it, both are gone, and the terminal shows `Stored the key of verifynicely in the keychain.`, then `Next time:` and ` ncly auth login verifynicely`, never the key. +- **Login for the cleanup.** Run `printf '%s\n' "$fake" | just verify step "$run" auth auth login verifynicely --stdin --force --json`. Exit 0, so the cleanup has a key to log out. +- **Proof.** Run `grep -c "$fake" "$run/evidence/auth.txt"` after the steps. It prints `0` and exits 1, grep's code for no match: no transcript holds the fake key, not even the one that the terminal steps typed. +- **Cleanup.** Run `just verify cleanup "$run"`, as SKILL.md says. Exit 0, stderr says `logged the fake key of verifynicely out of the keychain`, and `$run/evidence/cleanup.txt` holds an `ncly auth logout verifynicely --force --json` that exits 0, then an `ncly auth status --json` with `"keychain":"available"`, whose entry of `verifynicely` has `"source":"none"` and `"code":"AUTH_MISSING"`. diff --git a/.agents/skills/verify-nicely/features/extensions-skill.md b/.agents/skills/verify-nicely/features/extensions-skill.md index c8a46973..ad85d8ab 100644 --- a/.agents/skills/verify-nicely/features/extensions-skill.md +++ b/.agents/skills/verify-nicely/features/extensions-skill.md @@ -6,6 +6,7 @@ - `template-command` gives the command that creates an extension repository, in the `nicely-extensions` skill, with the VERSION of a development build - `template-install-command` gives the commands that install an extension repository at a release tag, in the `nicely-extensions` skill +- `revision-links` use the release tag for bundled documentation, or main in a development build - `template-remove-command` gives the steps that remove an extension, in the `nicely-extensions` skill ## How to get to it (user POV) @@ -23,6 +24,8 @@ Preconditions: - **Install command.** In the output of `skill view nicely-extensions`, the skill holds the section `## Install an extension`. It quotes the install rule, in a line that starts with `> A user installs a fixed version with`. Its code block holds the lines `git clone --branch TAG https://github.com/OWNER/REPO` and `cd REPO && just install`, and the text names `ncly describe ROOT --json`. - **Remove command.** In the output of `skill view nicely-extensions`, the skill holds the section `## Remove an extension`. It names `ncly doctor extensions --json`, the four files `ncly-SLUG`, `ncly-SLUG.toml`, `ncly-SLUG.locales/`, and `ncly-SLUG.skill/`, `just uninstall`, `NOT_FOUND`, and `GRANT_UNUSED`. +- **Revision links.** In a development or pseudo-version build, each Nicely `blob/` link in `skill view nicely-extensions` uses `main`. For a release-version check, build the same source into `$run/build/ncly` with `go build -ldflags '-X github.com/pascalandy/nicely/internal/cli.version=v0.26.0' -o "$run/build/ncly" ./cmd/ncly`. Run `just verify step "$run" extensions-skill --version`, then `just verify step "$run" extensions-skill skill view nicely-extensions`. Every Nicely `blob/` link now uses `v0.26.0`. Quote both outputs and the build flag; this is a test build, not a published release. + ## Gotchas - The skill comes with the `ncly` of the run, so this feature needs no `prepare` diff --git a/.agents/skills/verify-nicely/features/hello.md b/.agents/skills/verify-nicely/features/hello.md index 7dac906b..33922e33 100644 --- a/.agents/skills/verify-nicely/features/hello.md +++ b/.agents/skills/verify-nicely/features/hello.md @@ -16,10 +16,13 @@ - `hello-form` asks for a missing skill name in the form for missing values on a real terminal, then prints `Next time:` and the full command, and fails with that command as its hint outside a terminal - `hello-doctor` reports where `hello` runs from, and the `bash` that `hello count` needs +- `hello-staged-cleanup` preserves input and previous output while a resistant worker stops and handler staging is removed + ## How to get to it (user POV) - `ncly --help` and `ncly hello --help` - `ncly hello run `, `ncly hello check`, `ncly hello count `, `ncly hello scan ` +- `ncly hello staged --json`, through `just verify stress-cleanup "$run" 10 20000` - `ncly describe hello run` - `ncly skill view hello`, in a pipe and in a terminal - `ncly skill view` without a name, in a terminal and in a pipe @@ -32,7 +35,7 @@ Preconditions: - A new run from `just verify`, whose Doctor step passed - **Discovery.** Run `just verify step "$run" hello --help`. Exit 0, and the `EXTENSIONS` section lists `hello: Greet people` beside `skill`. -- **Help.** Run `just verify step "$run" hello hello --help`. Exit 0, and `COMMANDS` lists `check`, `count`, `pick`, `run`, `say`, `scan`, and `wait`. +- **Help.** Run `just verify step "$run" hello hello --help`. Exit 0, and `COMMANDS` lists `check`, `count`, `pick`, `run`, `say`, `scan`, `staged`, and `wait`. - **Greet as text.** Run `just verify step "$run" hello hello run Ada`. Exit 0, stdout is `Hello, Ada!`, and stderr is empty. - **Greet as JSON.** Run `just verify step "$run" hello hello run Ada --json`. Exit 0, and stdout is `{"ok":true,"contract_version":1,"greeting":"Hello, Ada!"}`. - **Warning.** Run `just verify step "$run" hello hello run --json`. Exit 0, `ok` is `true`, and `warnings` holds the warning `HELLO_UNNAMED`. @@ -51,6 +54,10 @@ Preconditions: - **Proof.** Run `just verify step "$run" hello doctor extensions --json`. Exit 0, and the check `extensions.hello` is a `pass` with the source `extension` and the path `/scratch/home/.local/share/nicely/extensions/ncly-hello`. - **Doctor of hello.** Run `just verify step "$run" hello doctor hello --json`. Exit 0, and the check `hello.bash` is a `pass` whose message names the version of `bash` and its path. +### Handler cleanup after a resistant worker + +Follow [the staged cleanup guide](hello/cleanup.md) for resistant and cooperative workers, their controls, and the repeated production-budget check. + ## Gotchas - `hello check` exits 78 on purpose: its report always fails, and that is the end state to check diff --git a/.agents/skills/verify-nicely/features/hello/cleanup.md b/.agents/skills/verify-nicely/features/hello/cleanup.md new file mode 100644 index 00000000..d4f37d93 --- /dev/null +++ b/.agents/skills/verify-nicely/features/hello/cleanup.md @@ -0,0 +1,32 @@ +# Handler cleanup after a resistant worker + +The fixture command `hello staged ` runs a worker with private staging, then removes staging when the worker returns. +The scenario [staged_cleanup.txtar](../../../../../testdata/extensions/hello/testdata/script/staged_cleanup.txtar) supplies the worker and an independent stage check. +Copy its `worker.sh`, `grandchild.sh`, `check.sh`, input and previous output into the run's scratch folder. +From the checkout root, set `repo="$PWD"` before entering that folder. +In that folder, drive the calls below that name `hello staged`. + +Call the same Verify entry point directly, because `just` starts its recipe at the checkout root: + +```bash +CLEANUP_WORKER=stubborn bash "$repo/.agents/skills/verify-nicely/scripts/verify.sh" step "$run" hello hello staged ./worker.sh --json +``` +After `worker.pid` appears, send SIGINT to the real host, then repeat with SIGTERM in a fresh scratch subfolder. +A process group around the Verify step can deliver the signal while the host reaches workers in their own sessions. +The transcript must show exit 130 with `INTERRUPTED`, or 143 with `TERMINATED`, and retain the `stage` data key. +Both calls must finish within the 15-second cleanup budget plus the 2-second stream drain and scheduling tolerance. +Check that `cleanup.done` exists, no `.ncly-stage-*` remains, both worker PIDs are gone, and input and previous output match their copies. +Also drive the cooperative worker with `CLEANUP_WORKER=cooperative`. +The scenario covers the direct-extension control with the same SDK and no host. +The [stubborn control](../../../../../testdata/script/extension_cancel_stubborn.txtar) proves the final handler deadline and the force-stop of a cleanup child. +Only the direct handler receives the reservation; see [Cancellation](../../../../../docs/north-star/contract/programs-that-ncly-runs/cancellation.md). + +From `$repo`, run `just verify stress-cleanup "$run" 10 20000` on the same clean committed checkout that built the run. +It runs the real public host ten times per signal, with 20,000 files and partial output in each private stage. +The driver counts the files before signalling the host through the worker's ancestry. +Every call records the signal answer, preserved files, stage absence and stopped worker PIDs. +It records deletion and handler cleanup times, plus the margin inside the 2-second reservation, in `evidence/cleanup-stress.txt` and `.json`. +That margin subtracts measured handler cleanup only. `cleanup_ms` excludes writes of `cleanup.json` and `cleanup.done`. +`signal_ms` ends when the Verify step returns, after host exit and transcript capture. +Thus `15000 - signal_ms` is a conservative lower bound on the host's budget margin. +Run Cleanup and quote `cleanup-stress` afterward. A failed run or nonpositive cleanup margin fails the driver. diff --git a/.agents/skills/verify-nicely/features/template.md b/.agents/skills/verify-nicely/features/template.md index 5c9a2ad0..68001c3c 100644 --- a/.agents/skills/verify-nicely/features/template.md +++ b/.agents/skills/verify-nicely/features/template.md @@ -7,6 +7,7 @@ - `template-loop` sends each command of the rendered repository to its `nicely-extension` skill, which builds it in rounds until `just finish` passes - `template-install` renders the template, and installs the rendered repository with `just install` in the scratch data folder +- `template-links` pin documentation to the selected tag - `template-rule-index` counts the rules of the rule index in `just check` of the rendered repository - `template-publication` refuses in `just check` a command that writes a user file with no row in `## Publication` of `spec.md` - `template-acceptance` reads the answer of `probe inspect` in its acceptance case, and refuses a count of 30 lines for 3 @@ -38,6 +39,7 @@ Preconditions: - `uv`, `just`, and a `python3` of 3.9 or later, which answers under the scratch home - **Render and install.** Run `just verify prepare "$run" template`. It exits 0. Its log names `just install` of `probe`, the line `installed /scratch/home/.local/share/nicely/extensions/ncly-probe`, and the `python3` that it puts first in the PATH of the wrapper. Then run `ls "$run/scratch/home/.local/share/nicely/extensions"`: it lists `ncly-probe`, `ncly-probe.locales`, `ncly-probe.skill`, and `ncly-probe.toml`, beside the four files of `hello`. Report the output of `prepare` and of `ls` beside the quote. +- **Links.** Run `rg --hidden 'nicely/blob/' "$run/scratch/ncly-probe" -g '*.md'`. Each link uses `blob/v0.0.0/`. - **Release and install text.** Run `grep -n -A12 '^## Release$' "$run/scratch/ncly-probe/AGENTS.md"` and `grep -n -A3 '^git clone' "$run/scratch/ncly-probe/README.md"`. Both exit 0. `AGENTS.md` names `just release vX.Y.Z` and the release receipt. `README.md` holds the lines `git clone --branch TAG https://example.com/ncly-probe ncly-probe`, `cd ncly-probe`, `just install`, and `ncly describe probe --json`. Report both outputs beside the quote. - **Loop text.** Run `grep -n -A12 '^## Done when$' "$run/scratch/ncly-probe/AGENTS.md"`, then `grep -n '^## Open lines$\|^## Case matrix$' "$run/scratch/ncly-probe/spec.md"`. Both exit 0, and `spec.md` lists both parts. "Done when" names `just finish`, the hosted checks, 5 rounds, and a "full stop" request that runs only after it. Then run `grep -n '^##' "$run/scratch/ncly-probe/.agents/skills/nicely-extension/SKILL.md"`. It lists `## Intake`, `## Acceptance lines`, `## Test files`, `## Rounds`, and `## Readiness review`. Then run `grep -n '^##' "$run/scratch/ncly-probe/.agents/skills/nicely-extension/intake.md"`. It lists `## Intake of a script`, `## Intake of a program`, and `## Evidence of the intake`. Report the four outputs. - **Finish gate.** Run `(cd "$run/scratch/ncly-probe" && just finish)`. Exit 1. Stderr names each `Fill:` line of `vision.md`, `spec.md`, `qa.md`, and `intake.toml` as `:: this line of the form is not filled`, and `review.toml: no readiness review is recorded`. Then run `just records` there. Exit 0, and it names `vision.md`. Report both outputs. diff --git a/.agents/skills/verify-nicely/features/transcript-run.md b/.agents/skills/verify-nicely/features/transcript-run.md index 8651c29d..0b3690af 100644 --- a/.agents/skills/verify-nicely/features/transcript-run.md +++ b/.agents/skills/verify-nicely/features/transcript-run.md @@ -6,6 +6,7 @@ - `transcript-grant` grants the key of `deepgram` to transcript after a typed `y` on a real terminal - `transcript-fake-key` runs one transcription with a fake key: Deepgram refuses it, and the item answers `AUTH_REJECTED` under `RUNTIME`, with its result folder +- `transcript-sign-in` stops a YouTube sign-in refusal with `TRANSCRIPT_DOWNLOAD_FAILED` and the browser-setting hint - `transcript-record` records the run with its failed step, and the same call starts nothing - `transcript-other-call` refuses another URL under the run ID of that run with `USAGE_INVALID`, before anything starts - `transcript-stop` stops a run with Ctrl-C during the real download: exit 130, no process left, and the step pending @@ -28,6 +29,7 @@ In `just verify step "$run" transcript-run `, `transcript-run` names this - **Grant.** Run `just verify tmux "$run" transcript-run --answer '[y/N]' y auth grant deepgram transcript`. Exit 0. Before Enter, the terminal shows `? Grant the key of deepgram to transcript?`, then ` Path ` and the path of `ncly-transcript` in `/scratch/home/.local/share/nicely/extensions/`, then ` [y/N] y`. After Enter, it shows `Granted the key of deepgram to transcript.` - **Fake key.** Run `just verify step "$run" transcript-run transcript run youtube --url 'https://www.youtube.com/watch?v=EIEc43CxIvY' --output "$run/scratch/home/videos" --run-id verify-fake --json`. Exit 1 within a minute, and stdout is empty. The first error of stderr is `RUNTIME`, and the answer holds `"run_id":"verify-fake"`. Its one item has `"ok":false`, `"transcript":null`, and an `output_dir` in `/scratch/home/videos/`, named after the title of the video. The item's one error has the code `AUTH_REJECTED`, the message `Deepgram refused the key.`, and the hint `ncly auth login deepgram`. +- **YouTube sign-in refusal.** YouTube may refuse the download before the fake key reaches Deepgram. In that case, require exit 1 and item code `TRANSCRIPT_DOWNLOAD_FAILED`, with a hint naming the `browser` setting in `[transcript]`. Read `run view verify-fake --json` and require one attempt. Report that login blocks the remaining `AUTH_REJECTED` assertions; do not read personal browser cookies or attempt a browser login for this proof. The stub scenarios cover the final-error classification independently of that live precondition. - **Result folder.** Run `ls -A` on the `output_dir` of Fake key. It prints `meta.txt` alone, since Deepgram answered no transcript. Report its output beside the quote. - **Record.** Run `just verify step "$run" transcript-run run view verify-fake --json`. Exit 0. The run has `"status":"failed"`, `"attempts":1`, and `"tools"` with `"transcript":"1.0.0"` and the version of yt-dlp. Its step `transcribe` has `"effect":"paid"`, `"status":"failed"`, no artifact, and the error `AUTH_REJECTED`. Its `output_dir` is the one of Fake key. - **Same call.** Run the call of Fake key again. Exit 1 within a few seconds, and stderr holds the code `RESUME_UNSAFE`, `"run_id":"verify-fake"`, and the hint `ncly run view verify-fake`, because the audio reached Deepgram once. diff --git a/.agents/skills/verify-nicely/features/transcript.md b/.agents/skills/verify-nicely/features/transcript.md index aaf5cb66..deced922 100644 --- a/.agents/skills/verify-nicely/features/transcript.md +++ b/.agents/skills/verify-nicely/features/transcript.md @@ -10,6 +10,7 @@ - `transcript-doctor` passes `transcript.uv` and `transcript.python3`, the programs that the manifest requires, then `transcript.cookies` - `transcript-cookies` warns when the config sets `browser = "arc"` and no cookie file of Arc's default profile opens, and passes once one does - `transcript-skill` serves the skill installed beside the extension +- `transcript-lock-refusals` creates no run lock when an unnamed execution fails its output or key check - `transcript-refusals` refuses a URL that names no single video and a missing output folder with `USAGE_INVALID`, before the engine is extracted - `transcript-dry-run` runs the engine through `uv` from an empty uv cache and answers the plan as JSON and as text. It creates no record and no result folder @@ -48,10 +49,13 @@ In `just verify step "$run" transcript `, `transcript` names this feature, - **No record.** Run `ls "$run/scratch/home/.local/state/nicely/runs"`, then `ls -A "$run/scratch/home/videos"`. The first fails and the second prints nothing, because a dry run records nothing and creates no result folder. Report both outputs beside the quote. - **Dry run as text.** Run the call of Dry run without `--json`. Exit 0, and stdout holds two lines: `Would transcribe https://www.youtube.com/watch?v=EIEc43CxIvY into a new folder in /scratch/home/videos.`, then `pending auth.deepgram Authentication is checked only during execution.` +- **Unnamed key refusal.** Before the grant of transcript-run, create `$run/scratch/home/.local/state/nicely/runs` and record its empty `ls -A` output. Run `just verify step "$run" transcript transcript run youtube --url 'https://www.youtube.com/watch?v=EIEc43CxIvY' --output "$run/scratch/home/videos" --json`. Exit 78, with `KEY_NOT_GRANTED` and no `run_id`. The wrapper supplies only a fake key, and no grant lets it reach the extension. A second `ls -A` of the runs folder is empty. +- **Unnamed output refusal.** Run the same call with `--output "$run/scratch/home/missing"`. Exit 2, with `USAGE_INVALID` and no `run_id`. The runs folder is still empty. Quote both CLI calls and report all three directory observations; neither call reaches YouTube or Deepgram. + ## Gotchas - Run `prepare` before any `transcript` step. Before it, the wrapper runs the build of `just verify`, without `ncly-transcript`. Its `PATH` may also start with a shim of `uv` that fails under the scratch home - Call `ncly` only through `just verify step` or `just verify tmux`, whose wrapper forces the fake key. Core reads `DEEPGRAM_API_KEY` before the keychain, so another call could bill a real key. Never run `ncly auth login deepgram`: it writes the real keychain, where it would replace a real key -- Every call of `transcript run youtube` here passes `--dry-run`, so none reaches YouTube or Deepgram +- The unnamed output and key refusals execute preparation and stop before the engine or network. The other `transcript run youtube` calls here use `--dry-run` - The dry run reaches PyPI through `uv` only. Nicely's cache in the scratch home holds the engine and the uv cache, and the cleanup removes both - The wrapper keeps the caller's variables, so a `UV_OFFLINE` or `UV_INDEX_URL` of the caller reaches `uv` diff --git a/.agents/skills/verify-nicely/scripts/cleanup-stress.py b/.agents/skills/verify-nicely/scripts/cleanup-stress.py new file mode 100755 index 00000000..eb53f173 --- /dev/null +++ b/.agents/skills/verify-nicely/scripts/cleanup-stress.py @@ -0,0 +1,200 @@ +#!/usr/bin/env python +"""Repeat the real public-host cancellation path with measured private staging.""" + +import json +import os +import re +import signal +import subprocess +import sys +import time +from pathlib import Path + + +def require(condition: object, problem: object) -> None: + if not condition: + raise RuntimeError(problem) + + +def running(pid: int) -> bool: + result = subprocess.run( + ["ps", "-p", str(pid), "-o", "stat="], + text=True, + capture_output=True, + check=False, + ) + status = result.stdout.strip() + return bool(status) and not status.startswith("Z") + + +def parent(pid: int) -> int: + return int( + subprocess.check_output(["ps", "-p", str(pid), "-o", "ppid="], text=True) + ) + + +def drive(run: Path, repo: Path, folder: Path, sig: signal.Signals, files: int) -> dict: + env = dict(os.environ, CLEANUP_WORKER="stubborn", CLEANUP_FILES=str(files)) + program = repo / ".agents/skills/verify-nicely/scripts/verify.sh" + call = subprocess.Popen( + [ + "bash", + str(program), + "step", + str(run), + "cleanup-stress", + "hello", + "staged", + str(folder / "worker.sh"), + "--json", + ], + cwd=folder, + env=env, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, + ) + pids: list[int] = [] + try: + deadline = time.monotonic() + 15 + while not (folder / "worker.pid").exists(): + if call.poll() is not None or time.monotonic() >= deadline: + raise RuntimeError("worker did not become ready") + time.sleep(0.01) + pids = [ + int((folder / name).read_text()) + for name in ["worker.pid", "grandchild.pid"] + ] + stages = list(folder.glob(".ncly-stage-*")) + require( + len(stages) == 1 and len(list(stages[0].iterdir())) == files + 1, + "wrong staging population", + ) + require(stages[0].stat().st_mode & 63 == 0, "staging is not private") + host = parent(parent(pids[0])) + command = subprocess.check_output( + ["ps", "-ww", "-p", str(host), "-o", "command="], text=True + ) + # ps keeps argv's spelling, while Path.resolve follows /var on macOS. + # Try word boundaries so an executable path may also contain spaces. + command = command.strip() + boundaries = [i for i, char in enumerate(command) if char == " "] + [ + len(command) + ] + candidates = [Path(command[:end]) for end in boundaries] + require( + any( + path.is_file() and path.samefile(run / "build/ncly") + for path in candidates + ), + f"worker ancestry expected {run / 'build/ncly'}, got {command!r}", + ) + started = time.monotonic() + os.kill(host, sig) + stdout, stderr = call.communicate(timeout=20) + elapsed = round((time.monotonic() - started) * 1000) + want_exit, want_code = ( + (130, "INTERRUPTED") if sig == signal.SIGINT else (143, "TERMINATED") + ) + require(call.returncode == want_exit, (call.returncode, stdout, stderr)) + answers = [ + json.loads(line) for line in stdout.splitlines() if line.startswith("{") + ] + answer = answers[-1] + require( + not answer["ok"] + and answer["errors"][0]["code"] == want_code + and answer["stage"], + "verification failed: not answer['ok'] and answer['errors'][0]['code'] == want_code and answer['stage']", + ) + require(not list(folder.glob(".ncly-stage-*")), "private staging remains") + require( + (folder / "cleanup.done").read_text() == "done\n", + "handler cleanup did not finish", + ) + require(not any(running(pid) for pid in pids), "a worker survived") + require( + (folder / "input.txt").read_text() == "original input\n", "input changed" + ) + require( + (folder / "output.txt").read_text() == "previous output\n", + "previous output changed", + ) + stats = json.loads((folder / "cleanup.json").read_text()) + require( + all(isinstance(stats[key], int) for key in ["delete_ms", "cleanup_ms"]), + "verification failed: all((isinstance(stats[key], int) for key in ['delete_ms', 'cleanup_ms']))", + ) + require(stats["cleanup_ms"] < 2000, "handler cleanup exhausted its reservation") + return dict( + stats, + signal=sig.name, + files=files, + signal_ms=elapsed, + margin_ms=2000 - stats["cleanup_ms"], + ) + finally: + # These IDs come only from this invocation's private fixture. + # Refuse a surviving worker before cleaning it up, even if the host exited. + for path in [folder / "worker.pid", folder / "grandchild.pid"]: + if path.exists(): + pid = int(path.read_text()) + if pid > 1 and running(pid): + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + if call.poll() is None: + os.killpg(call.pid, signal.SIGKILL) + call.communicate() + + +def main() -> None: + run = Path(sys.argv[1]).resolve() + count = int(sys.argv[2]) if len(sys.argv) > 2 else 10 + files = int(sys.argv[3]) if len(sys.argv) > 3 else 20000 + require(count > 0 and files >= 0, "verification failed: count > 0 and files >= 0") + repo = Path(__file__).resolve().parents[4] + head = subprocess.check_output( + ["git", "-C", str(repo), "rev-parse", "HEAD"], text=True + ).strip() + recorded = (run / "evidence/commit.txt").read_text().splitlines() + require( + recorded == [head[:12]], + "stress requires a clean committed build at the current head", + ) + archive = ( + repo / "testdata/extensions/hello/testdata/script/staged_cleanup.txtar" + ).read_text() + parts = re.split(r"^-- (.+) --\n", archive, flags=re.MULTILINE) + fixtures = dict(zip(parts[1::2], parts[2::2], strict=True)) + results = [] + for sig in [signal.SIGINT, signal.SIGTERM]: + for index in range(1, count + 1): + folder = run / "scratch" / f"stress-{sig.name}-{index:02d}" + folder.mkdir() + for name in ["worker.sh", "grandchild.sh", "input.txt", "output.txt"]: + (folder / name).write_text(fixtures[name]) + result = dict(drive(run, repo, folder, sig, files), run=index) + results.append(result) + text = json.dumps(result, sort_keys=True) + print(text, flush=True) + with (run / "evidence/cleanup-stress.txt").open("a") as evidence: + evidence.write(text + "\n") + summary = { + "head": head, + "runs": len(results), + "files": files, + "max_delete_ms": max(row["delete_ms"] for row in results), + "max_cleanup_ms": max(row["cleanup_ms"] for row in results), + "min_margin_ms": min(row["margin_ms"] for row in results), + } + (run / "evidence/cleanup-stress.json").write_text( + json.dumps(summary, indent=2) + "\n" + ) + print(json.dumps(summary, sort_keys=True), flush=True) + + +if __name__ == "__main__": + main() diff --git a/.agents/skills/verify-nicely/scripts/cleanup.sh b/.agents/skills/verify-nicely/scripts/cleanup.sh index 5d0efbfc..20ca3e51 100644 --- a/.agents/skills/verify-nicely/scripts/cleanup.sh +++ b/.agents/skills/verify-nicely/scripts/cleanup.sh @@ -6,9 +6,9 @@ # verify.sh cleanup remove the scratch folder, keep the evidence # verify.sh prune --keep remove the runs before the newest , once cleaned up -# logout_fake_key logs the fake key of the auth feature out of the real -# keychain, then checks that status no longer finds it. Both calls go to the -# transcript evidence/cleanup.txt. It fails when the key may remain. +# logout_fake_key removes the auth feature's fake key and checks its absence. +# Linux uses the private keyring. macOS uses the login keychain. +# Both calls go to evidence/cleanup.txt. It fails when the key may remain. logout_fake_key() { local run="$1" code=0 answer outcome # The key must come from the keychain, never from the caller's variable. @@ -41,11 +41,20 @@ logout_fake_key() { cleanup() { local run folder run="$(run_folder "${1:-}")" - # The auth feature leaves its fake key in the real keychain, after a failed - # run too. The scratch folder stays until the key is gone, so that a later - # cleanup can try again. - if [[ -f "$run/auth-prepared" && -d "$run/scratch" ]]; then - logout_fake_key "$run" || die "kept the scratch folder of $run: run cleanup again once the keychain answers" + # Auth can leave its fake key after a failed run. + # macOS keeps scratch for another logout attempt. + # On Linux, a failed logout calls private-keyring cleanup. + # Later cleanup discards a stopped private keyring with scratch. + if [[ -f "$run/auth-prepared" && -d "$run/scratch" && ! -f "$run/scratch/keyring/stopped" ]]; then + if ! logout_fake_key "$run"; then + if [[ -d "$run/scratch/keyring" ]]; then + bash "$script_dir/keyring.sh" stop "$run/scratch/keyring" | tee -a "$run/evidence/keyring.txt" || die "kept $run: private keyring cleanup failed" + fi + die "kept the scratch folder of $run: inspect the failed auth cleanup, then run cleanup again" + fi + fi + if [[ -d "$run/scratch/keyring" ]]; then + bash "$script_dir/keyring.sh" stop "$run/scratch/keyring" | tee -a "$run/evidence/keyring.txt" || die "kept the scratch folder of $run: private keyring cleanup failed" fi # Go can put read-only module folders under the scratch home. for folder in "$run/scratch" "$run/build"; do diff --git a/.agents/skills/verify-nicely/scripts/keyring.sh b/.agents/skills/verify-nicely/scripts/keyring.sh new file mode 100644 index 00000000..3e70d606 --- /dev/null +++ b/.agents/skills/verify-nicely/scripts/keyring.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +# A Linux verify run owns one D-Bus session and unlocked Secret Service. +# Its private environment never inherits the caller's bus or keyring control. +set -Eeuo pipefail + +fail() { + echo "verify-nicely: $*" >&2 + exit 1 +} + +require_programs() { + local program + for program in dbus-run-session gnome-keyring-daemon; do + command -v "$program" >/dev/null || fail "$program is missing: required for the private Linux keyring" + done +} + +start_session() { + local state="$1" home="$2" runtime self i + require_programs + mkdir -p "$state" + # Unix socket paths are short, even when the run folder has a long name. + runtime="$(mktemp -d "${TMPDIR:-/tmp}/nk.XXXXXXXX")" + chmod 700 "$runtime" "$state" + printf '%s\n' "$runtime" >"$state/runtime" + : >"$runtime/.verify-nicely-keyring" + self="$(cd "$(dirname "$0")" && pwd -P)/keyring.sh" + ( + code=0 + env -i PATH="$PATH" HOME="$home" XDG_CONFIG_HOME="$home/.config" \ + XDG_DATA_HOME="$home/.local/share" XDG_STATE_HOME="$home/.local/state" \ + XDG_CACHE_HOME="$home/.cache" XDG_RUNTIME_DIR="$runtime" \ + dbus-run-session -- bash "$self" serve "$state" "$runtime" || code=$? + # An abandoned run can lose its state before the private bus exits. + if [[ -f "$runtime/.verify-nicely-keyring" ]]; then rm -rf "${runtime:?}"; fi + if [[ -d "$state" ]]; then printf '%s\n' "$code" >"$state/stopped"; fi + ) >"$state/session.log" 2>&1 "$state/stop" + fail "private keyring did not start; see $state/session.log; run cleanup before removing scratch" +} + +serve() { + local state="$1" runtime="$2" daemon i started=$SECONDS + # Only a public fixture password protects this disposable keyring. + printf 'verify-nicely-private\n' | gnome-keyring-daemon --foreground --unlock \ + --components=secrets --control-directory="$runtime/keyring" >"$state/daemon.log" 2>&1 & + daemon=$! + printf '%s\n' "$daemon" >"$state/daemon-pid" + trap 'kill "$daemon" 2>/dev/null || true; wait "$daemon" 2>/dev/null || true' EXIT + trap 'exit 1' TERM INT + for ((i = 0; i < 100; i++)); do + kill -0 "$daemon" 2>/dev/null || fail "private gnome-keyring-daemon stopped" + if [[ -S "$runtime/keyring/control" ]]; then + gnome-keyring-daemon --start --components=secrets --control-directory="$runtime/keyring" >/dev/null + printf '%s\n' "$DBUS_SESSION_BUS_ADDRESS" >"$state/address" + break + fi + sleep 0.1 + done + [[ -s "$state/address" ]] || fail "private keyring control socket did not start" + while [[ -d "$state" && -f "$runtime/.verify-nicely-keyring" && ! -f "$state/stop" ]] && ((SECONDS - started < 21600)); do + kill -0 "$daemon" 2>/dev/null || fail "private gnome-keyring-daemon stopped" + sleep 1 + done + # Exit before the EXIT trap loses this function's local daemon PID. + exit 0 +} + +stop_session() { + local state="$1" runtime i + [[ -d "$state" ]] || return 0 + : >"$state/stop" + for ((i = 0; i < 100; i++)); do + if [[ -f "$state/stopped" ]]; then + runtime="$(cat "$state/runtime")" + [[ -n "$runtime" ]] || fail "private keyring runtime path is missing" + if [[ -e "$runtime" ]]; then + [[ -f "$runtime/.verify-nicely-keyring" ]] || fail "private keyring runtime marker is missing" + rm -rf "${runtime:?}" + fi + echo "private Linux keyring and D-Bus session stopped" + return 0 + fi + sleep 0.1 + done + fail "private keyring did not stop; kept $state and its runtime" +} + +case "${1:-}" in +check) require_programs ;; +start) start_session "$2" "$3" ;; +serve) serve "$2" "$3" ;; +stop) stop_session "$2" ;; +*) fail "usage: keyring.sh check|start |serve |stop " ;; +esac diff --git a/.agents/skills/verify-nicely/scripts/tmux.sh b/.agents/skills/verify-nicely/scripts/tmux.sh index feed01bd..eb4dc731 100644 --- a/.agents/skills/verify-nicely/scripts/tmux.sh +++ b/.agents/skills/verify-nicely/scripts/tmux.sh @@ -70,9 +70,9 @@ tmux_step() { # stays open until kill-server, so the capture holds only what ncly showed. # ncly gets only TERM, which tmux sets, and TMUX, beside what the wrapper # sets, because a variable of the caller, such as the NO_COLOR=1 that agents - # set or WT_SESSION, can skip the detection that asks tmux. The variables - # that reach the session bus of the user's keychain on Linux, which the - # pane gets from the caller, detect nothing, so they pass too. + # set or WT_SESSION, can skip the detection that asks tmux. + # The pane passes caller bus variables too. The Linux wrapper replaces them + # with this run's private D-Bus session before ncly starts. # shellcheck disable=SC2016 # the pane's sh expands these tmux -L "$pane_server" -f /dev/null new-session -d -s verify -x 200 -y 50 /bin/sh -c ' while [ ! -e "$1" ]; do sleep 0.1; done diff --git a/.agents/skills/verify-nicely/scripts/verify.sh b/.agents/skills/verify-nicely/scripts/verify.sh index de9f1364..156b9d6c 100755 --- a/.agents/skills/verify-nicely/scripts/verify.sh +++ b/.agents/skills/verify-nicely/scripts/verify.sh @@ -14,6 +14,7 @@ # server does not answer, type each answer once # its prompt shows, and record it the same way # verify.sh quote [] print transcripts with placeholders for home paths +# verify.sh stress-cleanup [count] [files] repeat real public cancellation with private staging # verify.sh cleanup remove the scratch folder, keep the evidence # verify.sh prune --keep remove the runs before the newest , once cleaned up set -Eeuo pipefail @@ -31,10 +32,10 @@ log() { echo "verify-nicely: $*" >&2; } # usage prints the commands of the header: on stdout for help, on stderr with exit 2 otherwise. usage() { if [[ "${1:-}" == help ]]; then - sed -n '6,18p' "$0" | sed 's/^# \{0,1\}//' + sed -n '6,19p' "$0" | sed 's/^# \{0,1\}//' exit 0 fi - sed -n '6,18p' "$0" | sed 's/^# \{0,1\}//' >&2 + sed -n '6,19p' "$0" | sed 's/^# \{0,1\}//' >&2 exit 2 } @@ -100,8 +101,22 @@ XDG_CACHE_HOME='$home/.cache' PATH='$path' export HOME XDG_CONFIG_HOME XDG_DATA_HOME XDG_STATE_HOME XDG_CACHE_HOME PATH unset NCLY_CONFIG -exec '$binary' "\$@" EOF + if [[ "$(uname -s)" == Linux ]]; then + cat >>"$wrapper" <&2 + exit 1 +fi +DBUS_SESSION_BUS_ADDRESS=\$(cat "\$state/address") +XDG_RUNTIME_DIR=\$(cat "\$state/runtime") +GNOME_KEYRING_CONTROL="\$XDG_RUNTIME_DIR/keyring" +export DBUS_SESSION_BUS_ADDRESS XDG_RUNTIME_DIR GNOME_KEYRING_CONTROL +unset DBUS_SESSION_BUS_PID DBUS_SESSION_BUS_WINDOWID DBUS_STARTER_ADDRESS DBUS_STARTER_BUS_TYPE +EOF + fi + printf 'exec '\''%s'\'' "$@"\n' "$binary" >>"$wrapper" chmod +x "$wrapper" } @@ -120,6 +135,9 @@ stamp() { start() { local repo run stamp home path command -v go >/dev/null || die "go is missing: install it from https://go.dev/dl/" + if [[ "$(uname -s)" == Linux ]]; then + bash "$script_dir/keyring.sh" check + fi repo="$(git -C "$script_dir" rev-parse --show-toplevel)" || die "run it from a checkout of nicely" stamp="$(date +%Y%m%d-%H%M%S)-$$" run="$(runs_root)/$stamp" @@ -132,6 +150,9 @@ start() { install_hello "$repo" "$home/.local/share/nicely/extensions" path="$(clean_path)" write_wrapper "$run/scratch/bin/ncly" "$run/build/ncly" "$home" "$path" + if [[ "$(uname -s)" == Linux ]]; then + bash "$script_dir/keyring.sh" start "$run/scratch/keyring" "$home" + fi git -C "$repo" rev-parse --short=12 HEAD >"$run/evidence/commit.txt" if [[ -n "$(git -C "$repo" status --porcelain)" ]]; then echo "dirty" >>"$run/evidence/commit.txt" @@ -194,6 +215,11 @@ start) start ;; prepare) prepare "$@" ;; step) step "$@" ;; tmux) tmux_step "$@" ;; +stress-cleanup) + run="$(run_folder "${1:-}")" + shift + exec uv run --no-project --python 3.12 "$script_dir/cleanup-stress.py" "$run" "$@" + ;; quote) quote "$@" ;; cleanup) cleanup "$@" ;; prune) prune "$@" ;; diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 262e7814..685d87e3 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -41,12 +41,13 @@ jobs: # The tests of tools/cmd/release-artifacts run ruby -c on the Homebrew formula, # and preflight-test.sh runs the real sqlite3 on a fake TCC.db (#371). + # The Linux Verify integration needs a private D-Bus and Secret Service. - name: Install check tools shell: bash run: | set -Eeuo pipefail sudo apt-get update - sudo apt-get install --yes ruby sqlite3 + sudo apt-get install --yes ruby sqlite3 dbus gnome-keyring curl --fail --silent --show-error --location \ https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz \ --output "${RUNNER_TEMP}/gitleaks.tar.gz" diff --git a/README.md b/README.md index a9055d6f..86560e3a 100644 --- a/README.md +++ b/README.md @@ -23,43 +23,24 @@ The current build brings: - The `agent` extension, which runs one trusted task through Codex and records it - The `transcript` extension, which transcribes one YouTube video with Deepgram, records the run, replays it, and stops cleanly on Ctrl-C - `ncly run list` and `ncly run view`, which read the records of the runs -- The extension template, which creates the repository of an extension outside Nicely, with its tests and its recipes. Its `AGENTS.md` explains how to add a first command, remove the example, and release a version. Its `nicely-extension` skill builds each command in rounds, until `just finish` passes. `just finish` reads the intake and the reviews only in the typed records `intake.toml` and `review.toml`. An extension of an older template adopts `just finish` through `just update`, and its first build starts after the update commit. Its `just check` counts the rules of its rule index that wait for a check. Its `spec.md` declares each command that writes a user file. The test kit proves that a dry run predicts its run, and on Linux, a run without a hard link. Each acceptance case reads the output with a reader that is not the wrapped program, and its bad variant must fail. Its `README.md` tells a user how to install a release +- The [extension template](README/extension-template.md), which creates an extension repository with its tests and recipes - SDK checks that explain broken manifest rules and missing catalog IDs in English, with their repository file names +YouTube sign-in refusals stop automatic retries with `TRANSCRIPT_DOWNLOAD_FAILED`. +Their hint names the transcript `browser` setting. +Output or key refusals leave no empty run-lock files when `--run-id` is omitted. + Extension manifests declare errors and warnings with optional command scopes, execution modes, and coverage exemptions linked to issues. `extension.Check` rejects invalid scopes with TOML line and column positions. The [test kit](docs/north-star/contract/test-kit.md#declared-codes) requires each declared code in every selected command and mode. It lists coverage exemptions for the UAT report. +Accepted privacy limits use [a declared case](docs/north-star/contract/acceptance-cases.md#accepted-privacy-limits), with `reads -case '' -limit ` and an independent reader. -[The current build](docs/status.md) describes each command. The commands below are planned work. - -## Planned commands - -Nicely is a small core that extensions add commands to, in the spirit of Pi. -Core brings `describe`, `doctor`, `auth`, and `run`. It bundles `skill`, which agents use to learn Nicely. -External extensions supply other commands under the [extension contract](docs/north-star/contract/extensions.md). - -The first `agent` command runs one trusted task through Codex. The first `transcript` command transcribes one explicitly selected YouTube video. A checkout installs the extensions with `just install`. Each capability starts with its smallest useful path; additional harnesses, transcript summaries, and taps remain in the [backlog](docs/backlog/README.md). +[The current build](docs/status.md) describes each command. -Named extensions share domain help, completion, and describe, such as `video convert` and `video subtitle`. -Read their skills by slug, such as `ncly skill view video-convert`. -Configuration, doctor components, and key grants use each extension's slug independently. -For example, `ncly doctor video-subtitle` checks that extension, and `ncly auth grant deepgram video-subtitle` grants only its key access. -Existing whole-domain grants keep working without migration. -The template creates named repositories with `--data domain=video --data name=convert`, such as `ncly-video-convert`. -Omit `name` for a whole-domain repository. -Both use the SDK test kit and the same update recipe. -The kit runs manifest examples with scratch fixtures and checks each answer. -It uses dry runs where available and prints skip reasons in plain `just check` output. -`just check` also runs the [analyzers](docs/north-star/contract/analyzers.md) of the pinned SDK, such as the refusal of `io.Discard` in a `runner.Program`. -The kit, `just signoff`, `just release`, and `just acceptance` run in [owned runs](docs/north-star/contract/owned-runs.md), without settings of the caller such as `NCLY_CONFIG`. -`just verify-models` checks the TLA+ models of `verification/tlc.json` with a pinned TLC, whose SHA-256 it checks before Java starts. `just check` reads only their [record](docs/north-star/contract/formal-models.md), with no Java and no network. -`just update VERSION` runs the target version's updater. -Older repositories use the bootstrap in [Update an extension](docs/north-star/contract/extension-repository.md#update-an-extension). -Further work lives in [docs hub](docs/backlog/XT-104-docs-hub.md), [Canadian French](docs/backlog/COR-102-canadian-french.md), -[Markdown view](docs/backlog/XT-103-markdown-view.md), and [video and image jobs](docs/backlog/XT-602-video-image-jobs.md). +## Commands and extensions -In a terminal, a form asks a human for each missing value. Scripts and agents set `NCLY_NO_INPUT=1` to prevent questions, including in a pseudo-terminal, and use `--json` or `NCLY_JSON=1` for machine output. Commands group actions by domain, such as `ncly transcript run youtube`. +Read the [commands and extension workflow](README/commands.md). ## Let your agent find it @@ -97,6 +78,13 @@ From a checkout, with Go and `just`, run `just install`. It builds `ncly` into ` An external extension, such as `ncly-video-convert`, installs from its repository at a release tag, with Go and `just`. [Release and install](docs/north-star/contract/extensions.md#release-and-install) gives the commands, and `ncly skill view nicely-extensions` gives them to an agent, with the steps that remove an extension. +## Cancellation + +The host force-stops resistant descendants after 13 seconds, with a nominal two-second reservation for handler cleanup. +Force-stop and exit overhead use part of that reservation. Cleanup children can be force-stopped too. +An unfinished handler stops at the 15-second deadline and can leave private staging behind. +The signal answer then omits the handler's result data. + ## Privacy Nicely sends no telemetry. It contacts a service only when a command you run needs it, such as Deepgram for `ncly transcript`. @@ -105,6 +93,9 @@ Nicely sends no telemetry. It contacts a service only when a command you run nee Issues and pull requests are welcome. [AGENTS.md](AGENTS.md) explains how changes are made. +On Linux, `just verify` uses a private D-Bus session and keyring for every step of its scratch run. +It needs `dbus-run-session` and `gnome-keyring-daemon`; a missing prerequisite stops the run. + ## License MIT. See [LICENSE](LICENSE). diff --git a/README/commands.md b/README/commands.md new file mode 100644 index 00000000..29346a7f --- /dev/null +++ b/README/commands.md @@ -0,0 +1,28 @@ +# Commands and extensions + +Nicely is a small core that extensions add commands to, in the spirit of Pi. +Core brings `describe`, `doctor`, `auth`, and `run`. It bundles `skill`, which agents use to learn Nicely. +External extensions supply other commands under the [extension contract](../docs/north-star/contract/extensions.md). + +The first `agent` command runs one trusted task through Codex. The first `transcript` command transcribes one explicitly selected YouTube video. A checkout installs the extensions with `just install`. Each capability starts with its smallest useful path; additional harnesses, transcript summaries, and taps remain in the [backlog](../docs/backlog/README.md). + +Named extensions share domain help, completion, and describe, such as `video convert` and `video subtitle`. +Read their skills by slug, such as `ncly skill view video-convert`. +Configuration, doctor components, and key grants use each extension's slug independently. +For example, `ncly doctor video-subtitle` checks that extension, and `ncly auth grant deepgram video-subtitle` grants only its key access. +Existing whole-domain grants keep working without migration. +The template creates named repositories with `--data domain=video --data name=convert`, such as `ncly-video-convert`. +Omit `name` for a whole-domain repository. +Both use the SDK test kit and the same update recipe. +The kit runs manifest examples with scratch fixtures and checks each answer. +It uses dry runs where available and prints skip reasons in plain `just check` output. +`just check` also runs the [analyzers](../docs/north-star/contract/analyzers.md) of the pinned SDK, such as the refusal of `io.Discard` in a `runner.Program`. +The kit, `just signoff`, `just release`, and `just acceptance` run in [owned runs](../docs/north-star/contract/owned-runs.md), without settings of the caller such as `NCLY_CONFIG`. +`just verify-models` checks the TLA+ models of `verification/tlc.json` with a pinned TLC, whose SHA-256 it checks before Java starts. `just check` reads only their [record](../docs/north-star/contract/formal-models.md), with no Java and no network. +`just update VERSION` runs the target version's updater. +It removes the caller's `GIT_*` settings from update programs, including Copier's Git calls, so a hook cannot redirect the update to another repository. +Older repositories use the bootstrap in [Update an extension](../docs/north-star/contract/extension-repository.md#update-an-extension). +Further work lives in [docs hub](../docs/backlog/XT-104-docs-hub.md), [Canadian French](../docs/backlog/COR-102-canadian-french.md), +[Markdown view](../docs/backlog/XT-103-markdown-view.md), and [video and image jobs](../docs/backlog/XT-602-video-image-jobs.md). + +In a terminal, a form asks a human for each missing value. Scripts and agents set `NCLY_NO_INPUT=1` to prevent questions, including in a pseudo-terminal, and use `--json` or `NCLY_JSON=1` for machine output. Commands group actions by domain, such as `ncly transcript run youtube`. diff --git a/README/extension-template.md b/README/extension-template.md new file mode 100644 index 00000000..3060a643 --- /dev/null +++ b/README/extension-template.md @@ -0,0 +1,6 @@ +# Extension template + +The extension template creates the repository of an extension outside Nicely, with its tests and recipes. Its `AGENTS.md` explains how to add a first command, remove the example, and release a version. Its `nicely-extension` skill builds each command in rounds, until `just finish` passes. `just finish` reads the intake and the reviews only in the typed records `intake.toml` and `review.toml`. An extension of an older template adopts `just finish` through `just update`, and its first build starts after the update commit. Its `just check` counts the rules of its rule index that wait for a check. Its `spec.md` declares each command that writes a user file. The test kit proves that a dry run predicts its run, and on Linux, a run without a hard link. Each acceptance case reads the output with a reader that is not the wrapped program, and its bad variant must fail. Its `README.md` tells a user how to install a release. + +The template's documentation links use its selected revision. +The bundled `nicely-extensions` skill links to the running release's documentation, or `main` for a development build. diff --git a/cmd/ncly/keyring_lifecycle_linux_test.go b/cmd/ncly/keyring_lifecycle_linux_test.go new file mode 100644 index 00000000..79adfdf5 --- /dev/null +++ b/cmd/ncly/keyring_lifecycle_linux_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// A private session stops cleanly on request and ends when its owning state +// or runtime marker disappears. +func TestPrivateKeyringLifecycle(t *testing.T) { + for _, name := range []string{"dbus-run-session", "gnome-keyring-daemon"} { + if _, err := exec.LookPath(name); err != nil { + t.Skipf("%s is required for the private Linux keyring test", name) + } + } + helper, err := filepath.Abs(filepath.Join("..", "..", ".agents", "skills", "verify-nicely", "scripts", "keyring.sh")) + if err != nil { + t.Fatal(err) + } + for _, ending := range []string{"normal stop", "state", "runtime marker"} { + t.Run(ending, func(t *testing.T) { + root := t.TempDir() + // Secret Service control sockets must fit Unix's short path limit. + runtimeRoot, err := os.MkdirTemp(os.TempDir(), "ka-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.RemoveAll(runtimeRoot); err != nil { + t.Error(err) + } + }) + state, home := filepath.Join(root, "state"), filepath.Join(root, "home") + if err := os.Mkdir(home, 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("TMPDIR", runtimeRoot) + call := func(verb string, args ...string) error { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "bash", append([]string{helper, verb}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Logf("private keyring %s: %v\n%s", verb, err, out) + } + return err + } + if err := call("start", state, home); err != nil { + _ = call("stop", state) + t.Fatal(err) + } + read := func(name string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(state, name)) + if err != nil { + t.Fatal(err) + } + return strings.TrimSpace(string(data)) + } + pid, err := strconv.Atoi(read("daemon-pid")) + if err != nil || pid <= 1 { + t.Fatalf("invalid private daemon PID: %v", err) + } + runtime := read("runtime") + if !strings.HasPrefix(runtime, runtimeRoot+string(os.PathSeparator)) { + t.Fatalf("runtime %q escaped the private test folder", runtime) + } + marker := filepath.Join(runtime, ".verify-nicely-keyring") + t.Cleanup(func() { + // Restore only our own moved state/marker so the old helper can + // stop after the failing probe. Never leave the regression daemon. + if _, err := os.Stat(state + ".moved"); err == nil { + if err := os.Rename(state+".moved", state); err != nil { + t.Error(err) + } + } + if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) { + if _, err := os.Stat(runtime); os.IsNotExist(err) { + return + } + } + if err := os.WriteFile(marker, nil, 0o600); err != nil { + t.Error(err) + } + if err := call("stop", state); err != nil { + t.Error(err) + } + }) + if ending == "normal stop" { + if err := call("stop", state); err != nil { + t.Fatal(err) + } + if code := read("stopped"); code != "0" { + t.Errorf("normal private session exited %s, want 0", code) + } + if log := read("session.log"); strings.Contains(log, "unbound variable") { + t.Errorf("normal private stop has a shell failure: %s", log) + } + if !errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) { + t.Error("private daemon survived normal stop") + } + if _, err := os.Stat(runtime); !os.IsNotExist(err) { + t.Errorf("private runtime remains after normal stop: %v", err) + } + if err := call("stop", state); err != nil { + t.Fatal(err) + } + if code := read("stopped"); code != "0" { + t.Errorf("repeated stop changed the clean session status: %s", code) + } + return + } + if ending == "state" { + err = os.Rename(state, state+".moved") + } else { + err = os.Remove(marker) + } + if err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(5 * time.Second) + for { + _, ended := os.Stat(filepath.Join(state, "stopped")) + finished := ended == nil + if ending == "state" { + _, err := os.Stat(runtime) + finished = os.IsNotExist(err) + } + if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) && finished { + break + } + if time.Now().After(deadline) { + t.Fatal("private daemon survived the loss of its owning " + ending) + } + time.Sleep(20 * time.Millisecond) + } + }) + } +} diff --git a/cmd/ncly/keyring_linux_test.go b/cmd/ncly/keyring_linux_test.go new file mode 100644 index 00000000..9a05b591 --- /dev/null +++ b/cmd/ncly/keyring_linux_test.go @@ -0,0 +1,163 @@ +package main + +import ( + "bytes" + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// This drives the shipped binary, not the test binary's fake keychain. Its +// caller bus is deliberately unusable, so the old wrapper cannot reach a real +// user's session during the failing regression. +func TestPrivateLinuxKeyring(t *testing.T) { + for _, name := range []string{"dbus-run-session", "gnome-keyring-daemon"} { + if _, err := exec.LookPath(name); err != nil { + t.Skipf("%s is required for the private Linux keyring test", name) + } + } + repo, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatal(err) + } + script := filepath.Join(repo, ".agents", "skills", "verify-nicely", "scripts", "verify.sh") + root := t.TempDir() + home := filepath.Join(root, "home") + if err := os.MkdirAll(home, 0o700); err != nil { + t.Fatal(err) + } + // Preserve the trusted compiler caches while all runtime state stays in + // scratch. The wrapper must replace this invalid bus for every CLI step. + for _, name := range []string{"GOCACHE", "GOMODCACHE"} { + out, err := exec.Command("go", "env", name).Output() + if err != nil { + t.Fatal(err) + } + t.Setenv(name, strings.TrimSpace(string(out))) + } + for name, value := range map[string]string{ + "HOME": home, "XDG_CACHE_HOME": filepath.Join(home, ".cache"), + "XDG_CONFIG_HOME": filepath.Join(home, ".config"), "XDG_DATA_HOME": filepath.Join(home, ".local", "share"), + "XDG_STATE_HOME": filepath.Join(home, ".local", "state"), "XDG_RUNTIME_DIR": filepath.Join(root, "caller-runtime"), + "DBUS_SESSION_BUS_ADDRESS": "unix:path=" + filepath.Join(root, "no-caller-bus"), + "DBUS_SESSION_BUS_PID": "", "GNOME_KEYRING_CONTROL": "", "VERIFYNICELY_API_KEY": "", + } { + t.Setenv(name, value) + } + call := func(input string, want int, args ...string) string { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "bash", append([]string{script}, args...)...) + cmd.Dir = repo + cmd.Stdin = strings.NewReader(input) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + code := 0 + if err != nil { + code = -1 + var exit *exec.ExitError + if errors.As(err, &exit) { + code = exit.ExitCode() + } + } + if code != want { + t.Fatalf("verify %v exit %d, want %d\n%s\n%s", args, code, want, &stdout, &stderr) + } + return stdout.String() + } + run := strings.TrimSpace(call("", 0, "start")) + cleaned := false + t.Cleanup(func() { + if !cleaned { + call("", 0, "cleanup", run) + } + }) + call("", 0, "prepare", run, "auth") + const fake = "private-linux-keyring-fixture" + call(fake+"\n", 0, "step", run, "auth", "auth", "login", "verifynicely", "--stdin", "--force", "--json") + status := call("", 78, "step", run, "auth", "auth", "status", "--json") + if !strings.Contains(status, `"keychain":"available"`) || !strings.Contains(status, `"source":"keychain"`) { + t.Fatalf("stored key is not read from the private keyring:\n%s", status) + } + call("", 0, "step", run, "auth", "auth", "logout", "verifynicely", "--force", "--json") + status = call("", 78, "step", run, "auth", "auth", "status", "--json") + if !strings.Contains(status, `"source":"none"`) || !strings.Contains(status, `"code":"AUTH_MISSING"`) { + t.Fatalf("logout did not remove the key:\n%s", status) + } + // Leave a fake key for the real cleanup procedure to remove and prove gone. + call(fake+"\n", 0, "step", run, "auth", "auth", "login", "verifynicely", "--stdin", "--force", "--json") + pidData, err := os.ReadFile(filepath.Join(run, "scratch", "keyring", "daemon-pid")) + if err != nil { + t.Fatal(err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(pidData))) + if err != nil || pid <= 0 { + t.Fatalf("invalid private daemon PID: %v", err) + } + call("", 0, "cleanup", run) + cleaned = true + if err := syscall.Kill(pid, 0); !errors.Is(err, syscall.ESRCH) { + t.Errorf("private keyring daemon still exists after cleanup: %v", err) + } + for _, name := range []string{"scratch", "build"} { + if _, err := os.Stat(filepath.Join(run, name)); !os.IsNotExist(err) { + t.Errorf("cleanup kept %s: %v", name, err) + } + } + for _, feature := range []string{"auth", "cleanup"} { + quoted := call("", 0, "quote", run, feature) + if strings.Contains(quoted, fake) { + t.Fatal("a transcript printed the fake key") + } + } +} + +func TestPrivateKeyringMissingProgram(t *testing.T) { + bash, err := exec.LookPath("bash") + if err != nil { + t.Fatal(err) + } + script, err := filepath.Abs(filepath.Join("..", "..", ".agents", "skills", "verify-nicely", "scripts", "verify.sh")) + if err != nil { + t.Fatal(err) + } + for _, missing := range []string{"dbus-run-session", "gnome-keyring-daemon"} { + t.Run(missing, func(t *testing.T) { + bin := t.TempDir() + for _, name := range []string{"bash", "dirname", "uname"} { + path, err := exec.LookPath(name) + if err != nil { + t.Fatal(err) + } + if err := os.Symlink(path, filepath.Join(bin, name)); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(bin, "go"), []byte("#!/bin/sh\nexit 99\n"), 0o700); err != nil { + t.Fatal(err) + } + for _, name := range []string{"dbus-run-session", "gnome-keyring-daemon"} { + if name != missing { + if err := os.WriteFile(filepath.Join(bin, name), []byte("#!/bin/sh\nexit 99\n"), 0o700); err != nil { + t.Fatal(err) + } + } + } + cmd := exec.Command(bash, script, "start") + cmd.Env = []string{"PATH=" + bin} + out, err := cmd.CombinedOutput() + if err == nil || !strings.Contains(string(out), missing+" is missing") { + t.Fatalf("missing %s did not fail with its name: %v\n%s", missing, err, out) + } + }) + } +} diff --git a/docs/backlog/COR-629-os-trash.md b/docs/backlog/COR-629-os-trash.md new file mode 100644 index 00000000..0fab9584 --- /dev/null +++ b/docs/backlog/COR-629-os-trash.md @@ -0,0 +1,13 @@ +--- +id: COR-629 +core: "OS trash" +state: "Idea from #447. Revisit with the first command that deletes a user file" +--- + +# COR-629 OS trash + +The SDK would send deleted user files to the OS trash on macOS and Linux. +No user command deletes a file today. + +The first such command must add this SDK support and prove that the deleted file can be recovered. +[Principle 4](../north-star/principles.md) records the condition. diff --git a/docs/backlog/COR-630-core-diagnostics.md b/docs/backlog/COR-630-core-diagnostics.md new file mode 100644 index 00000000..7bafddce --- /dev/null +++ b/docs/backlog/COR-630-core-diagnostics.md @@ -0,0 +1,16 @@ +--- +id: COR-630 +core: "core diagnostics" +state: "Idea from #447. Revisit with the first core command that needs debug or verbose details" +--- + +# COR-630 core diagnostics + +The SDK would let core commands add verbose step details and debug diagnostics on stderr. +Core adds neither today. +The current `--verbose` help entry says `Add step details on stderr`, although core adds none. +Revisit that catalog entry with the shared support. +An extension can already add diagnostics when it needs them, as [Global flags](../north-star/contract/global-flags.md) describes for transcript. + +The first core command that needs these details must add shared SDK support and prove its output. +Keys must stay out of diagnostics. diff --git a/docs/backlog/COR-631-tool-credit.md b/docs/backlog/COR-631-tool-credit.md new file mode 100644 index 00000000..92d3cbc5 --- /dev/null +++ b/docs/backlog/COR-631-tool-credit.md @@ -0,0 +1,12 @@ +--- +id: COR-631 +core: "tool credit" +state: "Idea from #447. Revisit with the next wrapping command" +--- + +# COR-631 tool credit + +The [wrap helper](../../sdk/wrap/wrap.go) would declare the wrapped tool so that command help names it. +The next wrapping command must add this support to `sdk/wrap` and prove the credit in its help. + +[Principle 10](../north-star/principles.md) records the condition. diff --git a/docs/north-star/contract/acceptance-cases.md b/docs/north-star/contract/acceptance-cases.md index 2a058639..e5844a79 100644 --- a/docs/north-star/contract/acceptance-cases.md +++ b/docs/north-star/contract/acceptance-cases.md @@ -22,14 +22,15 @@ café The archive also holds `show.sh`, `reference.txt`, and `bad.txt`. The made input is the file `header.md`, in ISO-8859-1. -`reads [-case ] [-private ]... [...]` takes these parts: +`reads [-case ] [-limit ] [-private ]... [...]` takes these parts: - **Output**: the primary output that the command wrote. When the answer is the output, `cp stdout answer.json` keeps it in a file first - **Reference**: the file that holds what the reader must print, byte for byte. The author writes it from the made input, without the extension -- **Bad**: a bad variant of the output, which a handler with the bug writes. For a block that a reader needs, it keeps the raw block +- **Bad**: a bad variant of the output, which a handler with the bug writes. Without an accepted privacy limit, a block that a reader needs stays raw in the bad variant - **Reader**: the program that reads the output, and its arguments. The word `{}` stands for the path of the file that it reads. Without that word, the kit adds the path as the last argument - `-private`: a text of the made input that the output must not hold. Give it once for each text - `-case`: the case of the Output group of [Generic cases](generic-cases.md) that the case proves, by its name +- `-limit`: an [accepted privacy limit](#accepted-privacy-limits) bound to that exact Output case Output, Reference, and Bad are paths in the folder of the scenario. The scenario makes each file that the case reads. @@ -49,9 +50,9 @@ The kit compares the whole value, so an answer of 30 lines never passes for 3. A 2. Output, Reference, and Bad are paths in the folder of the scenario. 3. The reader is not `ncly`, `ncly-`, or a program in `requires.bins` of a command of the manifest. 4. The reader exits 0 on the output, and prints the reference. -5. No private text is in the output, or in the text of the reader. +5. Each text given with `-private` is absent from the output and the reader's text. 6. The bad variant fails. With a private text, the bad variant holds it. Without one, the reader fails on the bad variant, or prints other text. -7. The cases `Keeps data that a reader needs` and `Keeps private data of the input` give a private text. +7. The cases `Keeps data that a reader needs` and `Keeps private data of the input` give private text or a validated `-limit`. 8. The case of `-case` is an Output case, and `## Case matrix` of `spec.md` names it. Step 6 proves that the reader, the reference, and the search can refuse a bad output. @@ -72,3 +73,28 @@ The reviewer judges these facts: - The matrix names each Output case that applies - The reader and the reference share no bug of the handler - The made input uses a value that is not the default + +## Accepted privacy limits + +An accepted privacy limit can retain metadata that the command does not remove. +Its case still needs a passing independent reader, the complete reference, and a bad variant that fails. + +1. Declare the limit under **Limits.**, with its ID, state `limit`, description, and the user's acceptance record. + Use the columns ID, State, Limit, and Accepted, in that order. + An empty Accepted cell, `None`, or a cell that starts with `Fill:` supplies no acceptance. + The Limit cell also declares each affected `Output: `, separated from its description by semicolons. + Bindings can come before or after the description. Empty text, whitespace, its own ID alone, and bindings alone do not describe retained data. + A semicolon-separated part that equals `None` or starts with `Fill:` prevents acceptance. These comparisons ignore case. + Missing description text fails with `limit "" is missing Limit text`. + A placeholder part fails with the same message. Remove any placeholder part and describe the retained data. +2. Name that ID in backticks in Accepted for each matrix row that names the applicable Output case. + Source uses the complete `Output: ` binding. Separate multiple bindings with semicolons. +3. Use `reads -case '' -limit [...]`. +4. Make the reader and reference check each retained field and the input's nondefault display declaration. +5. Keep `-private` for text that must stay absent. Every such search remains strict, including with `-limit`. + +Only the two privacy Output cases accept `-limit`. +The ID must identify exactly one accepted limit whose Limit cell names the case, and apply in every matrix row that names it. +An unknown ID, missing acceptance, wrong declared case, unbound matrix row, or ambiguous binding fails. +The reviewer rejects a description unrelated to the declared case, and checks that the reference covers every retained field. +A declaration alone proves no case and grants no coverage exemption. diff --git a/docs/north-star/contract/analyzers.md b/docs/north-star/contract/analyzers.md index bb6718ef..d72c384c 100644 --- a/docs/north-star/contract/analyzers.md +++ b/docs/north-star/contract/analyzers.md @@ -22,13 +22,20 @@ It skips each `_test.go` file, because a test can do on purpose what the rule re | Rule | Refuses | Use instead | Past mistake | |---|---|---|---| -| `callerenv` | `os.Environ` or `syscall.Environ` in `sdk/extensionrepo`, `sdk/extensiontest`, and `sdk/verification`, outside `extensionrepo.NewOwnedRun` | An [owned run](owned-runs.md) | The parent of 09809608 | +| `callerenv` | `os.Environ` or `syscall.Environ` in `sdk/extensionrepo`, `sdk/extensiontest`, `sdk/verification`, and their child packages, outside the exact allowed readers below | An [owned run](owned-runs.md) | The parent of 09809608 | | `runnerdiscard` | `io.Discard` as the `Stdout` or `Stderr` of a `runner.Program`, in a struct literal or a field assignment | `nil`, or no field | 3e6a4eff | **callerenv.** Each check that copies the environment of its caller filters it in its own way. Each new caller setting then needs a fix in each copy, as 17 past corrections did. The rule reads a call and a function value. -It reads only the packages of the kit, the repository gates, and the TLC runner, so an extension never meets it. +It reads each owned package and every path starting with that package followed by `/`. +A similar prefix such as `sdk/extensiontestx` is outside its scope. +The allowed readers match the exact package and function; a child package never inherits an allowance. +`extensionrepo.NewOwnedRun` builds owned runs. +`extensionrepo.updateEnviron` removes `GIT_*` for updates, repository checks, `just contract`, and readiness Git probes. +`extensiontest/guard.stage` preserves the isolated scenario environment across `exec`. +`extensiontest/fake.run` prints environment names, never values, for its `env` action. +An extension package is outside these SDK paths. A program that starts with no `Env` inherits the environment of the caller too, and the rule cannot see that. **runnerdiscard.** The runner reads a writer that is not a file through a pipe. diff --git a/docs/north-star/contract/extension-repository.md b/docs/north-star/contract/extension-repository.md index ad8e5ab4..169c8548 100644 --- a/docs/north-star/contract/extension-repository.md +++ b/docs/north-star/contract/extension-repository.md @@ -2,7 +2,9 @@ This part says how the Git repository of an extension outside Nicely is laid out, created, and checked. The extension template creates it, as [D062](../decisions/D062-copier-template.md) decides. [Release and install](extensions.md#release-and-install) says how an author releases a version, and how a user installs and removes it. -The repository is one Go module. Its `go.mod` pins one Nicely version, as [Compatibility](compatibility.md) defines. It also repeats the `replace` of Lip Gloss from Nicely's `go.mod`, because a `replace` never reaches another module. That fork fixes a `tmux info` call as the package loads, until a release of Lip Gloss ships the fix. The template writes `go.mod` only at creation, so the repository owns it with its own dependencies. Its root holds the files of a first-party extension in `extensions//`, plus its own `README.md`, `vision.md`, `worker/`, and `AGENTS.md`: +[Version pins](extension-repository/versions.md) defines the repository's Go module and dependency pins. + +Its root holds the files of a first-party extension in `extensions//`, plus its own `README.md`, `vision.md`, `worker/`, and `AGENTS.md`: | File | Holds | |---|---| diff --git a/docs/north-star/contract/extension-repository/versions.md b/docs/north-star/contract/extension-repository/versions.md new file mode 100644 index 00000000..f3291dd1 --- /dev/null +++ b/docs/north-star/contract/extension-repository/versions.md @@ -0,0 +1,7 @@ +# Version pins + +The repository is one Go module. Its `go.mod` pins one Nicely version, as [Compatibility](../compatibility.md) defines. It also repeats the `replace` of Lip Gloss from Nicely's `go.mod`, because a `replace` never reaches another module. That fork fixes a `tmux info` call as the package loads, until a release of Lip Gloss ships the fix. The template writes `go.mod` only at creation, so the repository owns it with its own dependencies. + +Links to Nicely documentation use the revision that Copier selected, `_commit`. +A template rendered from a release tag links to that tag's documentation. +The template teaches its procedures with public tools. diff --git a/docs/north-star/contract/generic-cases.md b/docs/north-star/contract/generic-cases.md index bd97e032..6ecadb3a 100644 --- a/docs/north-star/contract/generic-cases.md +++ b/docs/north-star/contract/generic-cases.md @@ -11,7 +11,8 @@ The line states the outcome of the case, such as a refusal with its code. The safe outcome below is the default. Another outcome is a limit, and only the user can accept it. A case that cannot occur for the command needs no line. A case name is not a test. For each case of the Output group that applies to the command, a scenario checks a test file of the extension's own domain. -The scenario makes that file, and `reads` proves the safe outcome on it with an independent reader, as [Acceptance cases](acceptance-cases.md) says. +The scenario makes that file. `reads` proves its safe outcome, or the declared outcome of an accepted privacy limit, with an independent reader. +[Acceptance cases](acceptance-cases.md) defines this check. An error that names the input gives its path or its item, so an agent knows which call failed. A command that creates or replaces a user file also declares its publication, as [Publication declaration](publication.md) says. diff --git a/docs/north-star/contract/global-flags.md b/docs/north-star/contract/global-flags.md index 3190936d..3c86200a 100644 --- a/docs/north-star/contract/global-flags.md +++ b/docs/north-star/contract/global-flags.md @@ -10,9 +10,13 @@ Each global flag other than `--help` and `--version` has an environment variable | `--no-input` | `NCLY_NO_INPUT=1` | off | Never prompts, even in a terminal. | | `--no-color` | `NO_COLOR`, or `TERM=dumb` | off | Plain text. | | `--lang ` | `NCLY_LANG` | detected | Language of human text, such as `en` or `fr-CA`. | -| `-v`, `--verbose` | `NCLY_VERBOSE=1` | off | Adds step details on stderr. | +| `-v`, `--verbose` | `NCLY_VERBOSE=1` | off | Passes the choice to extensions, which add details when needed. Core adds no details today. | -`NCLY_DEBUG=1` adds internals, timings, and child commands on stderr. It has no flag. Keys never appear in any output. +`NCLY_DEBUG=1` has no flag. Core adds no debug details today. +An extension adds details when it needs them. +Transcript forwards worker diagnostics to stderr with verbose or debug enabled, and debug enables Python debug logs. +[COR-630](../../backlog/COR-630-core-diagnostics.md) revisits core details when a core command needs them. +Keys never appear in any output. Every command that writes accepts `--dry-run`. The other flags below apply when the command confirms an action or produces files. diff --git a/docs/north-star/contract/owned-runs.md b/docs/north-star/contract/owned-runs.md index 21e14ebc..3c523d32 100644 --- a/docs/north-star/contract/owned-runs.md +++ b/docs/north-star/contract/owned-runs.md @@ -6,8 +6,8 @@ This part says how the SDK builds the environment of a verification run, and how ## The constructor `extensionrepo.NewOwnedRun` builds the environment and the folder of each run of the test kit, signoff, release, acceptance, and `just verify-models`. -It is the one function of `sdk/extensionrepo`, `sdk/extensiontest`, and `sdk/verification` that reads the environment of the caller as a whole. -The [analyzer](analyzers.md) `callerenv` refuses `os.Environ` and `syscall.Environ` everywhere else in these packages. +The `callerenv` analyzer checks these SDK package trees and permits only the exact whole-environment readers listed in [Analyzers](analyzers.md). +That list includes the constructor, the updater's Git filter, and two intentional fixture readers. A run has one of two profiles: diff --git a/docs/north-star/contract/programs-that-ncly-runs.md b/docs/north-star/contract/programs-that-ncly-runs.md index 821d10b5..2217ad82 100644 --- a/docs/north-star/contract/programs-that-ncly-runs.md +++ b/docs/north-star/contract/programs-that-ncly-runs.md @@ -15,7 +15,7 @@ One program starts outside the program runner. On macOS, go-keyring, which `inte **Process groups.** The program runner starts each program without the terminal in a new session, which gives it its own process group and no controlling terminal. A Ctrl-C at the terminal therefore reaches `ncly` alone, and the runner signals the program. A read of the terminal, such as an open of `/dev/tty`, fails at once, instead of stopping the program until its timeout. Such a program also gets no descriptor of the terminal. The runner refuses a terminal on its stdin, and copies its stdout and stderr to the terminal through pipes. A descriptor of the terminal that `ncly` inherits above stderr reaches no program. This one rule covers a worker, doctor's version probes, and the commands that core calls, as [Commands that core calls](commands-that-core-calls.md) defines. A program that may read the terminal stays in the process group of `ncly` instead: an extension, which asks through its forms. A process outside the terminal's foreground group stops when it reads the terminal, as [D047](../decisions/D047-terminal-process-group.md) explains. A program can therefore receive the same signal more than once, from the terminal, from `ncly`, and from the extension that started it, and it treats a repeat during its cleanup as no new event. -**Cancellation.** On Ctrl-C or SIGTERM, `ncly` sends the signal to every program it started and to every descendant that it can still reach. It sends SIGKILL to what still runs 15 seconds later, which leaves a Python program its own 10 seconds to stop its children. A list command that completion runs gets SIGKILL at once instead, as [Completion values](completion-values.md) defines. Before each signal, `ncly` finds descendants through their parent process, so a descendant that left the process group of its parent, as the children of transcript's Python program do, still receives the signal. On Linux, `ncly` registers as a child subreaper, so it also reaches a descendant whose parent already exited. macOS has no subreaper, so a descendant whose parent exited before the signal is the one exception there, and it keeps running. `ncly` keeps every finished output and removes only its own temporary files. It exits 130 with `INTERRUPTED` or 143 with `TERMINATED`, and under `--json` the answer keeps the data keys that apply, such as `output_dir` or the `results` of finished items. A second signal during this cleanup is ignored. A SIGKILL sent to `ncly` itself leaves no time to clean up. +**Cancellation.** [Cancellation](programs-that-ncly-runs/cancellation.md) defines signal propagation, cleanup budgets, and handler deadlines. **Output streams.** A descendant that inherits a program's stdout or stderr can hold it open after the program exits, as an orphan on macOS can. `ncly` stops reading the streams that it reads from a program 2 seconds after that program exits, including the exit that the SIGKILL of cleanup forces. A descendant therefore never keeps `ncly` waiting, and output written after that point is lost. This drain limit is its own clock, apart from the 15 seconds of cleanup. Doctor's version probes and `sdk/wrap` keep only the first bytes of each stream, up to a limit, so a noisy program never fills memory. When its limit ends a command that core calls, core closes its pipes at once, without a drain limit, as [Commands that core calls](commands-that-core-calls.md) defines. Doctor does the same for a version probe that it kills at its limit. A list command that completion runs gets no drain limit in any case, because its 2 seconds bound the reading too, as [Completion values](completion-values.md) defines. A stream that `ncly` passes on without reading, such as an extension's stdout without `--json`, stays with the descendant, which may still write to the terminal after `ncly` exits. diff --git a/docs/north-star/contract/programs-that-ncly-runs/cancellation.md b/docs/north-star/contract/programs-that-ncly-runs/cancellation.md new file mode 100644 index 00000000..d6c5e383 --- /dev/null +++ b/docs/north-star/contract/programs-that-ncly-runs/cancellation.md @@ -0,0 +1,20 @@ +# Cancellation + +On Ctrl-C or SIGTERM, `ncly` sends the signal to every program it started and to every descendant that it can still reach. The host keeps a 15-second cleanup bound. +After 13 seconds, it force-stops descendants, leaving a nominal 2-second reservation for the extension handler. +Force-stop and exit overhead use part of that reservation. +Staging cleanup must fit the remaining time. +The force-stop targets all observed descendants, including programs that the handler started for cleanup. +The reservation protects only the direct handler; the runner cannot distinguish cleanup children from workers. +A handler must account for stopped cleanup programs before reporting its signal outcome. +Keep child diagnostics separate from the structured answer. +At 15 seconds, it force-stops every remaining process, including an extension that does not finish cleanup. +An unfinished handler can leave staging behind. +The host then reports `INTERRUPTED` with exit 130, or `TERMINATED` with exit 143, without the handler's result data. +A Python program still has its own 10 seconds to stop its children. +The shared runner exposes this reservation as `Program.CleanupReserve`; its default is zero. +The reservation stays inside `Cleanup`, and values outside zero through `Cleanup` are clamped to that range. +A direct extension keeps the ordinary worker budget, because it has no outer host deadline. A list command that completion runs gets SIGKILL at once instead, as [Completion values](../completion-values.md) defines. Before each signal, `ncly` finds descendants through their parent process, so a descendant that left the process group of its parent, as the children of transcript's Python program do, still receives the signal. On Linux, `ncly` registers as a child subreaper, so it also reaches a descendant whose parent already exited. macOS has no subreaper, so a descendant whose parent exited before the signal is the one exception there, and it keeps running. `ncly` keeps every finished output and removes only its own temporary files. It exits 130 with `INTERRUPTED` or 143 with `TERMINATED`, and under `--json` the answer keeps the data keys that apply, such as `output_dir` or the `results` of finished items. A second signal during this cleanup is ignored. A SIGKILL sent to `ncly` itself leaves no time to clean up. + +On one macOS host, each probe removed 20,000 staging files. With 100,000 files on that host, both signals reached the 15-second cutoff with staging left. +[Issue #473](https://github.com/pascalandy/nicely/issues/473) records these measurements and signal answers, and tracks recovery of incomplete cleanup. diff --git a/docs/north-star/contract/sdk.md b/docs/north-star/contract/sdk.md index 270e92c4..04d7ad66 100644 --- a/docs/north-star/contract/sdk.md +++ b/docs/north-star/contract/sdk.md @@ -58,6 +58,9 @@ An undeclared service gets `RUNTIME`, because core never passes its key. `Main` A failure of the command itself prints as an ordinary failure for every kind: the error block, or the answer on stderr with `--json`. On Ctrl-C or SIGTERM, the SDK ends `ctx`, stops the extension's programs as [Programs that ncly runs](programs-that-ncly-runs.md) defines, and sets the outcome's signal, so a handler returns what it has. A handler that has not returned once the 15 seconds of cleanup and the 2 seconds of the drain limit have passed, the time that the SIGKILL of cleanup gives its programs to end, gets no more time: the SDK answers `INTERRUPTED` or `TERMINATED` alone and exits. Under `ncly`, core's own 15 seconds govern, and either path gives the same answer, because both carry the signal's code and no data key. Run directly, `ncly-` parses global flags and ordinary syntax relative to its extension root. Canonical arguments are a subset of that syntax. +Under the host, cleanup after a resistant worker has a nominal two-second reservation inside the 15-second bound. +Allow for force-stop and exit overhead when designing staging cleanup. +[Cancellation](programs-that-ncly-runs/cancellation.md) defines the outcome when that reservation is insufficient. `--version` prints the version of the module that built it. `Check` reports each fault below: - A manifest that breaks [Manifest](manifest.md) diff --git a/docs/north-star/contract/test-kit.md b/docs/north-star/contract/test-kit.md index 7cf0b324..e289d708 100644 --- a/docs/north-star/contract/test-kit.md +++ b/docs/north-star/contract/test-kit.md @@ -58,9 +58,11 @@ An acceptance case makes the same check on a made input, and the kit runs it, as ### Acceptance test files Each case of the Output group of [Generic cases](generic-cases.md) that applies to the command needs a test file (#420). -A scenario makes that file in the extension's own domain, and `reads` proves the safe outcome on it, as [Acceptance cases](acceptance-cases.md) says. Two cases need more: +A scenario makes that file in the extension's own domain. +`reads` proves its safe outcome, or the declared outcome of an accepted privacy limit, with an independent reader. +[Acceptance cases](acceptance-cases.md) defines this check. Two cases need more: -- **Keeps data that a reader needs.** The test file holds a valid block that a reader needs, and that block holds private text. The block uses a value that is not the default. The case proves both results with an independent reader: the private text is not in the output, and the output still shows the content correctly. Its bad variant keeps the raw block, and fails (#423) +- **Keeps data that a reader needs.** The test file holds a valid block that a reader needs, and that block holds private text. The block uses a value that is not the default. An independent reader proves the declared privacy behavior and that the output still shows the content correctly. Without an [accepted privacy limit](acceptance-cases.md#accepted-privacy-limits), private text is absent, and the bad variant keeps the raw block and fails (#423) - **Loses data that changes the meaning.** Some declarations change how a reader shows the content. The test file gives each of them a value that is not the default. The independent reference applies that declaration before it compares the content, so a lost declaration fails the check The tests make each file that they use. They never copy a file of the user, and the author never searches the home folder of the user for sample files. diff --git a/docs/north-star/contract/update-an-extension.md b/docs/north-star/contract/update-an-extension.md index 8798aae8..ace9a6d7 100644 --- a/docs/north-star/contract/update-an-extension.md +++ b/docs/north-star/contract/update-an-extension.md @@ -17,6 +17,11 @@ go -C "$(go mod download -json github.com/pascalandy/nicely@VERSION | sed -n 's/ ``` The updater accepts only `vX.Y.Z` tags and an absolute repository path. +Every program it starts gets an explicit environment, with every `GIT_*` variable of the caller removed. +The same helpers serve repository checks, `just contract`, and readiness Git probes. +Their Git configuration comes from the repository, HOME, and the system, rather than caller `GIT_*` overrides. +This includes Git started by Copier, so a hook or `git rebase --exec` cannot redirect the update to its repository. +Other caller settings remain available to the update programs. It first requires a committed, clean Git tree. Copier keeps the author's `go.mod` and applies the target template. The updater checks for conflict markers, `.rej` files, and unmerged index entries before changing dependencies. diff --git a/docs/north-star/decisions/D069-recorded-sessions.md b/docs/north-star/decisions/D069-recorded-sessions.md new file mode 100644 index 00000000..164d412f --- /dev/null +++ b/docs/north-star/decisions/D069-recorded-sessions.md @@ -0,0 +1,135 @@ +# D069 Record the sessions of authors and reviewers + +Chosen for [#450](https://github.com/pascalandy/nicely/issues/450), after Pascal chose recorded session IDs on 2026-11-03. +This decision specifies the recording design. It does not add production enforcement. +Today, `just finish` still accepts the reviewer name of its existing record. +The follow-up #451 implements the rule after refinement; it is outside this change. + +## 1. Capture identity from the harness + +| Harness | Supported floor for this design | Identity source | +|---|---|---| +| Claude Code | 2.1.317 | `SessionStart` command-hook input, field `session_id` | +| Codex | 0.166.0 | `SessionStart` command-hook input, field `session_id` | + +These are chosen, tested support floors, not the first versions that introduced hooks. +Claude introduced `SessionStart` in 1.0.62, but that fact alone does not establish every field of the chosen capture contract. +The earliest complete Codex version was not established. +The recorder checks the installed version and requires an actual capture; configuration alone proves nothing. + +A launcher creates one context file per invocation before starting the harness. +The hook writes the emitted ID, harness version, event, source, role and repository into that context. +The owned recording recipes accept no session ID argument, and no prompt asks an agent to provide its identity. +A launcher-generated UUID is allowed for Claude's `--session-id` only when the emitted hook confirms that exact value. +A transcript filename, display name, tool-call ID or Codex child thread ID is never a substitute. + +On resume, clear, compact or fork, the hook records the new event and emitted identity. +Each commit reads the current context of its invocation; it never reads a global "current session" file. +Missing or stale capture stops recording with a setup instruction. + +## 2. Install project hooks explicitly + +The template supplies project hook declarations and launcher recipes. +Their recording logic lives in the shared SDK repository program, following D067; the template copies only calls and forms. +An extension adopts these files through its template update. +Nicely does not silently edit a user's global Claude or Codex settings. + +The user owns project and hook trust. +Codex requires its project configuration and non-managed hooks to be trusted before they run. +The ordinary setup uses the supported trust interface, rather than editing trust storage. +The successful scratch proof used a project-trust entry in its private Codex home and the per-invocation hook-trust bypass. +That trust persisted only in the scratch home. No user-global trust setting changed. +The initial attempt without that project trust loaded no hooks and is excluded from the proof. +This exception is not the default author path. + +The launcher owns the context path, role and expected repository, and exports them to the harness and its Git subprocesses. +A missing context, wrong repository or unobserved hook fails; it never falls back to an agent-written ID. + +## 3. Bind each author to the actual commit + +The canonical author receipt maps an actual full commit SHA to its captured author sessions. +Store receipts as Git notes under one explicit `refs/notes/nicely-sessions` ref, outside source commit history. +This avoids a receipt commit changing the commit that its own receipt names. +Each receipt includes its capture source and harness version. +Readable commit trailers may repeat the captured identity, but copied trailers alone do not establish a fresh binding. + +The owned commit path reads the invocation context, creates the commit, resolves its actual SHA, then writes its receipt. +A launcher may finalize those receipts after the session returns only with exclusive ownership of its worktree and a recorded before/after range. +It must account for every commit in that range and reject unexpected writers or head movement. +The scratch proof uses Claude's Stop hook to check, commit and record the resulting SHA during the author session. +Its Git message hook adds the captured ID automatically; the launcher exports the exact-SHA receipt from those captures. + +For several authors, the round's author set is the union of its per-commit receipts. +A history rewrite creates new SHAs and invalidates the old bindings and review. +An owned rewrite must capture its writer and preserve any recorded source authors in fresh receipts. +It cannot infer missing historical authors from names or copied text. +An unsupported rewrite fails until a new captured round or supported rewrite records those bindings. + +## 4. Bind the reviewer to an immutable range + +The reviewer launcher resolves the round's full base and head SHAs before starting a fresh root session. +It passes that exact range as the review target and captures the reviewer's hook identity in a new invocation context. +It does not resume the author, fork the author's conversation or ask a native child to satisfy the different-session rule. + +After the review completes successfully, the launcher checks the same head and a clean source tree again. +It writes a review receipt attached to that exact head, preserving the head's author receipt. +The receipt names the base, head, captured reviewer session, harness version, root launch and completed result. +A failed, interrupted or missing capture yields no completed receipt. +A completed review with findings still follows the existing finding and resolution gates. + +Publish the explicit notes ref with the source branches, and export a portable receipt bundle for checks. +The publication recipe must verify both writes; ordinary branch push does not transport Git notes. +A review or author receipt changes neither the source HEAD nor the review range. +Source edits, amendments and rebases need fresh bindings and another review. + +## 5. Keep pre-adoption rounds as history + +An extension records its adoption commit as the boundary. +Finished rounds before that boundary remain legacy; no tool invents IDs for them or silently upgrades their claims. +The first enforced round starts from that boundary and records every new commit and its review. +An open round that crosses adoption starts a new captured round, rather than using a historical reviewer name as an ID. +Legacy ancestors stay outside the enforced range. + +## 6. Check a detached signoff without an active session + +A detached `just signoff` fetches the explicit notes ref and freezes its object ID before checking. +It reads the build's adoption boundary and round range, and requires: + +- The source HEAD equals the completed review's full head SHA and the base equals the round's base +- Every commit of the enforced range has a complete exact-SHA author receipt from a supported capture +- The reviewer receipt comes from a completed fresh root launch, with no missing capture or unresolved required finding +- The reviewer ID differs from every recorded author ID of the round +- The source HEAD and frozen receipt set remain unchanged through checks and status publication + +The check needs no active harness, environment session ID or ID argument. +Missing notes, a failed fetch, an incomplete export, a changed head or a missing author fails with a recording or re-review instruction. +A portable export carries the source head and notes-ref object ID, so detached checks identify the receipt set they checked. +Git notes are editable metadata, so the signoff reports that frozen receipt set rather than treating a moving ref as immutable. + +## 7. Reject a native child as a separate author review + +Codex's subagent hook reports its parent's `session_id`. +Its `agent_id` identifies the child, not a new recorded session for this rule. +A child of an author therefore fails reviewer inequality even when it has a different thread ID. +The skill must start a separate root reviewer session through the launcher and compare the captured IDs. +A fresh process alone does not prove inequality; the emitted values decide it. + +The scratch proof also asks a native child of the separate reviewer to report the parent ID supplied by its hook. +That third observation is expected to equal the reviewer ID. +It shows what a child exposes; it does not act as the round's reviewer. + +## 8. Claim a different recorded session + +The receipt proves a different recorded session under this workflow. +It does not prove independent reasoning, a different human, a different model or an absence of shared context. +Two sessions may share all those things and reach the same conclusion. +An editable hook, Git note or caller environment is not a cryptographic boundary against a hostile author. +The reviewer still judges the work; the receipt makes the identity and target claims explicit and checkable. + +## Sources and scratch proof + +The [OpenAI hooks contract](https://learn.chatgpt.com/docs/hooks#common-input-fields) defines the session and child fields and hook trust. +The [Claude hook contract](https://code.claude.com/docs/en/hooks#sessionstart-input) defines its emitted ID. +The [Claude changelog](https://code.claude.com/docs/en/changelog#1062) dates the earlier event introduction. +The [proof record](D069/proof.md) preserves the emitted identities and their Git binding, with private authentication folders excluded. +The prototype and negative controls exercise this design in a disposable extension; they do not make today's `just finish` enforce it. diff --git a/docs/north-star/decisions/D069/proof.md b/docs/north-star/decisions/D069/proof.md new file mode 100644 index 00000000..de22261f --- /dev/null +++ b/docs/north-star/decisions/D069/proof.md @@ -0,0 +1,102 @@ +# D069 scratch recording proof + +Recorded on 2026-11-03 with Claude Code 2.1.317 and Codex 0.166.0. +The disposable extension and harness state lived under the Cook work folder. +Private authentication folders were excluded from every evidence copy and quote. +Their auth symlinks were removed by link path after the harnesses stopped; both original auth files still exist. + +Claude edited the greeting implementation. Its Stop hook ran the real-host check and committed the edit. +The Git hook obtained the author trailer from SessionStart capture, without an agent-supplied ID. +A fresh Codex root reviewed the fixed base/head range, passed the same check and left the source tree unchanged. +Its native child reported the parent ID from the SubagentStart capture. +The child observation equals the reviewer ID; it is not a third distinct session. + +## Claude author + +```json +{ + "event": "SessionStart", + "session_id": "b7897dc7-005f-4d0b-8a9e-ba821f3952d4", + "cwd": "/Users/me/session-proof/extension", + "source": "startup", + "agent_id": null, + "harness": "claude-code", + "version": "2.1.317", + "role": "author" +} +``` + +## Codex reviewer + +```json +{ + "event": "SessionStart", + "session_id": "01a1a354-e946-7f63-b6a4-a832a1578c3f", + "cwd": "/Users/me/session-proof/extension", + "source": "startup", + "agent_id": null, + "harness": "codex", + "version": "0.166.0", + "role": "reviewer" +} +``` + +## Parent ID reported by the native child + +```json +{ + "event": "SubagentStart", + "session_id": "01a1a354-e946-7f63-b6a4-a832a1578c3f", + "cwd": "/Users/me/session-proof/extension", + "source": null, + "agent_id": "01a1a355-1252-7cc1-b136-503e9eba26c1", + "harness": "codex", + "version": "0.166.0", + "role": "reviewer" +} +``` + +## Git binding + +The recorded output of `git log -1 --format=fuller --notes=nicely-session-proof` includes both receipts. +One whitespace-only line is trimmed below: + +```text +commit 0f3f093575f806b5d1da5d07e23551b82ef524dc +Author: Nicely session proof +AuthorDate: Tue Nov 3 16:29:28 2026 -0500 +Commit: Nicely session proof +CommitDate: Tue Nov 3 16:29:28 2026 -0500 + + 🚑 fix: sessionproof: Greet the requested person + + Nicely-Session-ID: b7897dc7-005f-4d0b-8a9e-ba821f3952d4 + Nicely-Harness: claude-code + Nicely-Harness-Version: 2.1.317 + +Notes (nicely-session-proof): + {"author": {"commit": "0f3f093575f806b5d1da5d07e23551b82ef524dc", "session_id": "b7897dc7-005f-4d0b-8a9e-ba821f3952d4", "harness": "claude-code", "version": "2.1.317", "capture_source": "SessionStart+Stop"}, "review": {"base": "bf13be819abe8b8c06bf29ddaf0dadd47139ad81", "head": "0f3f093575f806b5d1da5d07e23551b82ef524dc", "session_id": "01a1a354-e946-7f63-b6a4-a832a1578c3f", "harness": "codex", "version": "0.166.0", "capture_source": "SessionStart", "completed": true, "root": true}} +``` + +The review receipt binds base `bf13be819abe8b8c06bf29ddaf0dadd47139ad81` and head `0f3f093575f806b5d1da5d07e23551b82ef524dc`. +The prototype uses `refs/notes/nicely-session-proof`; D069 selects `refs/notes/nicely-sessions` for production. +The final prototype notes object is `e5b66060e50addbc678e9ef56e2b111ae2a1b42b`. +No receipt mutation changed source HEAD. + +## Prototype controls + +The detached check ran without an active harness or a session context. + +| Control | Result | +|---|---| +| same-session reviewer | reject, exit 1 | +| missing author receipt | reject, exit 1 | +| changed review target | reject, exit 1 | +| incomplete reviewer | reject, exit 1 | +| missing author capture prevents commit | reject, exit 1 | +| detached check without active harness | accept, exit 0 | + +The first Codex attempt did not load the project hooks, so its review is excluded from the identity proof. +The successful invocation persisted its project-trust entry and sessions in the private scratch Codex home. +Its per-invocation hook-trust bypass applied to the vetted hooks. No user-global trust setting changed. +These prototype controls do not implement the production gate of #451. diff --git a/docs/north-star/decisions/README.md b/docs/north-star/decisions/README.md index fe5ed131..0ece19b8 100644 --- a/docs/north-star/decisions/README.md +++ b/docs/north-star/decisions/README.md @@ -74,3 +74,4 @@ A new decision takes the next number, a file named `D0NN-.md` that s | [D066](D066-refuse-unreadable-variables.md) | Refuse a variable or a language that ncly cannot read | | [D067](D067-share-verification-rules.md) | Share verification rules and keep extension facts | | [D068](D068-verification-map.md) | Map each check to an owner and a level | +| [D069](D069-recorded-sessions.md) | Record the sessions of authors and reviewers | diff --git a/docs/north-star/principles.md b/docs/north-star/principles.md index 063083eb..86276d9e 100644 --- a/docs/north-star/principles.md +++ b/docs/north-star/principles.md @@ -5,13 +5,13 @@ These rules shape every design choice in Nicely. A principle links the decisions 1. **Domain first.** Commands read `ncly [] `, such as `ncly video convert`, with one verb per action across domains. `ncly --help` lists everything a domain does. A few commands stand alone, such as `ncly doctor`. A script that joins Nicely is reshaped into this grammar, whatever its interface was before. ([D003](decisions/D003-domain-first.md)) 2. **One implementation, two faces.** In interactive mode, a missing value opens a short form. In non-interactive mode, the same command fails with exit code 2 and a hint that shows the full command. Both faces produce the same result. ([D009](decisions/D009-ask-in-a-terminal.md)) 3. **A stable agent contract.** Machine identifiers stay in English. An agent acts on codes, exit codes, and the next command as words, never on translated text. [Compatibility](contract/compatibility.md) protects JSON types, units, values, and meanings as well as key names, and defines how versions evolve. ([D007](decisions/D007-stable-agent-contract.md), [D027](decisions/D027-json-answer.md), [D053](decisions/D053-next-command.md)) -4. **Safe by default.** Every writing command shares preparation with `--dry-run`. Overwriting or deleting needs `--force` or a yes in the terminal. Deleted files go to the OS trash. [Retry safety](contract/retry-safety.md) covers all effects, including free writes. A caller that names its run repeats the same call without repeating an effect. Outside text reaches an agent only with its tools turned off, and a check after execution is never described as write prevention. ([D008](decisions/D008-one-confirmation-flag.md), [D028](decisions/D028-outside-text.md), [D034](decisions/D034-shared-preparation.md), [D052](decisions/D052-caller-run-ids.md)) +4. **Safe by default.** Every writing command shares preparation with `--dry-run`. Overwriting or deleting needs `--force` or a yes in the terminal. The first command that deletes a user file must send it to the OS trash through the SDK. [COR-629](../backlog/COR-629-os-trash.md) records this condition. [Retry safety](contract/retry-safety.md) covers all effects, including free writes. A caller that names its run repeats the same call without repeating an effect. Outside text reaches an agent only with its tools turned off, and a check after execution is never described as write prevention. ([D008](decisions/D008-one-confirmation-flag.md), [D028](decisions/D028-outside-text.md), [D034](decisions/D034-shared-preparation.md), [D052](decisions/D052-caller-run-ids.md)) 5. **Keys stay with the human.** Keys live in the OS keychain or in environment variables, never in flags, config files, the repository, or the binary. An agent that meets a missing key relays the hint, and the human runs `ncly auth login`. A key reaches an extension only after a human grants it. ([D010](decisions/D010-keychain-or-environment.md), [D030](decisions/D030-trusted-extensions.md), [D058](decisions/D058-grants-by-path.md)) 6. **Check everything, install nothing.** `ncly doctor` checks tools and keys, and never installs anything. A failed check names its fix in its hint, such as the install command of a missing tool, and the human applies it. ([D011](decisions/D011-local-checks.md)) 7. **Every word Nicely shows can be translated.** Core and each extension keep their text in message catalogs, never in code. English ships first, and Canadian French follows in [COR-102](../backlog/COR-102-canadian-french.md). A pseudo-locale exposes untranslated text from M00. Extensions receive the language in `NCLY_LANG` and translate their own text. ([D012](decisions/D012-translatable-from-day-one.md)) 8. **Core and every extension are written in Go.** An extension uses the SDK, the one supported way for now. Nicely's own code may embed a Python program while it moves to Go. That program talks to Go in JSON only, and Go renders and translates everything a human sees. A user's script that an extension wraps is a worker, which keeps its own language. ([D001](decisions/D001-go-cobra-charm.md), [D013](decisions/D013-go-everywhere.md)) 9. **Same paths on every machine.** Config, data, state, and cache follow XDG on macOS and Linux, under `nicely`. The shared config describes the setup the user wants and can travel between machines. The local config holds what belongs to one machine. ([D015](decisions/D015-macos-and-linux.md), [D023](decisions/D023-shared-and-local-config.md)) -10. **Import Go, wrap the rest, credit both.** Nicely imports Go libraries and wraps other tools, such as `ffmpeg`. The help of a wrapping command names the tool, and every release ships the license notices of its dependencies. +10. **Import Go, wrap the rest, credit both.** Nicely imports Go libraries and wraps other tools, such as `ffmpeg`. Once `sdk/wrap` supports tool credit, the help of each wrapping command names the tool. [COR-631](../backlog/COR-631-tool-credit.md) revisits this with the next wrapping command. Every release ships the license notices of its dependencies. 11. **Build what your card needs, and decide early what is costly to change.** Write a rule before the item that codes it when a later item would otherwise force a rewrite. ([D029](decisions/D029-decide-early.md), [D042](decisions/D042-milestones-and-cards.md)) 12. **Compose through the CLI.** An extension uses another extension through `ncly ... --json`, the interface that agents use, never through its code. Every new extension therefore adds a command that agents and other extensions can use at once. ([D040](decisions/D040-wire-contract.md)) 13. **Extensions add, never change.** An extension adds commands and never replaces one or changes how another command behaves. Shared behavior keeps its owner in the [code map](architecture.md#code-map). Each extension owns the steps beneath its root, including within a shared domain. Nicely has no hooks. [COR-615](../backlog/COR-615-hooks.md) records the deferred work. ([D044](decisions/D044-additive-extensions.md), [D064](decisions/D064-shared-domains.md)) diff --git a/docs/north-star/rules/README.md b/docs/north-star/rules/README.md index 26038a90..0fd37824 100644 --- a/docs/north-star/rules/README.md +++ b/docs/north-star/rules/README.md @@ -20,9 +20,12 @@ A card that adds a check moves its rule up, as D068 says. | [preferences.md](preferences.md) | D01 to D26 of `dev-preferences.md`: mockups, docs, and Python | | [landing.md](landing.md) | D27 to D46 of `dev-preferences.md`: reviews, merges, stacks, UATs, and releases | | [principles.md](principles.md) | P01 to P31 of `principles.md` | -| [sdk.md](sdk.md) | S01 to S26: contract, extension, repository, kit, and i18n. S52 to S53: owned runs and results. S54 to S55: publication. S56 to S58: the TLC runner | +| [sdk.md](sdk.md) | S01 to S26: contract, extension, repository, kit, and i18n. S52 to S53: owned runs and results. S54 to S55: publication. S66: update environments | +| [documentation.md](documentation.md) | S65: pinned documentation | +| [models.md](models.md) | S56 to S58: the TLC runner | | [acceptance.md](acceptance.md) | S59: acceptance cases of the kit | -| [runtime.md](runtime.md) | S27 to S51: platform, record, runner, tui, wrap, the fake child, and analysis | +| [runtime.md](runtime.md) | S27 to S51: platform, record, runner, tui, wrap, the fake child, and analysis. S68: handler cleanup reservation | +| [verification.md](verification.md) | V01: private Linux keyring runs. V02: the recorded-session design | | [publication.md](publication.md) | S60 to S64: the guard, `without`, `keeps`, and `holds` | ## Support code @@ -41,7 +44,7 @@ At 7f3ae60e, `ledger_history.go` enforced rules that no row named until b3dc8b48 | [extensionrepo/release_example.go](../../../sdk/extensionrepo/release_example.go) | Reads the words of an example without a shell | | [extensionrepo/release_install.go](../../../sdk/extensionrepo/release_install.go) | Runs the install block of `README.md` in scratch folders | | [extensionrepo/scenario.go](../../../sdk/extensionrepo/scenario.go) | Runs one scenario for `just scenario` | -| [extensionrepo/spec.go](../../../sdk/extensionrepo/spec.go) | Reads the lines of `spec.md` for the kit and finish | +| [extensionrepo/spec.go](../../../sdk/extensionrepo/spec.go) | Reads spec lines and accepted limits for the kit and finish | | [extensionrepo/update.go](../../../sdk/extensionrepo/update.go) | Runs `just update` | | [extensiontest/install.go](../../../sdk/extensiontest/install.go) | Installs the built extension into the scratch folders of the kit | diff --git a/docs/north-star/rules/acceptance.md b/docs/north-star/rules/acceptance.md index 452f645e..b93037f7 100644 --- a/docs/north-star/rules/acceptance.md +++ b/docs/north-star/rules/acceptance.md @@ -4,10 +4,12 @@ This part of the [core rule index](README.md) holds S59 of COR-104-T5: the accep | ID | Rule | Source | Enforced by | |---|---|---|---| -| S59 | An acceptance case reads the output with a reader that is not the extension or a wrapped program, and compares the reference. It searches each private text, and its bad variant fails. Each Output case of `## Case matrix` needs a case | [Acceptance cases][acceptance-cases] | [acceptance.go][kit-acceptance], [acceptance_matrix.go][kit-acceptance-matrix], [acceptance test][acceptance-test], [template acceptance test][template-acceptance-test], partial: the reviewer judges the independence of the reader and the reference. The reviewer also checks that the matrix names each case that applies | +| S59 | An acceptance case uses an independent reader and compares its output with the reference. It searches each `-private` text and refuses a bad variant. Each matrix Output case needs a case. A privacy exception has retained-data text and passes the accepted-limit grammar. Its accepted Limit row declares the case. Every matrix row of that case names the limit ID | [Acceptance cases][acceptance-cases] | [acceptance.go][kit-acceptance], [acceptance_matrix.go][kit-acceptance-matrix], [accepted-limit parser][spec-limits], [acceptance test][acceptance-test], [privacy-limit test][privacy-limit-test], [template acceptance test][template-acceptance-test]. Partial: the reviewer judges reader independence, reference independence, matrix completeness, and whether the accepted description covers the retained metadata | [acceptance-cases]: ../contract/acceptance-cases.md [kit-acceptance]: ../../../sdk/extensiontest/acceptance.go [kit-acceptance-matrix]: ../../../sdk/extensiontest/acceptance_matrix.go [acceptance-test]: ../../../sdk/extensiontest/acceptance_test.go +[spec-limits]: ../../../sdk/extensionrepo/spec.go +[privacy-limit-test]: ../../../sdk/extensiontest/privacy_limits_test.go [template-acceptance-test]: ../../../scripts/template-acceptance-test.sh diff --git a/docs/north-star/rules/documentation.md b/docs/north-star/rules/documentation.md new file mode 100644 index 00000000..53606e93 --- /dev/null +++ b/docs/north-star/rules/documentation.md @@ -0,0 +1,13 @@ +# Documentation + +This part of the [core rule index](README.md) holds the rule for pinned documentation. + +| ID | Rule | Source | Enforced by | +|---|---|---|---| +| S65 | Template documentation links use the selected revision. Bundled extension documentation uses the running release tag, or main for development | [Version pins][repository-versions], [bundled skills][skill-spec] | [Source links test][revision-sources], [render test][revision-render], [bundled links test][revision-bundled] | + +[repository-versions]: ../contract/extension-repository/versions.md +[skill-spec]: ../../../extensions/skill/spec/nicely.md +[revision-sources]: ../../../tools/cmd/plan/template_links_test.go +[revision-render]: ../../../scripts/template-repository-test.sh +[revision-bundled]: ../../../extensions/skill/bundled_test.go diff --git a/docs/north-star/rules/models.md b/docs/north-star/rules/models.md new file mode 100644 index 00000000..8577dd3b --- /dev/null +++ b/docs/north-star/rules/models.md @@ -0,0 +1,16 @@ +# Formal models + +This part of the [core rule index](README.md) holds S56 to S58, the TLC runner rules of COR-104-T8. + +| ID | Rule | Source | Enforced by | +|---|---|---|---| +| S56 | The TLC jar holds the SHA-256 that the suite pins before Java starts, from the URL or a local jar | [verification.go][verification] | [tlc.go][tlc], [verification test][verification-test], [template models test][models-test] | +| S57 | A model run writes its record only when each check gives its expected outcome, and no input changed | [verification.go][verification] | [verification.go][verification], [tlc.go][tlc], [verification test][verification-test] | +| S58 | `just check` reads only the record of the models, with their pins, inputs, and outcomes | [verification.go][verification] | [record.go][verification-record], [models.go][models], [verification test][verification-test], [template models test][models-test], partial: the record binds the model, not the code that it models | + +[models]: ../../../sdk/extensionrepo/models.go +[models-test]: ../../../scripts/template-models-test.sh +[tlc]: ../../../sdk/verification/tlc.go +[verification]: ../../../sdk/verification/verification.go +[verification-record]: ../../../sdk/verification/record.go +[verification-test]: ../../../sdk/verification/verification_test.go diff --git a/docs/north-star/rules/principles.md b/docs/north-star/rules/principles.md index 619ff834..6ea212a6 100644 --- a/docs/north-star/rules/principles.md +++ b/docs/north-star/rules/principles.md @@ -12,7 +12,7 @@ This part of the [core rule index](README.md) holds the rules of `principles.md` | P06 | Machine values keep their types, units, values, meanings, and version compatibility | [principles.md][principles] | [Compatibility test][compat], partial: listed consumer cases | | P07 | A writing command and its dry run share their preparation | [principles.md][principles] | [Kit `refuses`][paired], [its test][paired-test], partial: scenarios choose the cases | | P08 | Overwrite or delete only with force, or with consent at a terminal | [principles.md][principles] | [Auth login][auth-login], [auth form][auth-form], partial: each handler needs cases | -| P09 | Send a deleted user file to the trash of the OS | [principles.md][principles] | Prose only | +| P09 | The first command that deletes a user file sends it to OS trash through the SDK, per [COR-629][trash] | [principles.md][principles] | Prose only | | P10 | Each effect is safe to retry, and a named completed run replays without new effects | [principles.md][principles] | [Agent run boundaries][agent-run], [transcript replay][replay], partial: recorded commands | | P11 | Give outside text to an agent only with its tools off | [principles.md][principles] | Prose only | | P12 | Never call a check after execution a prevention of writes | [principles.md][principles] | Prose only | @@ -27,7 +27,7 @@ This part of the [core rule index](README.md) holds the rules of `principles.md` | P21 | A wrapped user worker can keep its own language | [principles.md][principles] | [Template test][template-test], [wrap scenario][wrap] | | P22 | Use XDG paths under `nicely` on macOS and Linux | [principles.md][principles], [config.go][config] | [Config test][config-test] | | P23 | Portable setup goes in the shared config, and the setup of one machine in the local config | [principles.md][principles] | [Config test][config-test] | -| P24 | Import Go libraries, wrap other tools, and credit a wrapped tool in its help | [principles.md][principles] | Prose only | +| P24 | Import Go libraries and wrap other tools. Credit tools in help once sdk/wrap supports it, per [COR-631][credit] | [principles.md][principles] | Prose only | | P25 | Each release ships the license notices of its dependencies | [principles.md][principles] | [Archive check][archive], [its test][artifacts-test] | | P26 | Build only what the card needs, and decide costly constraints before code forces a rewrite | [principles.md][principles] | Prose only | | P27 | Extensions compose through `ncly` JSON commands, never through the code of another extension | [principles.md][principles] | Prose only | @@ -37,6 +37,8 @@ This part of the [core rule index](README.md) holds the rules of `principles.md` | P31 | Use the keyboard, and typed y/N confirmations that keep the question and the answer visible | [principles.md][principles] | [Auth form][auth-form], [form scenario][form] | [principles]: ../principles.md +[trash]: ../../backlog/COR-629-os-trash.md +[credit]: ../../backlog/COR-631-tool-credit.md [spec-test]: ../../../internal/cli/spec_test.go [shared-domains]: ../../../testdata/script/extension_shared_domains.txtar [command]: ../../../sdk/command/command.go diff --git a/docs/north-star/rules/runtime.md b/docs/north-star/rules/runtime.md index 9272bec3..8630b568 100644 --- a/docs/north-star/rules/runtime.md +++ b/docs/north-star/rules/runtime.md @@ -1,6 +1,7 @@ # Platform, record, runner, tui, wrap, and analysis This part of the [core rule index](README.md) holds S27 to S51, the rules of SDK package comments. +It also holds S68 of [Cancellation][programs]. | ID | Rule | Source | Enforced by | |---|---|---|---| @@ -29,6 +30,7 @@ This part of the [core rule index](README.md) holds S27 to S51, the rules of SDK | S49 | The env action of the fake child prints names only, never values | [fake/main.go][fake] | [Fake test][fake-test] | | S50 | Discard a stream of a runner.Program with nil, never io.Discard, so no descendant holds its pipe | [runnerdiscard.go][runnerdiscard] | [runnerdiscard test][runnerdiscard-test], [template recipes test][template-recipes-test], partial: no variable flow | | S51 | Add an analyzer as a package listed in Analyzers, with cases that fail first on its past mistake | [analysis.go][analysis] | Prose only | +| S68 | The host reserves the last 2 seconds of its 15-second cancellation budget for handler cleanup, after force-stopping descendants, including cleanup children. The final deadline still kills an unresponsive handler | [Cancellation][programs] | [runner cleanup][cleanup], [host dispatch][dispatch], [staged cleanup scenario][staged-test], [real-host stress driver][staged-stress], [stubborn handler control][cancel-stubborn] | [proctree]: ../../../sdk/platform/proctree.go [platform-file]: ../../../sdk/platform/file_test.go @@ -56,3 +58,10 @@ This part of the [core rule index](README.md) holds S27 to S51, the rules of SDK [runnerdiscard-test]: ../../../sdk/analysis/runnerdiscard/runnerdiscard_test.go [template-recipes-test]: ../../../scripts/template-recipes-test.sh [analysis]: ../../../sdk/analysis/analysis.go + +[programs]: ../contract/programs-that-ncly-runs/cancellation.md +[cleanup]: ../../../sdk/runner/cleanup.go +[dispatch]: ../../../internal/cli/dispatch.go +[staged-test]: ../../../testdata/extensions/hello/testdata/script/staged_cleanup.txtar + +[staged-stress]: ../../../.agents/skills/verify-nicely/scripts/cleanup-stress.py diff --git a/docs/north-star/rules/sdk.md b/docs/north-star/rules/sdk.md index a9263c6a..cbad7cf9 100644 --- a/docs/north-star/rules/sdk.md +++ b/docs/north-star/rules/sdk.md @@ -1,6 +1,6 @@ # Contract, extension, repository, kit, and i18n -This part of the [core rule index](README.md) holds S01 to S26, the rules of SDK package comments. It also holds S52 to S53 of COR-104-T3, S54 to S55 of COR-104-T6, and S56 to S58 of COR-104-T8. +This part of the [core rule index](README.md) holds S01 to S26, the rules of SDK package comments. It also holds S52 to S53 of COR-104-T3 and S54 to S55 of COR-104-T6. S66 covers update environments. [Documentation](documentation.md) holds S65 for pinned documentation. [Formal models](models.md) holds S56 to S58 of COR-104-T8. | ID | Rule | Source | Enforced by | |---|---|---|---| @@ -30,14 +30,17 @@ This part of the [core rule index](README.md) holds S01 to S26, the rules of SDK | S24 | Format numbers, sizes, durations, and dates with i18n | [i18n.go][i18n] | Prose only | | S25 | Translated text sets its own width | [i18n.go][i18n] | Prose only | | S26 | Clip command-line and variable values before a refusal quotes them | [i18n.go][i18n] | [Long values scenario][long-values] | -| S52 | The kit, signoff, release, acceptance, and the TLC runner build each environment with `NewOwnedRun`, which alone reads `os.Environ` | [Owned runs][owned-runs] | [owned_run.go][owned-run], [callerenv][callerenv], [isolation test][isolation-test], [acceptance.go][acceptance], [models.go][models], [template recipes test][recipes-test], partial: a program with no `Env` inherits its caller | +| S52 | The kit, signoff, release, acceptance, and the TLC runner build each environment with `NewOwnedRun`. Only the exact readers of [Analyzers][analyzers] may read the whole caller environment in the owned package trees | [Owned runs][owned-runs] | [owned_run.go][owned-run], [callerenv][callerenv], [isolation test][isolation-test], [acceptance.go][acceptance], [models.go][models], [template recipes test][recipes-test], partial: a program with no `Env` inherits its caller | | S53 | Each action of the export program returns one result, whose answer, notices, and exit code one writer delivers | [Owned runs][owned-runs] | [result.go][result], [result test][result-test], [isolation test][isolation-test], [template recipes test][recipes-test] | | S54 | `## Publication` of `spec.md` declares each mode of each command that may write a user file. A passing check proves each obligation, or a reason covers it | [Publication declaration][publication-contract] | [publication.go][publication], [publication_row.go][publication-row], [publication_report.go][publication-report], [publication test][publication-test], [predicts test][predicts-test], [template recipes test][recipes-test], partial: the reviewer accepts each reason | | S55 | A successful dry run changes no file, and predicts the destination and the overwrite choice of its run | [Scenario commands][scenario-commands] | [predicts.go][predicts], [publication test][publication-test], [predicts test][predicts-test], partial: only Linux removes an ability of the destination, with `without` of [S61](publication.md) | -| S56 | The TLC jar holds the SHA-256 that the suite pins before Java starts, from the URL or a local jar | [verification.go][verification] | [tlc.go][tlc], [verification test][verification-test], [template models test][models-test] | -| S57 | A model run writes its record only when each check gives its expected outcome, and no input changed | [verification.go][verification] | [verification.go][verification], [tlc.go][tlc], [verification test][verification-test] | -| S58 | `just check` reads only the record of the models, with their pins, inputs, and outcomes | [verification.go][verification] | [record.go][verification-record], [models.go][models], [verification test][verification-test], [template models test][models-test], partial: the record binds the model, not the code that it models | +| S66 | Every update program gets an explicit environment without any caller `GIT_*` setting, including Git that Copier starts | [Update an extension][update-contract] | [update.go][update], [poisoned update test][update-test], [callerenv scope test][callerenv-scope] | +[analyzers]: ../contract/analyzers.md +[update-contract]: ../contract/update-an-extension.md +[update]: ../../../sdk/extensionrepo/update.go +[update-test]: ../../../sdk/extensionrepo/update_environment_test.go +[callerenv-scope]: ../../../sdk/analysis/callerenv/scope_test.go [agents]: ../../../AGENTS.md [contract-codes]: ../../../sdk/contract/codes.go [contract-spec]: ../../../sdk/contract/spec_test.go @@ -90,8 +93,3 @@ This part of the [core rule index](README.md) holds S01 to S26, the rules of SDK [publication-test]: ../../../sdk/extensiontest/publication_test.go [predicts-test]: ../../../sdk/extensiontest/predicts_test.go [models]: ../../../sdk/extensionrepo/models.go -[verification]: ../../../sdk/verification/verification.go -[tlc]: ../../../sdk/verification/tlc.go -[verification-record]: ../../../sdk/verification/record.go -[verification-test]: ../../../sdk/verification/verification_test.go -[models-test]: ../../../scripts/template-models-test.sh diff --git a/docs/north-star/rules/verification.md b/docs/north-star/rules/verification.md new file mode 100644 index 00000000..dfb04174 --- /dev/null +++ b/docs/north-star/rules/verification.md @@ -0,0 +1,17 @@ +# Verification runs + +This part of the [core rule index](README.md) holds rules for isolation of verification runs and for recorded author and reviewer identities. + +| ID | Rule | Source | Enforced by | +|---|---|---|---| +| V01 | Linux Verify uses one private D-Bus session and unlocked keyring across its steps. Missing prerequisites fail by name. Cleanup, missing owned state, or a six-hour lifetime ends the session. A normal stop records exit 0 without a shell diagnostic | [Verify skill][verify] | [Real CLI test][test], [lifecycle test][lifetime-test], [session helper][helper], [wrapper][wrapper], [cleanup][cleanup]. Partial: macOS keeps its existing login-keychain path | +| V02 | A future enforced build binds each author and a completed different root reviewer session to exact commits, using harness-captured IDs | [D069][sessions] | Prose on purpose: this decision specifies the design; #451 implements production enforcement after refinement. The [scratch proof][session-proof] is evidence of capture only | + +[sessions]: ../decisions/D069-recorded-sessions.md +[session-proof]: ../decisions/D069/proof.md +[verify]: ../../../.agents/skills/verify-nicely/SKILL.md +[test]: ../../../cmd/ncly/keyring_linux_test.go +[lifetime-test]: ../../../cmd/ncly/keyring_lifecycle_linux_test.go +[helper]: ../../../.agents/skills/verify-nicely/scripts/keyring.sh +[wrapper]: ../../../.agents/skills/verify-nicely/scripts/verify.sh +[cleanup]: ../../../.agents/skills/verify-nicely/scripts/cleanup.sh diff --git a/docs/qa/ledger.md b/docs/qa/ledger.md index 30f9984a..d10131b7 100644 --- a/docs/qa/ledger.md +++ b/docs/qa/ledger.md @@ -37,6 +37,8 @@ A correction that lands below the highest level that can prevent its failure say | Correction | Failure | Level | Owner | |---|---|---|---| +| [#468](https://github.com/pascalandy/nicely/issues/468) | In #427, equal host and worker cancellation budgets killed the extension handler before it removed private conversion staging | 2 | A | +| [#462](https://github.com/pascalandy/nicely/issues/462) | In #427, user-accepted metadata retention could not pass either privacy Output case. Omitting `-private` failed, and the retained author failed its scan. Review exposed accepted rows with no retained-data description, or with a placeholder beside that description | 2 | A | | COR-104-T7 | In the builds of 2026-10-30, a failed hard link of a create fell back to a check and a rename over the output. N3 of COR-104 found it. A host with hard links never ran that fallback, and no check could remove an ability | 2 | A | | COR-104-T5 | At the parent of 86d24e83, a prefix match of `acceptance.sh` accepted 30 lines for 3. In #425, no test file held private text in a block that a reader needs, and two builds kept it. The reviewer still judges that a reader and a reference are independent | 2 | A | | COR-104-T8 | On 2026-11-02, the pinned URL of TLC v1.8.0 served other bytes, and only the digest check of video-convert stopped them. At the parent of 59d33f38, a finding of this round with no state passed finish, which the finish model now shows | 2 | A | diff --git a/extensions/agent/spec.md b/extensions/agent/spec.md index 2ae1bcf0..91bd62f9 100644 --- a/extensions/agent/spec.md +++ b/extensions/agent/spec.md @@ -110,7 +110,7 @@ A dry run with `--run-id` tries the run lock without waiting and without creatin ### Stops and failures -Ctrl-C, SIGTERM, and the timeout stop Codex and every descendant that `ncly` can reach, with the process-tree cleanup of [Programs that ncly runs](../../docs/north-star/contract/programs-that-ncly-runs.md): the signal, or SIGTERM for the timeout, then SIGKILL 15 seconds later. They stop the Codex check at once, as they stop a version probe. Without `--dry-run`, what the call answers depends on where the stop comes. Each answer keeps the `run_id`, and each answer after the record has `ncly run view ` as its hint and `next`: +Ctrl-C and SIGTERM stop Codex and every descendant that `ncly` can reach under [Cancellation](../../docs/north-star/contract/programs-that-ncly-runs/cancellation.md). A timeout sends SIGTERM, then SIGKILL 15 seconds later. Each stop interrupts the Codex check and version probes at once. Without `--dry-run`, what the call answers depends on where the stop comes. Each answer keeps the `run_id`, and each answer after the record has `ncly run view ` as its hint and `next`: - Before the record, such as during the Codex check or the wait for the run lock, no record exists: a signal answers with exit 130 or 143, and the timeout answers `RUNTIME`, whose hint asks for a longer `--timeout` - After the intent, before Codex starts, and for a start that fails, such as a Codex that is no longer executable or a temporary folder that cannot be made, no task ran: the step turns `failed` with its error, and the run `interrupted` after a signal, or `failed`. The call answers `INTERRUPTED` with exit 130, `TERMINATED` with exit 143, or `RUNTIME` diff --git a/extensions/skill/bundled.go b/extensions/skill/bundled.go index cf3382ba..c5a7b8ec 100644 --- a/extensions/skill/bundled.go +++ b/extensions/skill/bundled.go @@ -1,10 +1,15 @@ package skill import ( + "cmp" "embed" "io/fs" + "os" + "strings" + "github.com/pascalandy/nicely/sdk/command" "github.com/pascalandy/nicely/sdk/extension" + "golang.org/x/mod/module" ) // bundled holds the skills that teach an agent to use Nicely: nicely, and @@ -18,6 +23,10 @@ var bundled embed.FS // bundledSkills returns the skills of the bundled source, in the order of // their folders, without a path. func bundledSkills() []Skill { + revision := cmp.Or(os.Getenv("NCLY_VERSION"), command.ModuleVersion()) + if revision == "v0.0.0-dev" || module.IsPseudoVersion(revision) { + revision = "main" + } files, _ := fs.Glob(bundled, "*/SKILL.md") skills := make([]Skill, 0, len(files)) for _, file := range files { @@ -26,7 +35,8 @@ func bundledSkills() []Skill { continue } if fm, err := extension.ParseSkill(data); err == nil { - skills = append(skills, Skill{Name: fm.Name, Description: fm.Description, Source: sourceBundled, content: string(data)}) + content := strings.ReplaceAll(string(data), "@NCLY_REVISION@", revision) + skills = append(skills, Skill{Name: fm.Name, Description: fm.Description, Source: sourceBundled, content: content}) } } return skills diff --git a/extensions/skill/bundled_test.go b/extensions/skill/bundled_test.go new file mode 100644 index 00000000..785731ec --- /dev/null +++ b/extensions/skill/bundled_test.go @@ -0,0 +1,35 @@ +package skill + +import ( + "regexp" + "testing" +) + +// Released skills link to their own docs, while development builds use main. +func TestBundledDocumentationRevision(t *testing.T) { + for _, tc := range []struct{ version, revision string }{ + {"", "main"}, + {"v0.0.0-dev", "main"}, + {"v0.27.0", "v0.27.0"}, + {"v0.27.0-rc.1", "v0.27.0-rc.1"}, + {"v0.26.1-0.20261103123456-0895b8b325fe", "main"}, + } { + t.Run(tc.version, func(t *testing.T) { + t.Setenv("NCLY_VERSION", tc.version) + for _, skill := range bundledSkills() { + if skill.Name != "nicely-extensions" { + continue + } + links := regexp.MustCompile(`https://github\.com/pascalandy/nicely/blob/([^/]+)/`).FindAllStringSubmatch(skill.content, -1) + if len(links) == 0 { + t.Fatal("the extensions skill has no documentation links") + } + for _, link := range links { + if link[1] != tc.revision { + t.Errorf("link %s uses revision %q, want %q", link[0], link[1], tc.revision) + } + } + } + }) + } +} diff --git a/extensions/skill/nicely-extensions/SKILL.md b/extensions/skill/nicely-extensions/SKILL.md index b93fc4e9..d0747f2a 100644 --- a/extensions/skill/nicely-extensions/SKILL.md +++ b/extensions/skill/nicely-extensions/SKILL.md @@ -7,7 +7,7 @@ description: Use to install, build, or remove an ncly extension. ## Install an extension -[Release and install](https://github.com/pascalandy/nicely/blob/main/docs/north-star/contract/extensions.md#release-and-install) holds the install rule: +[Release and install](https://github.com/pascalandy/nicely/blob/@NCLY_REVISION@/docs/north-star/contract/extensions.md#release-and-install) holds the install rule: > A user installs a fixed version with `git clone --branch `, then `just install` in the clone, which need Go and `just`. > For an annotated tag, Git can warn that the tag `is not a commit!`, then note a detached HEAD: the clone holds the tagged commit. @@ -32,7 +32,7 @@ uv tool run copier@9.19.1 copy --vcs-ref VERSION --defaults --data domain=DOMAIN The domain names a family of work, such as `video`, and the optional name identifies one job, such as `convert`. Neither names the wrapped tool. For a whole-domain extension, omit `--data name=NAME` and both `-NAME` suffixes. -Then build each command with the `nicely-extension` skill of the new repository, from the first command. That skill is in `.agents/skills/nicely-extension/`, and `AGENTS.md` gives the reading order and the recipes. [Extensions](https://github.com/pascalandy/nicely/blob/main/docs/north-star/contract/extensions.md) holds the rules of an extension. +Then build each command with the `nicely-extension` skill of the new repository, from the first command. That skill is in `.agents/skills/nicely-extension/`, and `AGENTS.md` gives the reading order and the recipes. [Extensions](https://github.com/pascalandy/nicely/blob/@NCLY_REVISION@/docs/north-star/contract/extensions.md) holds the rules of an extension. ## Update an extension repository @@ -48,7 +48,7 @@ go -C "$(go mod download -json github.com/pascalandy/nicely@VERSION | sed -n 's/ On a conflict, resolve each file and mark it with `git add`. Remove each `.rej` file, then run the printed bootstrap command with `--continue`. Success ends with `just check`. -[Update an extension](https://github.com/pascalandy/nicely/blob/main/docs/north-star/contract/update-an-extension.md) holds the recovery rules, and the first build after an update from a template before `just finish`. +[Update an extension](https://github.com/pascalandy/nicely/blob/@NCLY_REVISION@/docs/north-star/contract/update-an-extension.md) holds the recovery rules, and the first build after an update from a template before `just finish`. ## Remove an extension diff --git a/extensions/skill/spec.md b/extensions/skill/spec.md index 4cbbb9dc..ed5694d4 100644 --- a/extensions/skill/spec.md +++ b/extensions/skill/spec.md @@ -35,21 +35,7 @@ ncly skill status [--json] ### The nicely skill -`ncly` bundles four skills that teach an agent to operate Nicely. `nicely` holds what every call needs, and names when to read each other skill: - -| Skill | Teaches | Read | -|---|---|---| -| `nicely` | The variables, `ncly describe`, dry run first, codes, exit codes, `next`, doctor, the shell timeout, and extension skills | Before any `ncly` call | -| `nicely-auth` | `ncly auth`, and the rule to relay a missing key or grant to the human | For a key or a grant | -| `nicely-runs` | Run IDs, `ncly run`, and recovery after a lost answer | Before a recorded command | -| `nicely-extensions` | The install at a tag, the template command, and the removal | To install, build, or remove one | - -A bundled skill has no folder for a relative link, so each topic is a skill of its own. One line in an agent's instructions points to `nicely`: `When asked to use nicely, run ncly skill view nicely`. The first example of `ncly --help` names it too. A skill of `[skill] paths` that takes a bundled name wins over it, with `SKILL_SHADOWED`. - -The skills teach only what the release that ships them can do. Each item that adds a core or skill command adds its lines to the skill of its topic. `nicely-extensions` quotes the install rule of [Release and install](../../docs/north-star/contract/extensions.md#release-and-install). Explicit resume remains in [COR-621](../../docs/backlog/COR-621-resume-sdk.md#later). A skill writes each command in a code span, such as `ncly skill list`, so that a test reads it. A test fails when a bundled skill names an `ncly` command, or an error or warning code, that this `ncly` lacks. Each holds at most 60 lines, and `nicely` at most 2,000 bytes, because an agent reads it before every call. - -Every external extension ships a skill named after its slug. -An agent reads `ncly skill view ` for an extension root that it finds through describe. The `skill` extension ships no skill named `skill`: the bundled skills teach core and the skill domain together. +[The nicely skill](spec/nicely.md) defines the bundled skills and the rules of their content. ## Config diff --git a/extensions/skill/spec/nicely.md b/extensions/skill/spec/nicely.md new file mode 100644 index 00000000..51931d14 --- /dev/null +++ b/extensions/skill/spec/nicely.md @@ -0,0 +1,20 @@ +# The nicely skill + +`ncly` bundles four skills that teach an agent to operate Nicely. `nicely` holds what every call needs, and names when to read each other skill: + +| Skill | Teaches | Read | +|---|---|---| +| `nicely` | The variables, `ncly describe`, dry run first, codes, exit codes, `next`, doctor, the shell timeout, and extension skills | Before any `ncly` call | +| `nicely-auth` | `ncly auth`, and the rule to relay a missing key or grant to the human | For a key or a grant | +| `nicely-runs` | Run IDs, `ncly run`, and recovery after a lost answer | Before a recorded command | +| `nicely-extensions` | The install at a tag, the template command, and the removal | To install, build, or remove one | + +A bundled skill has no folder for a relative link, so each topic is a skill of its own. One line in an agent's instructions points to `nicely`: `When asked to use nicely, run ncly skill view nicely`. The first example of `ncly --help` names it too. A skill of `[skill] paths` that takes a bundled name wins over it, with `SKILL_SHADOWED`. + +The skills teach only what the release that ships them can do. Each item that adds a core or skill command adds its lines to the skill of its topic. `nicely-extensions` quotes the install rule of [Release and install](../../../docs/north-star/contract/extensions.md#release-and-install). Explicit resume remains in [COR-621](../../../docs/backlog/COR-621-resume-sdk.md#later). A skill writes each command in a code span, such as `ncly skill list`, so that a test reads it. A test fails when a bundled skill names an `ncly` command, or an error or warning code, that this `ncly` lacks. Each holds at most 60 lines, and `nicely` at most 2,000 bytes, because an agent reads it before every call. + +Every external extension ships a skill named after its slug. +An agent reads `ncly skill view ` for an extension root that it finds through describe. The `skill` extension ships no skill named `skill`: the bundled skills teach core and the skill domain together. + +Links in the bundled `nicely-extensions` skill use the running `ncly` release tag. +A development build uses `main`, including a build from a Go pseudo-version. diff --git a/extensions/transcript/execute.go b/extensions/transcript/execute.go index 8fa22bcb..6ecde1f4 100644 --- a/extensions/transcript/execute.go +++ b/extensions/transcript/execute.go @@ -20,16 +20,25 @@ import ( // then name-2 up to name-1000. const maxCandidates = 1000 +var newRunID = record.NewID + // execute runs the prepared job under the run ID of the call, or a new one, // as Records and replay in spec/records-and-replay.md defines. It takes the -// run lock, then replays a completed run, which needs no key, or refuses an -// uncertain one. Otherwise it resolves the job, reads the granted key before -// any record or effect, and starts an attempt. +// run lock first for a caller-named run, then replays completion or refuses +// uncertainty. An unnamed run checks its output and key before opening. +// A new attempt starts with the resolved job and granted key. func execute(ctx context.Context, call extension.Call, j job, id string) contract.Outcome { p := call.Printer runs := store() - if id == "" { - id = record.NewID(time.Now()) + generated := id == "" + var key string + var problem *contract.Problem + if generated { + key, problem = prepareAttempt(ctx, call, &j) + if problem != nil { + return refusal(problem, "") + } + id = newRunID(time.Now()) } run, err := runs.Open(ctx, id) var bad *record.UnreadableError @@ -56,18 +65,26 @@ func execute(ctx context.Context, call extension.Call, j job, id string) contrac return refusal(problem, id) } } - if problem := resolve(ctx, call, &j); problem != nil { - return refusal(problem, savedID(run)) + if !generated || run.Record != nil { + key, problem = prepareAttempt(ctx, call, &j) + if problem != nil { + return refusal(problem, savedID(run)) + } + } + a := &attempt{call: call, p: p, j: j, run: run, saved: run.Record != nil} + return a.execute(ctx, key) +} + +// prepareAttempt checks the output and key before a new attempt can start. +func prepareAttempt(ctx context.Context, call extension.Call, j *job) (string, *contract.Problem) { + if problem := resolve(ctx, call, j); problem != nil { + return "", problem } key, problem := call.Key("deepgram") if problem == nil && !headerSafe(key) { - problem = keyUnusable(p) + problem = keyUnusable(call.Printer) } - if problem != nil { - return refusal(problem, savedID(run)) - } - a := &attempt{call: call, p: p, j: j, run: run, saved: run.Record != nil} - return a.execute(ctx, key) + return key, problem } // headerSafe reports whether a key holds only visible ASCII. Only such a diff --git a/extensions/transcript/generated_collision_test.go b/extensions/transcript/generated_collision_test.go new file mode 100644 index 00000000..66b70d12 --- /dev/null +++ b/extensions/transcript/generated_collision_test.go @@ -0,0 +1,193 @@ +package transcript + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/pascalandy/nicely/sdk/contract" + "github.com/pascalandy/nicely/sdk/extension" + "github.com/pascalandy/nicely/sdk/record" + "github.com/rogpeppe/go-internal/txtar" +) + +// The subprocess serves the actual extension with only its generated ID fixed. +// The ordinary binary exposes no flag or environment setting for this seam. +func TestMain(m *testing.M) { + if len(os.Args) > 2 && os.Args[1] == "collision-cli" { + id := os.Args[2] + newRunID = func(time.Time) string { return id } + os.Args = append(os.Args[:1], os.Args[3:]...) + extension.Main(Extension) + } + os.Exit(m.Run()) +} + +// Named calls are the standing control for an ID that already exists. A fresh +// call must reach the same answers when its generated ID collides after preflight. +func TestGeneratedIDCollisionsKeepExistingAnswers(t *testing.T) { + fixture, err := os.ReadFile(filepath.Join("..", "..", "testdata", "script", "transcript_named_replay.txtar")) + if err != nil { + t.Fatal(err) + } + files := map[string][]byte{} + for _, file := range txtar.Parse(fixture).Files { + files[file.Name] = file.Data + } + binary, err := os.Executable() + if err != nil { + t.Fatal(err) + } + pending := func(r *record.Record) { + r.Status = record.Interrupted + step := &r.Items[0].Steps[0] + step.Status, step.StartedAt, step.FinishedAt = record.StepPending, nil, nil + step.Artifacts, step.Evidence = []record.Artifact{}, nil + } + cases := []struct { + name string + exit int + code contract.Code + busy bool + change func(*record.Record) + }{ + {name: "busy", exit: 75, code: contract.Temporary, busy: true}, + {name: "completed replay", exit: 0}, + {name: "other URL", exit: 2, code: contract.UsageInvalid, change: func(r *record.Record) { r.Items[0].Key = "https://youtu.be/other" }}, + {name: "other command", exit: 2, code: contract.UsageInvalid, change: func(r *record.Record) { r.Path = "agent run" }}, + {name: "other explicit output", exit: 2, code: contract.UsageInvalid, change: func(r *record.Record) { r.Inputs["output"] = "other-output" }}, + {name: "unknown paid effect", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { + r.Status, r.Items[0].Steps[0].Status = record.Unknown, record.StepUnknown + }}, + {name: "failed paid effect", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { + r.Status, r.Items[0].Steps[0].Status = record.Failed, record.StepFailed + }}, + {name: "wrong step", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { r.Items[0].Steps[0].ID = "other" }}, + {name: "missing item", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { r.Items = nil }}, + {name: "changed completed artifact", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { + r.Items[0].Steps[0].Artifacts[0].SHA256 = strings.Repeat("0", 64) + }}, + {name: "pending invalid resolution", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { + pending(r) + r.Resolved["output"] = "relative" + }}, + {name: "pending missing output", exit: 1, code: contract.ResumeUnsafe, change: func(r *record.Record) { + pending(r) + r.Resolved["output"] = filepath.Join(r.Resolved["output"].(string), "missing") + }}, + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + root := t.TempDir() + config := filepath.Join(root, ".config", "nicely") + runs := record.Store{Dir: filepath.Join(root, ".local", "state", "nicely", "runs")} + out, bin := filepath.Join(root, "out"), filepath.Join(root, "bin") + for _, dir := range []string{config, runs.Dir, filepath.Join(out, "saved"), bin} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + } + write := func(path string, data []byte, mode os.FileMode) { + t.Helper() + if err := os.WriteFile(path, data, mode); err != nil { + t.Fatal(err) + } + } + write(filepath.Join(config, "config.toml"), fmt.Appendf(nil, "[transcript.youtube]\noutput = %q\n", out), 0o600) + // Version probes pass, but an accidental engine call cannot reach + // uv, Python, YouTube or Deepgram through the caller's PATH. + for name, version := range map[string]string{"uv": "uv 0.12.0", "python3": "Python 3.12.0"} { + write(filepath.Join(bin, name), fmt.Appendf(nil, "#!/bin/sh\n[ \"$*\" = --version ] || exit 97\nprintf '%%s\\n' '%s'\n", version), 0o700) + } + var saved record.Record + if err := json.Unmarshal([]byte(strings.ReplaceAll(string(files["saved.json"]), "ROOT", root)), &saved); err != nil { + t.Fatal(err) + } + artifacts := map[string][]byte{} + for _, artifact := range saved.Items[0].Steps[0].Artifacts { + data := files["words.txt"] + if strings.HasSuffix(artifact.Path, ".json") { + data = files["paragraphs.json"] + } + write(artifact.Path, data, 0o600) + artifacts[artifact.Path] = data + } + if test.change != nil { + test.change(&saved) + } + before, err := json.Marshal(saved) + if err != nil { + t.Fatal(err) + } + write(runs.File("saved"), before, 0o600) + if test.busy { + run, err := runs.Open(context.Background(), "saved") + if err != nil { + t.Fatal(err) + } + t.Cleanup(run.Close) + } + call := func(named bool) (string, string) { + t.Helper() + args := []string{"collision-cli", "saved", "run", "youtube", "--url", "https://youtu.be/abc", "--json"} + if named { + args = append(args, "--run-id", "saved") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, binary, args...) + cmd.Dir = root + cmd.Env = []string{ + "HOME=" + root, "PATH=" + bin, "NO_COLOR=1", "NCLY_LANG=en", "NCLY_NO_INPUT=1", + "XDG_CONFIG_HOME=" + filepath.Join(root, ".config"), "XDG_DATA_HOME=" + filepath.Join(root, ".local", "share"), + "XDG_STATE_HOME=" + filepath.Join(root, ".local", "state"), "XDG_CACHE_HOME=" + filepath.Join(root, ".cache"), + "DEEPGRAM_API_KEY=generated-collision-fixture", "DBUS_SESSION_BUS_ADDRESS=disabled:", + } + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + if cmd.ProcessState == nil { + t.Fatalf("named=%t did not start: %v", named, err) + } + if code := cmd.ProcessState.ExitCode(); code != test.exit { + t.Fatalf("named=%t exit %d, want %d: %v\n%s\n%s", named, code, test.exit, err, &stdout, &stderr) + } + answer := stdout.Bytes() + if test.exit != 0 { + answer = stderr.Bytes() + } + var reply struct { + RunID string `json:"run_id"` + Errors []struct{ Code contract.Code } `json:"errors"` + } + if err := json.Unmarshal(answer, &reply); err != nil || reply.RunID != "saved" { + t.Fatalf("named=%t has no saved-run answer: %v\n%s", named, err, answer) + } + if test.code != "" && (len(reply.Errors) == 0 || reply.Errors[0].Code != test.code) { + t.Fatalf("named=%t has wrong code, want %s:\n%s", named, test.code, answer) + } + if data, err := os.ReadFile(runs.File("saved")); err != nil || !bytes.Equal(data, before) { + t.Fatalf("named=%t changed the existing record: %v", named, err) + } + for path, want := range artifacts { + if got, err := os.ReadFile(path); err != nil || !bytes.Equal(got, want) { + t.Fatalf("named=%t changed saved output: %v", named, err) + } + } + return stdout.String(), stderr.String() + } + namedOut, namedErr := call(true) + generatedOut, generatedErr := call(false) + if namedOut != generatedOut || namedErr != generatedErr { + t.Fatalf("generated collision differs from named control\nnamed:\n%s\n%s\ngenerated:\n%s\n%s", namedOut, namedErr, generatedOut, generatedErr) + } + }) + } +} diff --git a/extensions/transcript/locales/en.toml b/extensions/transcript/locales/en.toml index 05435cab..5218bfe9 100644 --- a/extensions/transcript/locales/en.toml +++ b/extensions/transcript/locales/en.toml @@ -261,8 +261,8 @@ description = "Item error of `ncly transcript run youtube`: yt-dlp could not dow other = "yt-dlp could not download the audio of {{.URL}}." ["transcript.download_failed_hint"] -description = "Hint of the item error transcript.download_failed. --verbose stays as is." -other = "Check that the video plays in a browser, then run the call again with --verbose." +description = "Hint of the item error transcript.download_failed. browser, arc, [transcript], and --verbose stay as is." +other = "Check that the video plays in a browser. If YouTube asks you to sign in, set browser = \"arc\" in [transcript]. Run the call again with --verbose." ["transcript.deepgram_unreachable"] description = "Item error of `ncly transcript run youtube`: Deepgram could not be reached, or refused the request for now, such as with HTTP 429. Deepgram is the name of a service." diff --git a/extensions/transcript/python/youtube.py b/extensions/transcript/python/youtube.py index 68a098f7..bf6db3d6 100644 --- a/extensions/transcript/python/youtube.py +++ b/extensions/transcript/python/youtube.py @@ -41,6 +41,7 @@ r"Network is unreachable|Remote end closed connection", re.IGNORECASE, ) +YTDLP_ANSI = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") log = logging.getLogger("transcript") @@ -136,22 +137,25 @@ def run_ytdlp( if anonymous.returncode == 0: return YtDlpResult(anonymous, "anonymous", warnings) - anonymous_diagnostic = _clean_subprocess_diagnostic( - anonymous.stderr or anonymous.stdout - ) + raw_diagnostic = YTDLP_ANSI.sub("", anonymous.stderr or anonymous.stdout) + error_records = [ + line for line in raw_diagnostic.splitlines() if line.startswith("ERROR:") + ] + failure = error_records[-1] if error_records else raw_diagnostic + anonymous_diagnostic = _clean_subprocess_diagnostic(raw_diagnostic) message = f"yt-dlp failed: {anonymous_diagnostic or 'no diagnostic was returned'}" if arc_diagnostic: message = f"Arc attempt: {arc_diagnostic}; anonymous fallback: {message}" raise YtDlpError( message, - temporary=bool(YTDLP_TEMPORARY.search(anonymous_diagnostic)), + temporary=bool(YTDLP_TEMPORARY.search(failure)), warnings=warnings, ) def _clean_subprocess_diagnostic(text: str) -> str: """Remove terminal controls and collapse a subprocess diagnostic.""" - without_ansi = re.sub(r"\x1b\[[0-?]*[ -/]*[@-~]", "", text) + without_ansi = YTDLP_ANSI.sub("", text) without_secrets = re.sub( r"(?im)^(\s*(?:authorization|cookie|set-cookie)\s*[:=])[^\r\n]*", r"\1 [redacted]", diff --git a/extensions/transcript/skill/SKILL.md b/extensions/transcript/skill/SKILL.md index 9ea138b5..290a1997 100644 --- a/extensions/transcript/skill/SKILL.md +++ b/extensions/transcript/skill/SKILL.md @@ -17,6 +17,7 @@ description: Use when transcribing one YouTube video with Deepgram through ncly - A failed item keeps its cause in `results[0].errors`, and the answer's own code is the verdict of the call. `AUTH_REJECTED` means that Deepgram refused the key, or that the key holds a character that no request can carry. Relay its hint to the human, who replaces the key - Exit 1 never permits a repeat: read the run with `ncly run view --json` first. The same call with the same `--run-id` returns a completed run again without a charge, and answers `RESUME_UNSAFE` once the audio may have reached Deepgram. Only a human chooses a new run ID, which may bill again - Exit 75 with `TEMPORARY` permits the same call again, such as after another call held the same run ID +- On `TRANSCRIPT_DOWNLOAD_FAILED`, inspect the download refusal. If YouTube asks you to sign in, relay the hint about the `browser` setting to the human - Ctrl-C exits 130 with `INTERRUPTED`, and SIGTERM 143 with `TERMINATED`. The answer keeps the item and its `output_dir`. Read the run before any repeat. A run that the signal stopped after the upload answers `RESUME_UNSAFE`. A run that completed before the signal returns its result again - A dry run with the `--run-id` of a saved run decides as the call would. A completed run plans `reuse`, with its `output_dir` and no effect, and a run that cannot continue answers the same refusal. A run that another call holds answers `TEMPORARY` with exit 75, as the call does: wait, then run it again - `USAGE_INVALID` under a saved `--run-id` means that the ID names another call, with another URL or `--output`. `RESUME_UNSAFE` also answers a saved file that changed or went missing, or an output folder of the record that is gone. Relay it to the human, who inspects the folder diff --git a/extensions/transcript/spec/protocol.md b/extensions/transcript/spec/protocol.md index 15460289..f0ebe18c 100644 --- a/extensions/transcript/spec/protocol.md +++ b/extensions/transcript/spec/protocol.md @@ -64,6 +64,13 @@ A dry-run plan or zero exit alone never proves execution succeeded. | `transcript_unusable` | `unknown` | `RUNTIME` | `transcript.transcript_unusable` | | `write_failed` | `pending` before intent, `unknown` after it | `RUNTIME` | `transcript.write_failed` | +The final `ERROR` record of a failed yt-dlp attempt decides its download code. +Classification reads the raw diagnostic before display cleanup and its 2,000-character cut. +A final network error uses `download_temporary`, including HTTP 429, HTTP 5xx, and `Connection refused`. +A sign-in request or another final error uses `download_failed`, whose hint names the transcript `browser` setting. +Earlier warnings do not decide the code. +Without an `ERROR` record, the whole diagnostic uses the same network-failure rules. + `browser_cookies_skipped` is the only program warning. Go maps it to `TRANSCRIPT_BROWSER_COOKIES_SKIPPED` with catalog ID `transcript.browser_cookies_skipped`. diff --git a/extensions/transcript/spec/records-and-replay.md b/extensions/transcript/spec/records-and-replay.md index d9d79aa7..18a1d34d 100644 --- a/extensions/transcript/spec/records-and-replay.md +++ b/extensions/transcript/spec/records-and-replay.md @@ -4,6 +4,13 @@ Each run has one item and one `transcribe` step, which declares the `paid` effec The manifest supports recorded execution and dry run; domain resume remains unimplemented. Use the SDK for records, locks, and publication, as [Operations](../../../docs/north-star/contract/operations.md) defines. +Without `--run-id`, output and key checks finish before the run opens. +A refusal from either check adds no file to the runs folder. +A caller-named run still takes its lock before those checks or replay. +A refusal after the run opens can leave its lock file, which execution never deletes. +After successful preflight, a generated ID opens once and follows the existing-run decision if it collides. +Busy, replay, mismatch, and uncertain-record answers then match a caller-named run of that ID. + Python emits the single-item plan before user-side effects. Go validates it and durably creates the initial record with a pending step. The step becomes unknown before result-folder reservation or upload. @@ -20,7 +27,9 @@ Go records completion only after it verifies and syncs the required artifacts, a A different command, URL, or explicit output argument is `USAGE_INVALID` before effects. A busy run returns `TEMPORARY` with a `run view` hint and starts no program. - A completed run returns its verified saved result without Python, a dependency upgrade, or a Deepgram request. - It needs no key: the extension reads the key only for a new attempt, after the run lock and this decision. + A caller-named completed run needs no key. + A collision of a generated ID reaches replay only after preflight. + For a caller-named run, the extension reads the key only for a new attempt, after the lock and replay decision. Core may still read a granted key first, as [Keys](../../../docs/north-star/contract/keys.md) defines, and a replay answers even when that read fails. A dry run with the same ID first tries the run lock without waiting and without creating its file, and releases it at once. A busy run returns `TEMPORARY`, as execution does, and the dry run starts no program. diff --git a/extensions/transcript/testdata/script/transcript_failures.txtar b/extensions/transcript/testdata/script/transcript_failures.txtar index fa6c6c2f..b6ffc1a4 100644 --- a/extensions/transcript/testdata/script/transcript_failures.txtar +++ b/extensions/transcript/testdata/script/transcript_failures.txtar @@ -56,7 +56,7 @@ grep -count=2 '(?m)^upload ' uploads.log env NCLY_TEST_DOWNLOAD=failed exits 1 ncly transcript run youtube --url $URL --output $OUT --run-id f3 --json answer stderr RUNTIME -stderr '"errors":\[\{"code":"TRANSCRIPT_DOWNLOAD_FAILED","message":"yt-dlp could not download the audio of https://www\.youtube\.com/watch\?v=EIEc43CxIvY\.","hint":"Check that the video plays in a browser, then run the call again with --verbose\."\}\]' +stderr '"errors":\[\{"code":"TRANSCRIPT_DOWNLOAD_FAILED","message":"yt-dlp could not download the audio of https://www\.youtube\.com/watch\?v=EIEc43CxIvY\.","hint":"Check that the video plays in a browser\. If YouTube asks you to sign in, set browser = .* in \[transcript\]\. Run the call again with --verbose\."\}\]' env NCLY_TEST_DOWNLOAD= grep -count=2 '(?m)^upload ' uploads.log diff --git a/extensions/transcript/testdata/script/transcript_ytdlp_errors.txtar b/extensions/transcript/testdata/script/transcript_ytdlp_errors.txtar new file mode 100644 index 00000000..23770aa7 --- /dev/null +++ b/extensions/transcript/testdata/script/transcript_ytdlp_errors.txtar @@ -0,0 +1,123 @@ +# The real engine classifies raw diagnostics from a failing yt-dlp subprocess. +# NCLY_TEST_DOWNLOAD is not used: this launcher changes only yt-dlp's executable. +# Every refusal happens before the paid step and leaves the output empty. +env LANG= LC_ALL= LC_MESSAGES= NCLY_LANG= +env DEEPGRAM_API_KEY= +mkdir bin +symlink bin/uv -> $UV +symlink bin/python3 -> $PYTHON3 +onlyncly transcript-external +env NCLY_TEST_LAUNCHER=$WORK/launcher.py +env URL=https://www.youtube.com/watch?v=EIEc43CxIvY +env OUT=$WORK/out +mkdir $OUT +env STATE=$WORK/.local/state/nicely +mkdir $STATE +cp seed.jsonl test-keychain.jsonl +exec /bin/sh -c 'printf "{\"format_version\":1,\"grants\":[{\"service\":\"deepgram\",\"domain\":\"transcript\",\"path\":\"%s\",\"granted_at\":\"2026-10-18T04:00:00Z\"}]}\n" "$DATA/ncly-transcript" > "$STATE/grants.json"' + +# A warning about 429 cannot make a later sign-in refusal retryable. +env CASE=signin +exits 1 ncly transcript run youtube --url $URL --output $OUT --run-id signin --json +answer stderr RUNTIME +stderr '"code":"TRANSCRIPT_DOWNLOAD_FAILED"' +stderr '"hint":"[^"]*browser = ' +stderr '\[transcript\]' + +# The final error can follow more than the diagnostic's 2,000-character display limit. +env CASE=long-signin +exits 1 ncly transcript run youtube --url $URL --output $OUT --run-id long-signin --json +answer stderr RUNTIME +stderr '"code":"TRANSCRIPT_DOWNLOAD_FAILED"' +env CASE=long-network +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id long-network --json +answer stderr TEMPORARY + +# Each supported final network error remains retryable. +env CASE=429 +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id rate-limit --json +answer stderr TEMPORARY +env CASE=503 +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id unavailable --json +answer stderr TEMPORARY +env CASE=refused +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id refused --json +answer stderr TEMPORARY + +# An unknown final error stops retries, even after an earlier network ERROR. +env CASE=unknown +exits 1 ncly transcript run youtube --url $URL --output $OUT --run-id unknown --json +answer stderr RUNTIME +stderr '"code":"TRANSCRIPT_DOWNLOAD_FAILED"' +env CASE=last-fatal +exits 1 ncly transcript run youtube --url $URL --output $OUT --run-id last-fatal --json +answer stderr RUNTIME +stderr '"code":"TRANSCRIPT_DOWNLOAD_FAILED"' +env CASE=last-network +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id last-network --json +answer stderr TEMPORARY + +# With no ERROR record, preserve the network classification. +env CASE=no-error +exits 75 ncly transcript run youtube --url $URL --output $OUT --run-id no-error --json +answer stderr TEMPORARY +grep -count=10 '(?m)^download$' downloads.log +! exists uploads.log +exec /bin/ls -A $OUT +! stdout . + +-- seed.jsonl -- +{"call":"set","account":"deepgram","key":"dg-test-key"} +-- launcher.py -- +import os +import sys +from pathlib import Path + +sys.path.insert(0, sys.argv[1]) +import httpx +import transcript +import youtube + + +def forbid_upload(*_args: object, **_kwargs: object) -> None: + Path(os.environ["WORK"], "uploads.log").write_text("attempted", encoding="utf-8") + raise AssertionError("a download refusal must not upload") + + +httpx.post = forbid_upload +youtube.YTDLP_MODULE_COMMAND = ( + sys.executable, + str(Path(os.environ["WORK"], "yt_dlp_stub.py")), +) +raise SystemExit(transcript.main()) +-- yt_dlp_stub.py -- +import os +import sys +from pathlib import Path + +signin = ( + "ERROR: [youtube] EIEc43CxIvY: Sign in to confirm you're not a bot. " + "Use --cookies-from-browser or --cookies for the authentication.\n" +) +network = "ERROR: HTTP Error 429: Too Many Requests\n" +unknown = "ERROR: an unknown extractor refusal\n" +diagnostics = { + "signin": ( + "WARNING: Unable to download webpage: HTTP Error 429: Too Many Requests\n" + "WARNING: Unable to fetch GVS PO Token: Missing required Visitor Data\n" + + signin + ), + "long-signin": "WARNING: HTTP Error 429 " + "x" * 2100 + "\n" + signin, + "long-network": "WARNING: " + "x" * 2100 + "\n" + network, + "429": network, + "503": "ERROR: HTTP Error 503: Service Unavailable\n", + "refused": "ERROR: Connection refused\n", + "unknown": unknown, + "last-fatal": network + unknown, + "last-network": unknown + network, + "no-error": "Connection refused\n", +} +with Path(os.environ["WORK"], "downloads.log").open("a", encoding="utf-8") as log: + log.write("download\n") +sys.stderr.write(diagnostics[os.environ["CASE"]]) +raise SystemExit(1) diff --git a/go.mod b/go.mod index 4f2d0342..0173702a 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/zalando/go-keyring v0.2.8 go.yaml.in/yaml/v3 v3.0.5 + golang.org/x/mod v0.41.0 golang.org/x/sys v0.48.0 golang.org/x/term v0.46.0 golang.org/x/text v0.42.0 @@ -55,7 +56,6 @@ require ( github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yuin/goldmark v1.7.17 // indirect github.com/yuin/goldmark-emoji v1.0.5 // indirect - golang.org/x/mod v0.41.0 // indirect golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.23.0 // indirect ) diff --git a/internal/cli/dispatch.go b/internal/cli/dispatch.go index a13359ee..706342fb 100644 --- a/internal/cli/dispatch.go +++ b/internal/cli/dispatch.go @@ -9,6 +9,7 @@ import ( "os" "slices" "strconv" + "time" "github.com/pascalandy/nicely/internal/keychain" "github.com/pascalandy/nicely/sdk/command" @@ -69,13 +70,14 @@ func (c *call) run(p *i18n.Printer, g command.Globals, b keychain.Backend, stdou } maps.Copy(vars, keys) program := runner.Program{ - Path: c.extension.path, - Args: c.args, - Env: runner.Environ(os.Environ(), vars), - Stdin: os.Stdin, - Stdout: stdout, - Stderr: stderr, - Terminal: true, + Path: c.extension.path, + Args: c.args, + Env: runner.Environ(os.Environ(), vars), + Stdin: os.Stdin, + Stdout: stdout, + Stderr: stderr, + Terminal: true, + CleanupReserve: 2 * time.Second, } if c.extension.bundled { // Started under the extension's name, the binary of ncly runs the diff --git a/justfile b/justfile index dc8fc110..277aad0e 100644 --- a/justfile +++ b/justfile @@ -98,7 +98,7 @@ template: race: go test -race ./sdk/... ./internal/... -# Check the Python of transcript and of the extension template with ruff, and transcript with pyright and pytest, through uv +# Check transcript, template and Verify Python with ruff; transcript also gets pyright and pytest [group('checks')] python: #!/usr/bin/env bash @@ -107,8 +107,8 @@ python: # that folder the root of pyright, which otherwise takes the first parent with a pyproject.toml, # such as a home folder, and checks all of it. cd extensions/transcript/python - uv tool run ruff@0.16.9 check --isolated . ../../../template/worker - uv tool run ruff@0.16.9 format --isolated --check . ../../../template/worker + uv tool run ruff@0.16.9 check --isolated . ../../../template/worker ../../../.agents/skills/verify-nicely/scripts/cleanup-stress.py + uv tool run ruff@0.16.9 format --isolated --check . ../../../template/worker ../../../.agents/skills/verify-nicely/scripts/cleanup-stress.py requirements="$(mktemp)" trap 'rm -f "$requirements"' EXIT uv export --quiet --no-hashes --script transcript.py > "$requirements" diff --git a/scripts/template-repository-test.sh b/scripts/template-repository-test.sh new file mode 100644 index 00000000..1ce12e74 --- /dev/null +++ b/scripts/template-repository-test.sh @@ -0,0 +1,93 @@ +#!/usr/bin/env bash +# Sourced by template-test.sh to render and check one extension repository. +# shellcheck disable=SC2154 # root and copier come from fct_main in template-test.sh. + +# Every Nicely documentation link in a render uses its selected template tag. +fct_revision_links() { + local links link + links="$(grep -RhoE --exclude-dir=.git 'https://github\.com/pascalandy/nicely/blob/[^[:space:])>]+' "${1}")" || fct_fail 'the render has no Nicely documentation links' + while IFS= read -r link; do + [[ "${link}" == "https://github.com/pascalandy/nicely/blob/${2}/"* ]] || fct_fail "rendered link uses another revision: ${link}" + done <<<"${links}" +} + +fct_repository() { + local scratch="${1}" domain="${2}" name="${3}" slug="${4}" command="${5}" src repo data extensions lipgloss file + shift 5 + src="${scratch}/source" repo="${scratch}/ncly-${slug}" data="${scratch}/data" + extensions="${data}/nicely/extensions" + mkdir -p "${scratch}" + # The module proxy and Copier source both hold this checkout, with its changes. + mkdir "${src}" + fct_copy_source "${src}" + git -C "${src}" init -q + fct_revision v0.0.1 + "${copier[@]}" copy --quiet --vcs-ref v0.0.1 --defaults --data "domain=${domain}" --data "module=example.com/ncly-${slug}" "$@" "${src}" "${repo}" + fct_revision_links "${repo}" v0.0.1 + + # The reading order must send an author to the real-input acceptance criteria. + awk ' + /^## Read, in this order$/ { reading = 1; next } + reading && /^## / { reading = 0 } + reading && /^[0-9]+\. / && /## Acceptance/ { found = 1 } + END { exit !found } + ' "${repo}/AGENTS.md" || fct_fail 'AGENTS.md reading order must name ## Acceptance' + + # The author's go.mod points at this checkout, so the rendered repository tests the SDK and ncly of this change. + go -C "${repo}" mod edit -replace "github.com/pascalandy/nicely=${root}" + # The rendered go.mod repeats the replace of Lip Gloss of Nicely's go.mod, or drops it with Nicely's. + lipgloss="$(grep '^replace charm.land/lipgloss/v2 ' "${root}/go.mod" || true)" + [[ "$(grep '^replace charm.land/lipgloss/v2 ' "${repo}/go.mod" || true)" == "${lipgloss}" ]] || + fct_fail "the go.mod of the template does not repeat \"${lipgloss}\" of Nicely's go.mod" + go -C "${repo}" mod tidy + fct_has "${repo}/manifest.toml" "domain = \"${domain}\"" + fct_has "${repo}/extension.go" "package ${domain}" + if [[ -n "${name}" ]]; then + fct_has "${repo}/manifest.toml" "name = \"${name}\"" + else + [[ -z "$(sed -n '/^\[/q; /^name = /p' "${repo}/manifest.toml")" ]] || fct_fail 'the whole-domain manifest declares a name' + fi + [[ -f "${repo}/cmd/ncly-${slug}/main.go" ]] || fct_fail "the entry directory is not cmd/ncly-${slug}" + fct_test_files "${repo}" "${slug}" + git -C "${repo}" init -q + fct_commit "rendered from v0.0.1" + (cd "${repo}" && just check) + fct_test_gates "${repo}" "${scratch}" "${domain}" "${command}" + fct_test_acceptance_cases "${repo}" "${scratch}" + fct_test_finish "${repo}" "${scratch}" + fct_test_ledger "${repo}" "${scratch}" + fct_test_example_notices "${repo}" "${scratch}" "${slug}" + export XDG_CONFIG_HOME="${scratch}/config" XDG_STATE_HOME="${scratch}/state" XDG_DATA_HOME="${data}" NCLY_NO_INPUT=1 + (cd "${repo}" && just install) + for file in "ncly-${slug}" "ncly-${slug}.toml" "ncly-${slug}.locales/en.toml" "ncly-${slug}.skill/SKILL.md"; do + [[ -f "${extensions}/${file}" ]] || fct_fail "just install wrote no ${extensions}/${file}" + done + local command_words + read -r -a command_words <<<"${command}" + # The installed extension runs under ncly. /usr/bin comes first, so the shim of a version manager never answers for python3. + # go stays the caller's: the /usr/bin/go of the release runner is older than go.mod allows. + PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly "${command_words[@]}" "${repo}/go.mod" --json >"${scratch}/inspect.out" + grep -qF '"ok":true' "${scratch}/inspect.out" || fct_fail "the installed command failed: $(cat "${scratch}/inspect.out")" + PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly skill view "${slug}" >"${scratch}/skill.out" + fct_has "${scratch}/skill.out" "name: ${slug}" + grep -qF "ncly ${command}" "${scratch}/skill.out" || fct_fail 'the installed skill teaches the wrong command' + [[ "$(cd "${repo}" && just contract)" == "${root}/docs/north-star/contract" ]] || fct_fail "just contract does not print ${root}/docs/north-star/contract" + printf 'stale\n' >"${extensions}/ncly-${slug}.locales/stale.toml" + printf 'stale\n' >"${extensions}/ncly-${slug}.skill/stale.md" + printf 'stale\n' >"${extensions}/ncly-${slug}.toml" + printf 'stale\n' >"${extensions}/ncly-${slug}" + (cd "${repo}" && just install) + [[ ! -e "${extensions}/ncly-${slug}.locales/stale.toml" && ! -e "${extensions}/ncly-${slug}.skill/stale.md" ]] || fct_fail 'install keeps stale sidecars' + PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly "${command_words[@]}" "${repo}/go.mod" --json >"${scratch}/reinstalled.out" + grep -qF '"ok":true' "${scratch}/reinstalled.out" || fct_fail 'the replacement install does not run' + fct_test_uninstall "${repo}" "${slug}" "${data}" "${domain}${name:+ ${name}}" "${scratch}" + fct_test_acceptance "${repo}" "${scratch}" + fct_test_release "${repo}" "${scratch}" "${slug}" "${domain}${name:+ ${name}}" + fct_test_update "${src}" "${repo}" "${scratch}" "${root}" + if [[ -z "${name}" ]]; then + [[ -z "$(sed -n '/^\[/q; /^name = /p' "${repo}/manifest.toml")" ]] || fct_fail 'update changes the whole-domain default' + else + fct_has "${repo}/manifest.toml" "name = \"${name}\"" + fi + printf 'template %s passes checks, replaces its install, and preserves author edits through updates\n' "${slug}" +} diff --git a/scripts/template-test.sh b/scripts/template-test.sh index 826c0351..7de0cdc6 100644 --- a/scripts/template-test.sh +++ b/scripts/template-test.sh @@ -31,91 +31,13 @@ fct_commit() { git -C "${repo}" commit -qm "${1}" } -fct_repository() { - local scratch="${1}" domain="${2}" name="${3}" slug="${4}" command="${5}" src repo data extensions lipgloss file - shift 5 - src="${scratch}/source" repo="${scratch}/ncly-${slug}" data="${scratch}/data" - extensions="${data}/nicely/extensions" - mkdir -p "${scratch}" - # The module proxy and Copier source both hold this checkout, with its changes. - mkdir "${src}" - fct_copy_source "${src}" - git -C "${src}" init -q - fct_revision v0.0.1 - "${copier[@]}" copy --quiet --vcs-ref v0.0.1 --defaults --data "domain=${domain}" --data "module=example.com/ncly-${slug}" "$@" "${src}" "${repo}" - - # The reading order must send an author to the real-input acceptance criteria. - awk ' - /^## Read, in this order$/ { reading = 1; next } - reading && /^## / { reading = 0 } - reading && /^[0-9]+\. / && /## Acceptance/ { found = 1 } - END { exit !found } - ' "${repo}/AGENTS.md" || fct_fail 'AGENTS.md reading order must name ## Acceptance' - - # The author's go.mod points at this checkout, so the rendered repository tests the SDK and ncly of this change. - go -C "${repo}" mod edit -replace "github.com/pascalandy/nicely=${root}" - # The rendered go.mod repeats the replace of Lip Gloss of Nicely's go.mod, or drops it with Nicely's. - lipgloss="$(grep '^replace charm.land/lipgloss/v2 ' "${root}/go.mod" || true)" - [[ "$(grep '^replace charm.land/lipgloss/v2 ' "${repo}/go.mod" || true)" == "${lipgloss}" ]] || - fct_fail "the go.mod of the template does not repeat \"${lipgloss}\" of Nicely's go.mod" - go -C "${repo}" mod tidy - fct_has "${repo}/manifest.toml" "domain = \"${domain}\"" - fct_has "${repo}/extension.go" "package ${domain}" - if [[ -n "${name}" ]]; then - fct_has "${repo}/manifest.toml" "name = \"${name}\"" - else - [[ -z "$(sed -n '/^\[/q; /^name = /p' "${repo}/manifest.toml")" ]] || fct_fail 'the whole-domain manifest declares a name' - fi - [[ -f "${repo}/cmd/ncly-${slug}/main.go" ]] || fct_fail "the entry directory is not cmd/ncly-${slug}" - fct_test_files "${repo}" "${slug}" - git -C "${repo}" init -q - fct_commit "rendered from v0.0.1" - (cd "${repo}" && just check) - fct_test_gates "${repo}" "${scratch}" "${domain}" "${command}" - fct_test_acceptance_cases "${repo}" "${scratch}" - fct_test_finish "${repo}" "${scratch}" - fct_test_ledger "${repo}" "${scratch}" - fct_test_example_notices "${repo}" "${scratch}" "${slug}" - export XDG_CONFIG_HOME="${scratch}/config" XDG_STATE_HOME="${scratch}/state" XDG_DATA_HOME="${data}" NCLY_NO_INPUT=1 - (cd "${repo}" && just install) - for file in "ncly-${slug}" "ncly-${slug}.toml" "ncly-${slug}.locales/en.toml" "ncly-${slug}.skill/SKILL.md"; do - [[ -f "${extensions}/${file}" ]] || fct_fail "just install wrote no ${extensions}/${file}" - done - local command_words - read -r -a command_words <<<"${command}" - # The installed extension runs under ncly. /usr/bin comes first, so the shim of a version manager never answers for python3. - # go stays the caller's: the /usr/bin/go of the release runner is older than go.mod allows. - PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly "${command_words[@]}" "${repo}/go.mod" --json >"${scratch}/inspect.out" - grep -qF '"ok":true' "${scratch}/inspect.out" || fct_fail "the installed command failed: $(cat "${scratch}/inspect.out")" - PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly skill view "${slug}" >"${scratch}/skill.out" - fct_has "${scratch}/skill.out" "name: ${slug}" - grep -qF "ncly ${command}" "${scratch}/skill.out" || fct_fail 'the installed skill teaches the wrong command' - [[ "$(cd "${repo}" && just contract)" == "${root}/docs/north-star/contract" ]] || fct_fail "just contract does not print ${root}/docs/north-star/contract" - printf 'stale\n' >"${extensions}/ncly-${slug}.locales/stale.toml" - printf 'stale\n' >"${extensions}/ncly-${slug}.skill/stale.md" - printf 'stale\n' >"${extensions}/ncly-${slug}.toml" - printf 'stale\n' >"${extensions}/ncly-${slug}" - (cd "${repo}" && just install) - [[ ! -e "${extensions}/ncly-${slug}.locales/stale.toml" && ! -e "${extensions}/ncly-${slug}.skill/stale.md" ]] || fct_fail 'install keeps stale sidecars' - PATH="/usr/bin:${PATH}" "$(command -v go)" -C "${repo}" tool ncly "${command_words[@]}" "${repo}/go.mod" --json >"${scratch}/reinstalled.out" - grep -qF '"ok":true' "${scratch}/reinstalled.out" || fct_fail 'the replacement install does not run' - fct_test_uninstall "${repo}" "${slug}" "${data}" "${domain}${name:+ ${name}}" "${scratch}" - fct_test_acceptance "${repo}" "${scratch}" - fct_test_release "${repo}" "${scratch}" "${slug}" "${domain}${name:+ ${name}}" - fct_test_update "${src}" "${repo}" "${scratch}" "${root}" - if [[ -z "${name}" ]]; then - [[ -z "$(sed -n '/^\[/q; /^name = /p' "${repo}/manifest.toml")" ]] || fct_fail 'update changes the whole-domain default' - else - fct_has "${repo}/manifest.toml" "name = \"${name}\"" - fi - printf 'template %s passes checks, replaces its install, and preserves author edits through updates\n' "${slug}" -} - fct_main() { local root pin scratch copier template_src proxy cache_proxy root="$(cd "${BASH_SOURCE[0]%/*}/.." && pwd -P)" # shellcheck source=scripts/template-test-helpers.sh source "${root}/scripts/template-test-helpers.sh" + # shellcheck source=scripts/template-repository-test.sh + source "${root}/scripts/template-repository-test.sh" # shellcheck source=scripts/template-update-test.sh source "${root}/scripts/template-update-test.sh" # shellcheck source=scripts/template-adoption-test.sh @@ -153,6 +75,12 @@ fct_main() { proxy="${scratch}/proxy/github.com/pascalandy/nicely/@v" mkdir -p "${proxy}" cache_proxy="file://$(go env GOMODCACHE)/cache/download" + # Update drops GIT_CONFIG_GLOBAL too. Its children need the same private + # identity through HOME, while tool caches keep their existing locations. + export GOCACHE="${GOCACHE:-$(go env GOCACHE)}" UV_CACHE_DIR="${UV_CACHE_DIR:-$(uv cache dir)}" + export HOME="${scratch}/home" XDG_CONFIG_HOME="${scratch}/home/.config" + mkdir -p "${HOME}" + cp "${scratch}/gitconfig" "${HOME}/.gitconfig" export GOPROXY="file://${scratch}/proxy,${cache_proxy},${GOPROXY:-https://proxy.golang.org,direct}" export GOMODCACHE="${scratch}/modcache" GONOSUMDB=github.com/pascalandy/nicely # Go normally makes cached module folders read-only, which prevents scratch cleanup. diff --git a/scripts/template-update-test.sh b/scripts/template-update-test.sh index ed13389f..d84dd277 100644 --- a/scripts/template-update-test.sh +++ b/scripts/template-update-test.sh @@ -61,6 +61,7 @@ fct_test_update() { fct_has "${repo}/AGENTS.md" '# AGENTS.md of the author' fct_has "${repo}/AGENTS.md" 'This line comes from template v0.0.2.' fct_has "${repo}/.copier-answers.yml" '_commit: v0.0.2' + fct_revision_links "${repo}" v0.0.2 fct_has "${repo}/go.mod" "${dependency}" [[ "$(go -C "${repo}" list -m -f '{{.Version}}' github.com/pascalandy/nicely)" == v0.0.2 ]] || fct_fail 'the SDK pin did not move' fct_has "${repo}/go.mod" "replace github.com/pascalandy/nicely => ${root}" diff --git a/sdk/analysis/callerenv/callerenv.go b/sdk/analysis/callerenv/callerenv.go index 69060014..fdec9bea 100644 --- a/sdk/analysis/callerenv/callerenv.go +++ b/sdk/analysis/callerenv/callerenv.go @@ -1,6 +1,6 @@ -// Package callerenv refuses a read of the whole environment of the caller in -// the test kit, the repository gates, and the TLC runner. Only extensionrepo.NewOwnedRun reads -// it, as Analyzers in contract.md defines. +// Package callerenv refuses whole caller environment reads in the test kit, +// repository gates, TLC runner and their child packages. +// The exact allowed readers are listed below and in Analyzers of contract.md. // // At the parent of 09809608, the lifecycle check of the kit passed // os.Environ() to a scratch install, which replaced HOME and kept an invalid @@ -14,31 +14,42 @@ package callerenv import ( "go/ast" "go/types" + "slices" "strings" "golang.org/x/tools/go/analysis" ) -// Analyzer reports os.Environ and syscall.Environ outside the constructor of -// owned runs. +// Analyzer reports os.Environ and syscall.Environ outside the exact allowed +// readers in the owned package trees. var Analyzer = &analysis.Analyzer{ Name: "callerenv", - Doc: "refuse os.Environ in the test kit, the repository gates, and the TLC runner outside extensionrepo.NewOwnedRun", + Doc: "refuse whole caller environment reads in owned SDK package trees outside their exact allowed readers", Run: run, } -// owners maps each package that the rule reads to the one function there that -// may read the environment of the caller as a whole. An empty name allows -// none. -var owners = map[string]string{ - "github.com/pascalandy/nicely/sdk/extensionrepo": "NewOwnedRun", - "github.com/pascalandy/nicely/sdk/extensiontest": "", - "github.com/pascalandy/nicely/sdk/verification": "", +// Each key owns its package and descendants. Its allowed function names apply +// only to that exact package, never to descendants. +var owners = map[string][]string{ + "github.com/pascalandy/nicely/sdk/extensionrepo": {"NewOwnedRun", "updateEnviron"}, + "github.com/pascalandy/nicely/sdk/extensiontest": nil, + "github.com/pascalandy/nicely/sdk/verification": nil, + // stage preserves the isolated scenario environment across syscall.Exec. + "github.com/pascalandy/nicely/sdk/extensiontest/guard": {"stage"}, + // run implements the fixture's env action, which prints names, never values. + "github.com/pascalandy/nicely/sdk/extensiontest/fake": {"run"}, } func run(pass *analysis.Pass) (any, error) { - owner, ok := owners[pass.Pkg.Path()] - if !ok { + path := pass.Pkg.Path() + owned := false + for parent := range owners { + if path == parent || strings.HasPrefix(path, parent+"/") { + owned = true + break + } + } + if !owned { return nil, nil } for _, file := range pass.Files { @@ -46,7 +57,7 @@ func run(pass *analysis.Pass) (any, error) { continue } for _, decl := range file.Decls { - if fn, ok := decl.(*ast.FuncDecl); ok && fn.Recv == nil && owner != "" && fn.Name.Name == owner { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Recv == nil && slices.Contains(owners[path], fn.Name.Name) { continue } ast.Inspect(decl, func(node ast.Node) bool { diff --git a/sdk/analysis/callerenv/callerenv_test.go b/sdk/analysis/callerenv/callerenv_test.go index f198cbec..36164dc7 100644 --- a/sdk/analysis/callerenv/callerenv_test.go +++ b/sdk/analysis/callerenv/callerenv_test.go @@ -15,13 +15,13 @@ var root = filepath.Join("..", "..", "..") // 09809608, and notices.go on the go test of e35bd0bb. func TestCallerEnvironment(t *testing.T) { cases := "github.com/pascalandy/nicely/sdk/analysis/callerenv/testdata/cases" - owners[cases] = "NewOwnedRun" + owners[cases] = []string{"NewOwnedRun"} t.Cleanup(func() { delete(owners, cases) }) analysistest.Run(t, root, Analyzer, "./sdk/analysis/callerenv/testdata/cases") } // TestOwnedPackages requires that the kit, the repository gates, and the TLC -// runner read the environment of the caller only in NewOwnedRun. +// runner and their descendants use only their exact allowed readers. func TestOwnedPackages(t *testing.T) { - analysistest.Run(t, root, Analyzer, "./sdk/extensionrepo", "./sdk/extensiontest", "./sdk/verification") + analysistest.Run(t, root, Analyzer, "./sdk/extensionrepo/...", "./sdk/extensiontest/...", "./sdk/verification/...") } diff --git a/sdk/analysis/callerenv/scope_test.go b/sdk/analysis/callerenv/scope_test.go new file mode 100644 index 00000000..e00ed537 --- /dev/null +++ b/sdk/analysis/callerenv/scope_test.go @@ -0,0 +1,41 @@ +package callerenv + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestOwnedPathBoundaries(t *testing.T) { + const base = "github.com/pascalandy/nicely/sdk/" + dir, cleanup, err := analysistest.WriteFiles(map[string]string{ + base + "extensionrepo/child/env.go": `package child +import "os" +func NewOwnedRun() { _ = os.Environ() } // want "os.Environ" +`, + base + "extensiontestx/env.go": `package extensiontestx +import "os" +func other() { _ = os.Environ() } +`, + base + "extensiontest/guard/env.go": `package guard +import "os" +func stage() { _ = os.Environ() } +func other() { _ = os.Environ() } // want "os.Environ" +`, + base + "extensiontest/fake/env.go": `package fake +import "syscall" +func run() { _ = syscall.Environ() } +func other() { _ = syscall.Environ() } // want "syscall.Environ" +`, + base + "extensiontest/guard/child/env.go": `package child +import "os" +func stage() { _ = os.Environ() } // want "os.Environ" +`, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(cleanup) + analysistest.Run(t, dir, Analyzer, base+"extensionrepo/child", base+"extensiontestx", + base+"extensiontest/guard", base+"extensiontest/fake", base+"extensiontest/guard/child") +} diff --git a/sdk/extensionrepo/owned_run.go b/sdk/extensionrepo/owned_run.go index d0ef6d58..4171ae55 100644 --- a/sdk/extensionrepo/owned_run.go +++ b/sdk/extensionrepo/owned_run.go @@ -13,7 +13,8 @@ import ( // OwnedRun is the environment and the scratch folder of one verification // run, as Owned runs in the contract defines. The test kit, signoff, // release, and acceptance each build theirs with NewOwnedRun. The analyzer -// callerenv refuses os.Environ elsewhere in this package and the kit. +// callerenv permits whole caller reads only in the exact functions of +// Analyzers in the contract. type OwnedRun struct { // Dir is the folder of the run. A scratch run holds HOME, TMPDIR, and // the XDG folders there. A host run keeps its notices there. @@ -55,9 +56,8 @@ func dropped(name string) bool { } // NewOwnedRun builds the environment of a run in dir, which must be an -// absolute path, or empty for a host run without notices. It is the one -// function of the kit and the repository gates that reads the environment -// of the caller as a whole. +// absolute path, or empty for a host run without notices. Its profile decides +// which caller settings reach the programs of that run. // // Each run masks GOFLAGS with a blank value, which also masks one that go // env -w keeps, and sets GOWORK=off, unless Keep names them. A host run with diff --git a/sdk/extensionrepo/spec.go b/sdk/extensionrepo/spec.go index 6de595e6..1cc4cca4 100644 --- a/sdk/extensionrepo/spec.go +++ b/sdk/extensionrepo/spec.go @@ -1,10 +1,95 @@ package extensionrepo import ( + "fmt" "regexp" + "slices" "strings" ) +// SpecCheckOutputLimit requires one accepted Limits row with a description and +// the exact Output case. The matrix must bind that ID too. +func SpecCheckOutputLimit(spec, id, name string) error { + notAccepted := fmt.Errorf("limit %q is not accepted", id) + _, limits := specProblems(spec) + count, accepted := 0, false + for _, limit := range limits { + if limit.id == id { + count++ + accepted = limit.accepted + } + } + if count != 1 || !accepted { + return notAccepted + } + header := false + for _, line := range SpecLines(spec) { + if line.Part != "Limits" { + continue + } + switch line.Kind { + case SpecLabel, SpecHeading: + header = false + case SpecHeader: + header = slices.Equal(line.Cells, []string{"ID", "State", "Limit", "Accepted"}) + case SpecRow: + if line.ID != id { + continue + } + if !header || len(line.Cells) != 4 { + return notAccepted + } + if !specLimitDescription(line.Cells[2], id) { + return fmt.Errorf("limit %q is missing Limit text describing what is retained", id) + } + if specPlaceholder(line.Cells[3]) || !SpecNamesOutput(line.Cells[2], name) { + return notAccepted + } + return nil + } + } + return notAccepted +} + +// SpecNamesOutput reports whether a semicolon-separated cell declares the exact +// Output case. The Limit and matrix Source cells share this grammar. +func SpecNamesOutput(cell, name string) bool { + for _, part := range specParts(cell) { + if part == "Output: "+strings.ReplaceAll(name, "`", "") { + return true + } + } + return false +} + +func specParts(cell string) []string { + parts := strings.Split(strings.ReplaceAll(cell, "`", ""), ";") + for i := range parts { + parts[i] = strings.TrimSpace(parts[i]) + } + return parts +} + +func specLimitDescription(cell, id string) bool { + described := false + for _, part := range specParts(cell) { + text := strings.Trim(part, "* .-") + if text == "" || text == id || strings.HasPrefix(strings.ToLower(text), "output:") { + continue + } + if specPlaceholder(text) { + return false + } + described = true + } + return described +} + +func specPlaceholder(text string) bool { + text = strings.Trim(text, "`* .-") + return text == "" || strings.EqualFold(text, "None") || strings.HasPrefix(strings.ToLower(text), "fill:") +} + // SpecKind says what a line of spec.md is. type SpecKind int diff --git a/sdk/extensionrepo/update.go b/sdk/extensionrepo/update.go index 1d37da81..67b52f88 100644 --- a/sdk/extensionrepo/update.go +++ b/sdk/extensionrepo/update.go @@ -243,12 +243,28 @@ func updateBootstrap(version, repo string, resume bool) string { func updateRun(name string, args ...string) error { fmt.Fprintln(os.Stderr, strings.Join(append([]string{name}, args...), " ")) cmd := exec.Command(name, args...) + cmd.Env = updateEnviron() cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr return cmd.Run() } func updateOutput(name string, args ...string) ([]byte, error) { cmd := exec.Command(name, args...) + cmd.Env = updateEnviron() cmd.Stderr = os.Stderr return cmd.Output() } + +// updateEnviron builds the environment for programs started by updateRun or updateOutput. +// These helpers serve updates, repository checks, just contract and readiness Git probes. +// Other caller settings stay available. Every GIT_* variable is removed. +// A hook or rebase --exec can set one. +func updateEnviron() []string { + var env []string + for _, value := range os.Environ() { + if !strings.HasPrefix(value, "GIT_") { + env = append(env, value) + } + } + return env +} diff --git a/sdk/extensionrepo/update_environment_test.go b/sdk/extensionrepo/update_environment_test.go new file mode 100644 index 00000000..7f0a71f0 --- /dev/null +++ b/sdk/extensionrepo/update_environment_test.go @@ -0,0 +1,153 @@ +package extensionrepo + +import ( + "bytes" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// A Copier subprocess uses Git too. Under a hook's environment, even its -C +// can address the caller's repository. Both repositories here belong to this +// test, and the control reader never inherits the poisoned environment. +func TestUpdateDropsCallerGitEnvironment(t *testing.T) { + git, err := exec.LookPath("git") + if err != nil { + t.Skip("git is required") + } + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + home, bin := filepath.Join(root, "home"), filepath.Join(root, "bin") + for _, path := range []string{home, bin, filepath.Join(root, "hooks")} { + if err := os.MkdirAll(path, 0o700); err != nil { + t.Fatal(err) + } + } + controlEnv := []string{ + "PATH=" + os.Getenv("PATH"), "HOME=" + home, + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "LC_ALL=C", + } + control := func(repo string, args ...string) []byte { + t.Helper() + cmd := exec.Command(git, append([]string{"-C", repo}, args...)...) + cmd.Env = controlEnv + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("control git %v: %v\n%s", args, err, out) + } + return out + } + a, b := filepath.Join(root, "target"), filepath.Join(root, "other") + for _, repo := range []string{a, b} { + control(root, "init", "--quiet", repo) + control(repo, "config", "core.hooksPath", filepath.Join(root, "hooks")) + control(repo, "config", "user.name", "Update fixture") + control(repo, "config", "user.email", "fixture@example.com") + control(repo, "config", "commit.gpgsign", "false") + updateFixtureWrite(t, filepath.Join(repo, "tracked.txt"), filepath.Base(repo)+"\n", 0o600) + control(repo, "add", "tracked.txt") + control(repo, "commit", "--quiet", "-m", "fixture") + } + read := func(path string) []byte { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return data + } + snapshot := func() map[string][]byte { + return map[string][]byte{ + "refs": control(b, "for-each-ref"), "HEAD": read(filepath.Join(b, ".git", "HEAD")), + "index": read(filepath.Join(b, ".git", "index")), "config": read(filepath.Join(b, ".git", "config")), + "tracked": read(filepath.Join(b, "tracked.txt")), + } + } + before := snapshot() + targetHead := strings.TrimSpace(string(control(a, "rev-parse", "HEAD"))) + oldMod, newMod := filepath.Join(root, "old.mod"), filepath.Join(root, "new.mod") + for _, path := range []string{oldMod, newMod} { + updateFixtureWrite(t, path, "module github.com/pascalandy/nicely\n", 0o600) + } + oldJSON, err := json.Marshal(map[string]string{"Version": "v0.25.0", "GoMod": oldMod}) + if err != nil { + t.Fatal(err) + } + newJSON, err := json.Marshal(map[string]string{"GoMod": newMod}) + if err != nil { + t.Fatal(err) + } + // The stubs record names only. Their output lives outside either repository, + // so checking the initial clean tree cannot mask the regression. + preamble := `#!/bin/sh +set -eu +env | sed -n 's/^\(GIT_[^=]*\)=.*/\1/p' >> "$NCLY_UPDATE_REPORT" +test "$NCLY_UPDATE_KEEP" = kept +` + updateFixtureWrite(t, filepath.Join(bin, "uv"), preamble+` +test "$*" = 'tool run copier@9.19.1 update --defaults --conflict inline --vcs-ref v0.99.0' +"$NCLY_UPDATE_GIT" -C "$NCLY_UPDATE_TARGET" update-ref refs/heads/copier-probe HEAD +"$NCLY_UPDATE_GIT" -C "$NCLY_UPDATE_TARGET" rev-parse --show-toplevel > "$NCLY_UPDATE_TOP" +printf 'copied\n' > "$NCLY_UPDATE_TARGET/copied.txt" +`, 0o700) + updateFixtureWrite(t, filepath.Join(bin, "go"), preamble+` +case "$*" in + 'list -m -json github.com/pascalandy/nicely') printf '%s\n' "$NCLY_UPDATE_OLD" ;; + 'mod download -json github.com/pascalandy/nicely@v0.99.0') printf '%s\n' "$NCLY_UPDATE_NEW" ;; + 'mod edit -json '*) printf '{"Replace":[]}\n' ;; + 'get github.com/pascalandy/nicely@v0.99.0') printf 'v0.99.0\n' > "$NCLY_UPDATE_TARGET/pin.txt" ;; + 'mod tidy') ;; + *) exit 91 ;; +esac +`, 0o700) + updateFixtureWrite(t, filepath.Join(bin, "just"), preamble+` +test "$*" = check +printf 'checked\n' > "$NCLY_UPDATE_TARGET/checked.txt" +`, 0o700) + values := map[string]string{ + "HOME": home, "XDG_CONFIG_HOME": home, "PATH": bin + string(os.PathListSeparator) + os.Getenv("PATH"), + "GIT_DIR": filepath.Join(b, ".git"), "GIT_WORK_TREE": b, "GIT_INDEX_FILE": filepath.Join(b, ".git", "index"), + "GIT_POISON": "sentinel", "NCLY_UPDATE_KEEP": "kept", "NCLY_UPDATE_GIT": git, + "NCLY_UPDATE_TARGET": a, "NCLY_UPDATE_OLD": string(oldJSON), "NCLY_UPDATE_NEW": string(newJSON), + "NCLY_UPDATE_REPORT": filepath.Join(root, "git-names"), + "NCLY_UPDATE_TOP": filepath.Join(root, "top"), + } + for name, value := range values { + t.Setenv(name, value) + } + t.Chdir(a) + if code := update([]string{"v0.99.0", a}); code != 0 { + t.Errorf("update exit = %d", code) + } + for name, data := range snapshot() { + if !bytes.Equal(data, before[name]) { + t.Errorf("second repository changed: %s", name) + } + } + if names := string(read(values["NCLY_UPDATE_REPORT"])); names != "" { + t.Errorf("update programs inherited Git variable names:\n%s", names) + } + if top := strings.TrimSpace(string(read(values["NCLY_UPDATE_TOP"]))); top != a { + t.Errorf("Copier's Git addressed %q, want %q", top, a) + } + if refs := string(control(a, "for-each-ref", "--format=%(objectname)", "refs/heads/copier-probe")); strings.TrimSpace(refs) != targetHead { + t.Errorf("Copier did not update the target ref: %q", refs) + } + for name, want := range map[string]string{"copied.txt": "copied\n", "pin.txt": "v0.99.0\n", "checked.txt": "checked\n"} { + if got := string(read(filepath.Join(a, name))); got != want { + t.Errorf("target %s = %q, want %q", name, got, want) + } + } +} + +func updateFixtureWrite(t *testing.T, path, content string, mode os.FileMode) { + t.Helper() + if err := os.WriteFile(path, []byte(content), mode); err != nil { + t.Fatal(err) + } +} diff --git a/sdk/extensiontest/acceptance.go b/sdk/extensiontest/acceptance.go index 4346f23c..4afdfd22 100644 --- a/sdk/extensiontest/acceptance.go +++ b/sdk/extensiontest/acceptance.go @@ -10,25 +10,32 @@ import ( "slices" "strings" + "github.com/pascalandy/nicely/sdk/extensionrepo" "github.com/rogpeppe/go-internal/testscript" ) // A readCase is the arguments of one reads call. type readCase struct { - name string + name, limit string private []string output, reference, bad string reader []string } -// parseReads reads reads [-case ] [-private ]... +// parseReads reads reads [-case ] [-limit ] [-private ]... // [...]. func parseReads(args []string) (readCase, bool) { var c readCase - for len(args) > 1 && (args[0] == "-case" || args[0] == "-private") { - if args[0] == "-case" { + for len(args) > 1 && (args[0] == "-case" || args[0] == "-limit" || args[0] == "-private") { + switch args[0] { + case "-case": c.name = caseName(args[1]) - } else { + case "-limit": + if c.limit != "" || args[1] == "" { + return c, false + } + c.limit = args[1] + case "-private": c.private = append(c.private, args[1]) } args = args[2:] @@ -46,7 +53,7 @@ func parseReads(args []string) (readCase, bool) { func (k *kit) reads(ts *testscript.TestScript, neg bool, args []string) { c, ok := parseReads(args) if neg || !ok { - ts.Fatalf("usage: reads [-case ] [-private ]... [...]") + ts.Fatalf("usage: reads [-case ] [-limit ] [-private ]... [...]") } direct := "ncly-" + k.manifest.Slug() if !k.acceptance.wasCalled(ts) { @@ -56,6 +63,9 @@ func (k *kit) reads(ts *testscript.TestScript, neg bool, args []string) { ts.Fatalf("reads: the reader %s is the extension or a program that it wraps, so it can share the bug of the handler. "+ "Fix: read the output with another program, such as a decoder of its format", c.reader[0]) } + if c.limit != "" && !slices.Contains(privateCases, c.name) { + ts.Fatalf("reads: -limit needs a privacy Output case. Fix: use -case with a privacy case and its accepted limit ID") + } if c.name != "" { if !slices.ContainsFunc(outputCases, func(name string) bool { return caseName(name) == c.name }) { ts.Fatalf("reads: -case %q names no case of the Output group of Generic cases. Fix: give its name, such as %q", c.name, caseName(outputCases[5])) @@ -63,7 +73,15 @@ func (k *kit) reads(ts *testscript.TestScript, neg bool, args []string) { if !slices.ContainsFunc(k.outputs, func(o outputCase) bool { return o.name == c.name }) { ts.Fatalf("reads: ## Case matrix of spec.md names no family from the Output case %q. Fix: add the row of its family, with the Source Output: %s", c.name, c.name) } - if slices.Contains(privateCases, c.name) && len(c.private) == 0 { + if c.limit != "" { + if err := extensionrepo.SpecCheckOutputLimit(k.spec, c.limit, c.name); err != nil { + ts.Fatalf("reads: %v. Fix: describe the retained data and declare its Output case and acceptance in Limits", err) + } + } + if c.limit != "" && !slices.ContainsFunc(k.outputs, func(o outputCase) bool { return o.name == c.name && slices.Contains(o.limits, c.limit) }) { + ts.Fatalf("reads: limit %q is not accepted for every matrix row of the Output case %q. Fix: declare the Output case in the accepted Limit row and name its ID in Accepted for every applicable matrix row", c.limit, c.name) + } + if slices.Contains(privateCases, c.name) && len(c.private) == 0 && c.limit == "" { ts.Fatalf("reads: the case %q needs the private text of its test file, given with -private. Fix: put private text in the block, and give it with -private", c.name) } } diff --git a/sdk/extensiontest/acceptance_matrix.go b/sdk/extensiontest/acceptance_matrix.go index 96c72721..8f024f57 100644 --- a/sdk/extensiontest/acceptance_matrix.go +++ b/sdk/extensiontest/acceptance_matrix.go @@ -36,8 +36,9 @@ func caseName(text string) string { return strings.ReplaceAll(text, "`", "") } // An outputCase is an Output case that a cell Source of ## Case matrix in // spec.md names, at its line. type outputCase struct { - name string - line int + name string + line int + limits []string } // acceptance is what reads records across parallel scenarios, as @@ -86,6 +87,12 @@ func (a *acceptance) wasProved(name string) bool { // cell can name more than one. func matrixOutputs(spec string) []outputCase { var found []outputCase + var limitIDs []string + for _, line := range extensionrepo.SpecLines(spec) { + if line.Part == "Limits" && line.ID != "" { + limitIDs = append(limitIDs, line.ID) + } + } header := false for _, l := range extensionrepo.SpecLines(spec) { switch { @@ -94,8 +101,21 @@ func matrixOutputs(spec string) []outputCase { header = slices.Equal(l.Cells, []string{"Family", "Source", "Accepted", "Refused"}) case l.Kind == extensionrepo.SpecRow && header && len(l.Cells) == 4 && strings.Contains(l.Cells[1], "Output"): for _, name := range outputCases { - if strings.Contains(caseName(l.Cells[1]), caseName(name)) && !slices.ContainsFunc(found, func(c outputCase) bool { return c.name == caseName(name) }) { - found = append(found, outputCase{name: caseName(name), line: l.Number}) + if !strings.Contains(caseName(l.Cells[1]), caseName(name)) { + continue + } + var bound []string + if extensionrepo.SpecNamesOutput(l.Cells[1], name) { + for _, id := range limitIDs { + if strings.Contains(l.Cells[2], "`"+id+"`") && extensionrepo.SpecCheckOutputLimit(spec, id, name) == nil { + bound = append(bound, id) + } + } + } + if index := slices.IndexFunc(found, func(c outputCase) bool { return c.name == caseName(name) }); index >= 0 { + found[index].limits = slices.DeleteFunc(found[index].limits, func(id string) bool { return !slices.Contains(bound, id) }) + } else { + found = append(found, outputCase{name: caseName(name), line: l.Number, limits: slices.Clone(bound)}) } } } diff --git a/sdk/extensiontest/extensiontest.go b/sdk/extensiontest/extensiontest.go index 9d4c1153..0f7eaf2f 100644 --- a/sdk/extensiontest/extensiontest.go +++ b/sdk/extensiontest/extensiontest.go @@ -18,7 +18,7 @@ // protect ... selects paths that refuses and the next signal or appears keep // refuses checks the same refusal in a dry run and a run // predicts ... checks that a dry run predicts the publication of its run -// reads [-case ] [-private ]... [...] +// reads [-case ] [-limit ] [-private ]... [...] // reads the output with an independent reader, as Acceptance cases defines // keeps checks the exact exit, the protected paths, and the children of any call // holds checks the public path while the worker waits, then the commit @@ -108,6 +108,8 @@ type kit struct { ext extension.Extension // root is the folder of the repository's go.mod. root string + // spec is the same spec.md snapshot that links and reads validate. + spec string // bin holds ncly, which go tool ncly built. bin string // built is the ncly- that go build made, and fake the fake diff --git a/sdk/extensiontest/links.go b/sdk/extensiontest/links.go index 38f5f659..1492c549 100644 --- a/sdk/extensiontest/links.go +++ b/sdk/extensiontest/links.go @@ -32,7 +32,8 @@ func (k *kit) links() []string { problems = append(problems, k.specLinks(scripts)...) } if spec, err := os.ReadFile(filepath.Join(k.root, "spec.md")); err == nil { - k.outputs = matrixOutputs(string(spec)) + k.spec = string(spec) + k.outputs = matrixOutputs(k.spec) } publications, reasons, publicationProblems := k.readPublication() k.publications, k.reasons = publications, reasons diff --git a/sdk/extensiontest/privacy_limits_test.go b/sdk/extensiontest/privacy_limits_test.go new file mode 100644 index 00000000..ac6210e7 --- /dev/null +++ b/sdk/extensiontest/privacy_limits_test.go @@ -0,0 +1,151 @@ +package extensiontest_test + +import ( + "fmt" + "strings" + "testing" +) + +const privacyLimitSpec = ` +## Case matrix + +| Family | Source | Accepted | Refused | +|---|---|---|---| +| Retained SVG metadata | Output: Keeps private data of the input; Output: Keeps data that a reader needs | ` + "`private-metadata`" + ` | None | + +**Limits.** + +| ID | State | Limit | Accepted | +|---|---|---|---| +| ` + "`private-metadata`" + ` | limit | Output: Keeps private data of the input; Output: Keeps data that a reader needs; Retain author, capture date, and location | Pascal, 2026-11-03, issue #462 | +| ` + "`unrelated`" + ` | limit | Network calls can be slow on a hotel network | Pascal, 2026-11-03, issue #462 | +` + +const retainedSVG = ` +Ana Lima2026-11-0345.5,-73.6 + + +` + +const privacyReader = `package main + +import ( + "encoding/xml" + "fmt" + "os" +) + +func main() { + data, err := os.ReadFile(os.Args[1]) + if err != nil { panic(err) } + var svg struct { + ViewBox string ` + "`xml:\"viewBox,attr\"`" + ` + Metadata struct { + Author string ` + "`xml:\"author\"`" + ` + Date string ` + "`xml:\"date\"`" + ` + GPS string ` + "`xml:\"gps\"`" + ` + } ` + "`xml:\"metadata\"`" + ` + Group struct { Transform string ` + "`xml:\"transform,attr\"`" + ` } ` + "`xml:\"g\"`" + ` + } + if err := xml.Unmarshal(data, &svg); err != nil { panic(err) } + fmt.Printf("author: %s\ndate: %s\ngps: %s\nviewBox: %s\ntransform: %s\n", svg.Metadata.Author, svg.Metadata.Date, svg.Metadata.GPS, svg.ViewBox, svg.Group.Transform) +} +` + +const privacyReference = "author: Ana Lima\ndate: 2026-11-03\ngps: 45.5,-73.6\nviewBox: 0 0 40 20\ntransform: rotate(90)\n" + +func privacyScript(reads string) string { + return "env BUG=raw\nncly video convert privacy.md --json\n" + reads + + "\n-- privacy.md --\n" + retainedSVG + + "-- privacy-reference.txt --\n" + privacyReference + + "-- privacy-bad.txt --\n" + strings.ReplaceAll(retainedSVG, "rotate(90)", "rotate(0)") + + "-- read-privacy.go --\n" + privacyReader +} + +func TestAcceptedPrivacyLimitKeepsIndependentProof(t *testing.T) { + reads := "reads -case 'Keeps private data of the input' -limit private-metadata privacy.txt privacy-reference.txt privacy-bad.txt go run read-privacy.go\n" + + "reads -case 'Keeps data that a reader needs' -limit private-metadata privacy.txt privacy-reference.txt privacy-bad.txt go run read-privacy.go\n" + if out, err := acceptanceFixture(t, privacyLimitSpec, map[string]string{"privacy": privacyScript(reads)}); err != nil { + t.Fatalf("accepted privacy limit failed: %v\n%s", err, out) + } +} + +func TestPrivacyLimitsRejectMissingOrUnrelatedApproval(t *testing.T) { + const declared = "Output: Keeps private data of the input; Retain author" + missingText := map[string]string{ + "empty": "", + "whitespace": " \t ", + "id-only": "`id-only`", + "binding-only": "Output: Keeps private data of the input", + "binding-id-only": "Output: Keeps private data of the input; `binding-id-only`", + "binding-none": "Output: Keeps private data of the input; None.", + "binding-fill-first": "Fill: what is retained; Output: Keeps private data of the input; Retain author", + "binding-fill-last": "Output: Keeps private data of the input; Retain author; fill: acceptance details", + "binding-none-text": "Output: Keeps private data of the input; None.; Retain author", + } + invalid := strings.Replace(privacyLimitSpec, "`private-metadata` | None |", "`private-metadata`, `pending`, `wrong-state`, `duplicate`, `placeholder`, `unrelated`, `wrong-case` | None |", 1) + + "| `pending` | limit | " + declared + " | |\n" + + "| `wrong-state` | open | " + declared + " | Pascal, 2026-11-03, issue #462 |\n" + + "| `duplicate` | limit | " + declared + " | Pascal, 2026-11-03, issue #462 |\n" + + "| `duplicate` | limit | " + declared + " | Pascal, 2026-11-03, issue #462 |\n" + + "| `placeholder` | limit | " + declared + " | None. |\n" + + "| `wrong-case` | limit | Output: Loses data that changes the meaning; Retain author | Pascal, 2026-11-03, issue #462 |\n" + for id, text := range missingText { + invalid = strings.Replace(invalid, "`private-metadata`,", "`"+id+"`, `private-metadata`,", 1) + invalid += fmt.Sprintf("| `%s` | limit | %s | Pascal, 2026-11-03, issue #462 |\n", id, text) + } + const tail = " privacy.txt privacy-reference.txt privacy-bad.txt go run read-privacy.go\n" + scripts := map[string]string{} + for _, id := range []string{"pending", "wrong-state", "duplicate", "placeholder", "missing", "unrelated", "wrong-case"} { + scripts[id] = privacyScript("reads -case 'Keeps private data of the input' -limit " + id + tail) + } + for id := range missingText { + scripts[id] = privacyScript("reads -case 'Keeps private data of the input' -limit " + id + tail) + } + scripts["strict"] = privacyScript("reads -case 'Keeps private data of the input' -limit private-metadata -private 'Ana Lima'" + tail) + scripts["opt-in"] = privacyScript("reads -case 'Keeps private data of the input'" + tail) + scripts["bad"] = privacyScript("reads -case 'Keeps private data of the input' -limit private-metadata privacy.txt privacy-reference.txt privacy.txt go run read-privacy.go\n") + scripts["nonprivacy"] = privacyScript("reads -case 'Loses data that changes the meaning' -limit private-metadata" + tail) + scripts["repeated"] = privacyScript("reads -case 'Keeps private data of the input' -limit missing -limit private-metadata" + tail) + out, err := acceptanceFixture(t, invalid, scripts) + if err == nil { + t.Fatal("invalid privacy bindings passed") + } + for _, id := range []string{"pending", "wrong-state", "duplicate", "placeholder", "missing", "unrelated", "wrong-case"} { + if want := `reads: limit "` + id + `" is not accepted`; !strings.Contains(out, want) { + t.Errorf("missing rejection %q:\n%s", want, out) + } + } + for id := range missingText { + if want := `reads: limit "` + id + `" is missing Limit text`; !strings.Contains(out, want) { + t.Errorf("missing rejection %q:\n%s", want, out) + } + } + for _, want := range []string{ + `privacy.txt holds the private text "Ana Lima"`, + `needs the private text of its test file`, + `reads: the bad variant privacy.txt passes`, + `reads: -limit needs a privacy Output case`, + `usage: reads [-case ] [-limit ]`, + } { + if !strings.Contains(out, want) { + t.Errorf("missing rejection %q:\n%s", want, out) + } + } +} + +func TestPrivacyLimitsRequireEveryExactCaseBinding(t *testing.T) { + for name, spec := range map[string]string{ + "another family": strings.Replace(privacyLimitSpec, "\n**Limits.**", "\n| Another family | Output: Keeps private data of the input | `unrelated` | None |\n\n**Limits.**", 1), + "case in prose": strings.Replace(privacyLimitSpec, "Output: Keeps private data of the input; Output: Keeps data that a reader needs", "Output: Loses data that changes the meaning; note: Keeps private data of the input; note: Keeps data that a reader needs", 1), + } { + t.Run(name, func(t *testing.T) { + reads := "reads -case 'Keeps private data of the input' -limit private-metadata privacy.txt privacy-reference.txt privacy-bad.txt go run read-privacy.go\n" + out, err := acceptanceFixture(t, spec, map[string]string{"privacy": privacyScript(reads)}) + want := `reads: limit "private-metadata" is not accepted for every matrix row` + if err == nil || !strings.Contains(out, want) { + t.Fatalf("ambiguous case binding passed or failed elsewhere: %v\n%s", err, out) + } + }) + } +} diff --git a/sdk/runner/cleanup.go b/sdk/runner/cleanup.go new file mode 100644 index 00000000..44f62163 --- /dev/null +++ b/sdk/runner/cleanup.go @@ -0,0 +1,26 @@ +package runner + +import ( + "syscall" + "time" +) + +// waitCleanup reserves a tail of the existing deadline for the direct program +// after forcing its descendants to stop. The final deadline never moves. +func waitCleanup(exited <-chan struct{}, program int, ends time.Time, reserve time.Duration) { + reserve = min(max(reserve, 0), Cleanup) + if reserve > 0 { + select { + case <-exited: + return + case <-time.After(time.Until(ends) - reserve): + signalTree(syscall.SIGKILL, program) + } + } + select { + case <-exited: + case <-time.After(time.Until(ends)): + signalTree(syscall.SIGKILL, 0) + <-exited + } +} diff --git a/sdk/runner/runner.go b/sdk/runner/runner.go index cd6b7ef1..19574707 100644 --- a/sdk/runner/runner.go +++ b/sdk/runner/runner.go @@ -60,6 +60,10 @@ type Program struct { // Completion also bounds output reading by Limit and skips cleanup on // cancellation, as a list command that completion runs requires. Completion bool + // CleanupReserve leaves this much of Cleanup for the direct program after + // force-stopping its descendants. Zero keeps the ordinary single deadline. + // The reservation is clamped to zero through Cleanup; it never extends it. + CleanupReserve time.Duration } // Result is how a program ended. @@ -168,7 +172,7 @@ func Run(ctx context.Context, p Program) (Result, error) { if stopped { result.Signal = SignalOf(ctx) } - signalTree(syscall.SIGKILL) + signalTree(syscall.SIGKILL, 0) <-exited drain(streams, time.Now()) result.Exit = exitCode(cmd.ProcessState) @@ -178,13 +182,8 @@ func Run(ctx context.Context, p Program) (Result, error) { if stopped { result.Signal = SignalOf(ctx) cleanupEnds = time.Now().Add(Cleanup) - signalTree(result.Signal) - select { - case <-exited: - case <-time.After(Cleanup): - signalTree(syscall.SIGKILL) - <-exited - } + signalTree(result.Signal, 0) + waitCleanup(exited, cmd.Process.Pid, cleanupEnds, p.CleanupReserve) } var stopping sync.WaitGroup if result.Signal != nil { @@ -196,7 +195,7 @@ func Run(ctx context.Context, p Program) (Result, error) { // by its execution limit and then ends what still runs. if p.Completion && p.Limit > 0 && result.Signal == nil { drain(streams, limitEnds) - signalTree(syscall.SIGKILL) + signalTree(syscall.SIGKILL, 0) } else { drain(streams, exitedAt.Add(DrainLimit)) } @@ -215,7 +214,7 @@ const signalSettle = 25 * time.Millisecond func stopDescendants(cleanupEnds time.Time) { for len(platform.Descendants(os.Getpid())) > 0 { if time.Now().After(cleanupEnds) { - signalTree(syscall.SIGKILL) + signalTree(syscall.SIGKILL, 0) return } time.Sleep(50 * time.Millisecond) @@ -229,13 +228,16 @@ func stopDescendants(cleanupEnds time.Time) { // macOS, so a command that a signal ends while it waits for one calls it. func Stop(sig os.Signal) { cleanupEnds := time.Now().Add(Cleanup) - signalTree(sig) + signalTree(sig, 0) stopDescendants(cleanupEnds) } -// signalTree sends sig to every descendant of ncly. -func signalTree(sig os.Signal) { +// signalTree sends sig to every descendant of ncly except keep, when nonzero. +func signalTree(sig os.Signal, keep int) { for _, pid := range platform.Descendants(os.Getpid()) { + if pid == keep { + continue + } if process, err := os.FindProcess(pid); err == nil { _ = process.Signal(sig) // On Linux, FindProcess opens a pidfd that only Release or the diff --git a/template/.agents/skills/nicely-extension/SKILL.md b/template/.agents/skills/nicely-extension/SKILL.md index e40ccb2a..a48c296e 100644 --- a/template/.agents/skills/nicely-extension/SKILL.md +++ b/template/.agents/skills/nicely-extension/SKILL.md @@ -26,6 +26,10 @@ Read [intake.md](intake.md) before you write code. It gives the facts to get fro - Declare each effect, program, and key in `manifest.toml`. When you cannot tell whether an effect occurs, declare it - Give `--dry-run` to a command that writes or bills. The dry run shares the preparation and starts no effect +Under `ncly`, cleanup after a resistant worker has a nominal two-second reservation inside the host's 15-second bound. +Allow for force-stop and exit overhead. +Read `programs-that-ncly-runs/cancellation.md` in the pinned contract before designing staging cleanup. + ## Error mapping Read [error-mapping.md](error-mapping.md) before you map the failures of the wrapped program. It says which failures `sdk/wrap` answers, and how the handler maps the rest. @@ -48,8 +52,12 @@ The tests of the extension use files that the tests make, such as a text file th Before the first review, each command that writes an output gets an acceptance case. Its test file has a block that a reader needs (#423): - The block uses a value that is not the default, such as `encoding: latin-1`, and it holds private text, such as `author: Ana Lima` -- An independent reader proves both results: the private text is not in the output, and the output still shows the content correctly -- Its bad variant is the output of a handler that keeps the raw block. `reads` with `-private` requires that this variant fails +- An independent reader proves that the content shows correctly and matches the declared privacy behavior +- Without an accepted privacy limit, `reads -private` proves that private text is absent and rejects a bad variant that keeps the raw block + +For an accepted limit, follow Accepted privacy limits in the pinned contract's `acceptance-cases.md` before using `reads -limit`. +Describe the retained data in its Limit cell. The ID or Output bindings alone do not supply that text. +Keep the complete retained-metadata reference, nondefault display value, and a bad variant that fails. The tests and `just acceptance` need no root, no mount, and no system service. Each experiment uses a new scratch folder, such as one that `mktemp -d` makes. diff --git a/template/.agents/skills/nicely-extension/replace-a-file.md b/template/.agents/skills/nicely-extension/replace-a-file.md index 82917237..f0a6201b 100644 --- a/template/.agents/skills/nicely-extension/replace-a-file.md +++ b/template/.agents/skills/nicely-extension/replace-a-file.md @@ -49,4 +49,5 @@ Prove these invariants of each command that writes a user file with the commands A `protect` before a `signal`, `appears`, `keeps`, or `holds` call covers that one call. For `appears`, the stub of the wrapped program writes the marker, then waits until the file exists. -For a command that replays paid work, run `create-formal-verification-skill`. +For a command that replays paid work, write a bounded TLA+ model of its run and replay invariants. +Run it with `just verify-models`, and keep its model, mapping, probes, and expected results. diff --git a/template/.agents/skills/nicely-extension/update.md b/template/.agents/skills/nicely-extension/update.md.jinja similarity index 94% rename from template/.agents/skills/nicely-extension/update.md rename to template/.agents/skills/nicely-extension/update.md.jinja index 528e6684..07ed31e8 100644 --- a/template/.agents/skills/nicely-extension/update.md +++ b/template/.agents/skills/nicely-extension/update.md.jinja @@ -21,7 +21,7 @@ A Markdown `readiness.md` of an older template stays in the Git history. Finish Some failures come from the SDK or the template, not from the extension. For example, `sdk/wrap` writes the timeout answer, so an extension could not add the input file to it (#407). -When only Nicely can fix a failure, do the step of [D068](https://github.com/pascalandy/nicely/blob/main/docs/north-star/decisions/D068-verification-map.md#move-a-correction): +When only Nicely can fix a failure, do the step of [D068](https://github.com/pascalandy/nicely/blob/{{ _commit }}/docs/north-star/decisions/D068-verification-map.md#move-a-correction): 1. Open an issue in pascalandy/nicely with the input, the host, and the failure. 2. Record a limit under **Limits.** of `spec.md` that links the issue. The user accepts it like any other limit, as `readiness-review.md` says. diff --git a/template/AGENTS/rule-index.md.jinja b/template/AGENTS/rule-index.md.jinja index c7ec36c6..6d970e1a 100644 --- a/template/AGENTS/rule-index.md.jinja +++ b/template/AGENTS/rule-index.md.jinja @@ -33,30 +33,31 @@ The linked checks enforce these rules. | A failure after the commit keeps the new output and reports the failure | Prose only | [The build loop](rule-index/loop.md) holds the rules of the rounds of the `nicely-extension` skill. +[Acceptance proof](rule-index/acceptance.md) holds the rules for its input files, independent readers, and matrix coverage. -[calls]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/calls.go -[links]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/links.go -[examples]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/examples.go -[declarations]: https://github.com/pascalandy/nicely/blob/main/sdk/extension/extension.go -[codes]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/codes.go -[refuses]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/paired.go -[signal]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/signal.go -[appears]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/appears.go -[spec]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/spec.go -[coverage]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/coverage_report.go -[protected]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/protected.go -[publication]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/publication.go -[publication-row]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/publication_row.go -[publication-report]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/publication_report.go -[predicts]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/predicts.go -[reads]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/acceptance.go -[keeps]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/keeps.go -[holds]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/holds.go -[guarded]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/guarded.go -[guard]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/guard/main.go +[calls]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/calls.go +[links]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/links.go +[examples]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/examples.go +[declarations]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extension/extension.go +[codes]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/codes.go +[refuses]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/paired.go +[signal]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/signal.go +[appears]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/appears.go +[spec]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/spec.go +[coverage]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/coverage_report.go +[protected]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/protected.go +[publication]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/publication.go +[publication-row]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/publication_row.go +[publication-report]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/publication_report.go +[predicts]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/predicts.go +[reads]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/acceptance.go +[keeps]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/keeps.go +[holds]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/holds.go +[guarded]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/guarded.go +[guard]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/guard/main.go -[signoff]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/signoff.go -[signoff-receipt]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/signoff_receipt.go -[release]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/release.go -[release-receipt]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/release_receipt.go -[template-test]: https://github.com/pascalandy/nicely/blob/main/scripts/template-test.sh +[signoff]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/signoff.go +[signoff-receipt]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/signoff_receipt.go +[release]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/release.go +[release-receipt]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/release_receipt.go +[template-test]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/scripts/template-test.sh diff --git a/template/AGENTS/rule-index/acceptance.md.jinja b/template/AGENTS/rule-index/acceptance.md.jinja new file mode 100644 index 00000000..3e9c4b68 --- /dev/null +++ b/template/AGENTS/rule-index/acceptance.md.jinja @@ -0,0 +1,18 @@ +# Acceptance proof + +This part of the [rule index](../rule-index.md) holds the acceptance rules of the build loop. + +| Rule | Enforced by | +|---|---| +| Before the first review, a test file holds a block that a reader needs, with a value that is not the default and private text | [Kit `reads`][reads] needs private text unless an accepted limit describes retained data and its [privacy declaration][privacy-limits] applies. Each matrix Output case needs [a case][reads-matrix]. Round 1 runs `just check` before review. The reviewer judges the value and the limit's meaning | +| An independent reader proves the declared privacy behavior and that the content shows correctly. A handler that violates that rule fails | [Kit `reads`][reads] searches private text unless an [accepted privacy limit][privacy-limits] applies, and compares the full reference. A bad variant fails the declared check. For a private-text search, the bad variant holds the private text | +| The tests and `just acceptance` need no root, no mount, and no system service | Prose only | +| Each case of the Output group that applies has a test file that the tests make, checked by an independent reader | [Kit Output cases][reads-matrix] for each case that `## Case matrix` names; partial: the reviewer checks that the matrix names each case that applies | +| The tests make their files; never search the home folder or copy a file of the user | [Kit `reads`][reads] reads only files of the folder of the scenario; partial: no isolation of the system hides the files of the user | +| Round 1 tests each generic case that applies, and the first review tests families of cases | Prose on purpose: no manifest lists every family of a domain, so only the reviewer finds a missing one | +| Before the review of round 1, `## Case matrix` gives each family its tests, and each line of Failures and Acceptance a family | [Finish matrix][finish-matrix] checks the references only, not that the matrix is complete or correct | + +[reads]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/acceptance.go +[reads-matrix]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensiontest/acceptance_matrix.go +[privacy-limits]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/docs/north-star/contract/acceptance-cases.md#accepted-privacy-limits +[finish-matrix]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/spec_matrix.go diff --git a/template/AGENTS/rule-index/loop.md.jinja b/template/AGENTS/rule-index/loop.md.jinja index 79a0e175..ef71ec68 100644 --- a/template/AGENTS/rule-index/loop.md.jinja +++ b/template/AGENTS/rule-index/loop.md.jinja @@ -13,14 +13,7 @@ This part of the [rule index](../rule-index.md) holds the rules of the rounds of | Each settled fact of the intake links the part read, at a fixed revision; a fact without a read is marked `inference` | The [intake record][intake] gives each fact one kind, and only a `sourced` fact a source | | Each unknown fact of the intake is an open line with an ID before the spec | The [intake record][intake] and the [ledger][ledger] check the spec ID at the Spec commit | | The reviewer of round 1 refuses a fact that only cites a source | Prose on purpose: only a reader can judge that the cited part supports the fact | -| Before the first review, a test file holds a block that a reader needs, with a value that is not the default and private text | [Kit `reads`][reads] needs the private text, and [a case][reads-matrix] for each Output case of the matrix; partial: round 1 runs `just check` before its review. The reviewer judges the value | -| An independent reader proves that the private text is gone and the content shows correctly; a handler that keeps the raw block fails | [Kit `reads`][reads] searches the private text, compares the reference, and needs a bad variant that holds the private text | | Each experiment uses a new scratch folder; never delete files recursively inside `sh -c` or a loop | Prose only | -| The tests and `just acceptance` need no root, no mount, and no system service | Prose only | -| Each case of the Output group that applies has a test file that the tests make, checked by an independent reader | [Kit Output cases][reads-matrix] for each case that `## Case matrix` names; partial: the reviewer checks that the matrix names each case that applies | -| The tests make their files; never search the home folder or copy a file of the user | [Kit `reads`][reads] reads only files of the folder of the scenario; partial: no isolation of the system hides the files of the user | -| Round 1 tests each generic case that applies, and the first review tests families of cases | Prose on purpose: no manifest lists every family of a domain, so only the reviewer finds a missing one | -| Before the review of round 1, `## Case matrix` gives each family its tests, and each line of Failures and Acceptance a family | [Finish matrix][finish-matrix] checks the references only, not that the matrix is complete or correct | | Each refusal and each branch that classifies an input has a pair of tests | [Finish matrix][finish-matrix] needs a spec ID in Accepted of each family; the reviewer checks that it passes | | The first reviewer checks that the matrix is complete and runs it, before any repair | Prose on purpose: a receipt cannot show that the matrix is complete, or that the reviewer judged it | | A repair adds a pair of tests for its whole family, and only then is its finding `fixed` | [Ledger findings][ledger-findings] need a passing and a refused line in its family; partial: the reviewer judges that the family is whole | @@ -48,20 +41,18 @@ This part of the [rule index](../rule-index.md) holds the rules of the rounds of | Accepting a limit starts no round; after a review, only **Limits.** of `spec.md` changes | [Finish gate][finish-verdict] | | A change of `spec.md` that narrows a promise to close a `warn` or `block` is a limit that only the user accepts | Prose on purpose: a diff cannot decide that new words narrow a promise, or that the user agreed | -[finish]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/finish.go -[reads]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/acceptance.go -[reads-matrix]: https://github.com/pascalandy/nicely/blob/main/sdk/extensiontest/acceptance_matrix.go -[ledger]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/ledger.go -[ledger-run]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/ledger_run.go -[ledger-findings]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/ledger_findings.go -[ledger-history]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/ledger_history.go -[finish-matrix]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/spec_matrix.go -[finish-verdict]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/readiness_verdict.go -[readiness]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/readiness.go -[intake]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/intake.go -[records]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/records.go -[records-view]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/records_view.go -[signoff]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/signoff.go -[signoff-replay]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/signoff_replay.go -[release]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/release.go -[release-receipt]: https://github.com/pascalandy/nicely/blob/main/sdk/extensionrepo/release_receipt.go +[finish]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/finish.go +[ledger]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/ledger.go +[ledger-run]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/ledger_run.go +[ledger-findings]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/ledger_findings.go +[ledger-history]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/ledger_history.go +[finish-matrix]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/spec_matrix.go +[finish-verdict]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/readiness_verdict.go +[readiness]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/readiness.go +[intake]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/intake.go +[records]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/records.go +[records-view]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/records_view.go +[signoff]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/signoff.go +[signoff-replay]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/signoff_replay.go +[release]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/release.go +[release-receipt]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/release_receipt.go diff --git a/template/qa.md.jinja b/template/qa.md.jinja index 95360f59..7222682a 100644 --- a/template/qa.md.jinja +++ b/template/qa.md.jinja @@ -1,6 +1,6 @@ # {{ slug }} QA -This file answers one question: what does the UAT of `ncly {{ root }}` add to the [UAT protocol](https://github.com/pascalandy/nicely/blob/main/docs/qa/protocol.md) of Nicely? [spec.md](spec.md) holds the behavior. +This file answers one question: what does the UAT of `ncly {{ root }}` add to the [UAT protocol](https://github.com/pascalandy/nicely/blob/{{ _commit }}/docs/qa/protocol.md) of Nicely? [spec.md](spec.md) holds the behavior. This file is a form filled for the example command, `ncly {{ example_command }}`. Fill in each section for your commands, then delete each line that starts with "Fill:". diff --git a/template/spec.md.jinja b/template/spec.md.jinja index 5d19a860..d276cc63 100644 --- a/template/spec.md.jinja +++ b/template/spec.md.jinja @@ -61,6 +61,11 @@ Before the review of round 1, this table lists each family of cases of each comm A family comes from a generic case that applies, or from a family that the intake found. An example of such a family is each way that the input declares how to read it. Accepted names the lines whose tests give an input that passes. Refused names the lines whose tests refuse an input, or `None`. +An accepted privacy limit names its ID in Accepted for every applicable Output case. +The Limit cell declares the same `Output: ` bindings, separated from its description by semicolons. +Bindings can come before or after the description. Its own ID or bindings alone do not describe retained data. +A semicolon-separated part that equals `None` or starts with `Fill:` prevents acceptance. These comparisons ignore case. +Its acceptance case uses `reads -case '' -limit ` and independently checks the retained metadata and nondefault display declaration. Each refusal and each branch that classifies an input has a pair: a test that passes, and a test that refuses. Each line of Failures and of Acceptance is in a family. `just finish` fails when a family has no test. A repair of a finding adds a pair of tests for its whole family, not only a test of the reported example. diff --git a/template/testdata/script/acceptance.txtar.jinja b/template/testdata/script/acceptance.txtar.jinja index 6065b88e..50cefb2b 100644 --- a/template/testdata/script/acceptance.txtar.jinja +++ b/template/testdata/script/acceptance.txtar.jinja @@ -13,6 +13,11 @@ # Give a case to each Output case that ## Case matrix names, with -case and # its name. For a block that a reader needs, give its private text with # -private, and make the bad variant keep the raw block. +# An accepted privacy limit uses -case '' -limit . Name its ID in +# Accepted for every matrix row of that case, and declare the same Output: +# binding in its Limit cell. Keep -private for text that +# must stay absent, and keep the independent reader and failing bad variant. +# Check the retained metadata and a nondefault display declaration there. chmod 755 bin/python3 # spec: size lines diff --git a/testdata/extensions/hello/hello.go b/testdata/extensions/hello/hello.go index a0eee203..2768c976 100644 --- a/testdata/extensions/hello/hello.go +++ b/testdata/extensions/hello/hello.go @@ -35,13 +35,14 @@ var Extension = extension.Extension{ Skill: skill, Resources: resources, Handlers: map[string]extension.Handler{ - "hello count": count, - "hello run": run, - "hello check": check, - "hello say": say, - "hello wait": wait, - "hello pick": pick, - "hello scan": scan, + "hello count": count, + "hello run": run, + "hello check": check, + "hello say": say, + "hello wait": wait, + "hello pick": pick, + "hello scan": scan, + "hello staged": staged, }, } diff --git a/testdata/extensions/hello/locales/en.toml b/testdata/extensions/hello/locales/en.toml index 04ff4625..78bb64e6 100644 --- a/testdata/extensions/hello/locales/en.toml +++ b/testdata/extensions/hello/locales/en.toml @@ -155,3 +155,10 @@ other = "{{.File}} is no file that hello can scan." ["hello.scan.done"] description = "Text of hello scan once its worker read the whole file. {{.File}} is the path that the caller gave." other = "Scanned {{.File}}" +["hello.staged.summary"] +description = "Summary of the handler cleanup fixture." +other = "Run a worker with private staging" + +["hello.staged.worker"] +description = "Path of the worker used by the handler cleanup fixture." +other = "Worker script" diff --git a/testdata/extensions/hello/manifest.toml b/testdata/extensions/hello/manifest.toml index 4f1077d6..acd30b7f 100644 --- a/testdata/extensions/hello/manifest.toml +++ b/testdata/extensions/hello/manifest.toml @@ -2,6 +2,21 @@ format_version = 1 contract_version = 1 domain = "hello" +[[commands]] +path = "hello staged" +summary = "hello.staged.summary" +effects = { user_writes = true } + +[[commands.args]] +name = "worker" +summary = "hello.staged.worker" +required = true + +[commands.result.schema] +type = "object" +required = ["stage"] +properties = { stage = { type = "string" } } + [[commands]] path = "hello" summary = "hello.summary" diff --git a/testdata/extensions/hello/staged.go b/testdata/extensions/hello/staged.go new file mode 100644 index 00000000..e005b3e4 --- /dev/null +++ b/testdata/extensions/hello/staged.go @@ -0,0 +1,60 @@ +package hello + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strconv" + "time" + + "github.com/pascalandy/nicely/sdk/contract" + "github.com/pascalandy/nicely/sdk/extension" + "github.com/pascalandy/nicely/sdk/runner" +) + +// staged models a handler whose worker writes into private staging. Cleanup +// takes a little time after Run returns, as removing a real staging tree does. +func staged(ctx context.Context, call extension.Call) contract.Outcome { + stage, err := os.MkdirTemp(".", ".ncly-stage-") + if err != nil { + return stageFailure(call) + } + defer func() { + started := time.Now() + time.Sleep(100 * time.Millisecond) + removing := time.Now() + _ = os.RemoveAll(stage) + stats, _ := json.Marshal(map[string]int64{ + "delete_ms": time.Since(removing).Milliseconds(), "cleanup_ms": time.Since(started).Milliseconds(), + }) + _ = os.WriteFile("cleanup.json", stats, 0o600) + _ = os.WriteFile("cleanup.done", []byte("done\n"), 0o600) + }() + if value := os.Getenv("CLEANUP_FILES"); value != "" { + count, err := strconv.Atoi(value) + if err != nil || count < 0 { + return stageFailure(call) + } + for i := range count { + if err := os.WriteFile(filepath.Join(stage, fmt.Sprintf("f%d", i)), nil, 0o600); err != nil { + return stageFailure(call) + } + } + } + _, err = runner.Run(ctx, runner.Program{ + Path: "/bin/sh", Args: []string{call.Args[0], stage}, Env: os.Environ(), + }) + if err != nil { + return stageFailure(call) + } + return contract.Outcome{Data: map[string]string{"stage": stage}} +} + +func stageFailure(call extension.Call) contract.Outcome { + return contract.Outcome{Errors: []contract.Problem{{ + Code: contract.Runtime, + Message: call.Printer.T("hello.program"), Hint: "ncly hello staged worker.sh", + }}} +} diff --git a/testdata/extensions/hello/testdata/script/sdk_extension.txtar b/testdata/extensions/hello/testdata/script/sdk_extension.txtar index 43a2bb8f..75a203f6 100644 --- a/testdata/extensions/hello/testdata/script/sdk_extension.txtar +++ b/testdata/extensions/hello/testdata/script/sdk_extension.txtar @@ -70,7 +70,7 @@ pseudo stderr hello run Ada exec ncly --lang en-XA hello run pseudo stderr hello run Ada exec ncly --lang en-XA hello --help -pseudo stdout hello run check say wait pick count scan +pseudo stdout hello run check say wait pick count scan staged # Run directly from the data folder, the fixture gives the same answer as through ncly exec $DATA/ncly-hello run --json -- Ada diff --git a/testdata/extensions/hello/testdata/script/staged_cleanup.txtar b/testdata/extensions/hello/testdata/script/staged_cleanup.txtar new file mode 100644 index 00000000..3fd35a34 --- /dev/null +++ b/testdata/extensions/hello/testdata/script/staged_cleanup.txtar @@ -0,0 +1,93 @@ +# The host and SDK worker runners used equal cleanup deadlines. The host could +# kill the handler before its deferred cleanup removed private partial output. +# Every call uses the ordinary signal adapter; no fixture-specific exception. +install hello +cp input.txt input.before +cp output.txt output.before + +env CLEANUP_WORKER=stubborn +env CLEANUP_FILES=20000 +protect input.txt output.txt +signal INT worker.pid ncly hello staged worker.sh +answer stderr INTERRUPTED +cp stderr answer.json +exec sh check.sh +exists cleanup.done +grep '"stage":"' answer.json +gone worker.pid +gone grandchild.pid +cmp input.txt input.before +cmp output.txt output.before + +rm worker.pid grandchild.pid cleanup.done +protect input.txt output.txt +signal TERM worker.pid ncly hello staged worker.sh +answer stderr TERMINATED +cp stderr answer.json +exec sh check.sh +exists cleanup.done +grep '"stage":"' answer.json +gone worker.pid +gone grandchild.pid +cmp input.txt input.before +cmp output.txt output.before + +# The same extension also cleans up directly, with its ordinary worker budget. +rm worker.pid grandchild.pid cleanup.done +protect input.txt output.txt +signal TERM worker.pid ncly-hello staged worker.sh +answer stderr TERMINATED +exists cleanup.done +exec sh check.sh +gone worker.pid +gone grandchild.pid +cmp input.txt input.before +cmp output.txt output.before + +# A cooperative worker follows the same path without needing the force stop. +rm worker.pid grandchild.pid cleanup.done +env CLEANUP_WORKER=cooperative +protect input.txt output.txt +signal INT worker.pid ncly hello staged worker.sh +answer stderr INTERRUPTED +exists cleanup.done +exec sh check.sh +gone worker.pid +gone grandchild.pid +cmp input.txt input.before +cmp output.txt output.before + +-- input.txt -- +original input +-- output.txt -- +previous output +-- check.sh -- +for stage in .ncly-stage-*; do + if [ -e "$stage" ]; then + echo "private staging remains: $stage" >&2 + exit 91 + fi +done +-- worker.sh -- +printf 'partial output\n' > "$1/partial" +python3 -c 'import os, signal; os.setsid(); signal.signal(signal.SIGINT, signal.SIG_DFL); signal.signal(signal.SIGTERM, signal.SIG_DFL); os.execl("/bin/sh", "sh", "grandchild.sh")' >/dev/null 2>&1 & +while [ ! -f grandchild.pid ]; do sleep 0.01; done +if [ "$CLEANUP_WORKER" = stubborn ]; then + trap '' INT TERM +else + trap 'exit 130' INT + trap 'exit 143' TERM +fi +echo $$ > worker.tmp +mv worker.tmp worker.pid +exec sleep 30 +-- grandchild.sh -- +if [ "$CLEANUP_WORKER" = stubborn ]; then + trap '' INT TERM +else + trap 'exit 130' INT + trap 'exit 143' TERM +fi +echo $$ > grandchild.tmp +mv grandchild.tmp grandchild.pid +exec sleep 30 diff --git a/testdata/extensions/hello/testdata/script/wrap_stop.txtar b/testdata/extensions/hello/testdata/script/wrap_stop.txtar index a660ad5b..fb321a37 100644 --- a/testdata/extensions/hello/testdata/script/wrap_stop.txtar +++ b/testdata/extensions/hello/testdata/script/wrap_stop.txtar @@ -55,8 +55,8 @@ answer stderr INTERRUPTED stderr '"message":"Stopped by Ctrl-C\. Finished work is kept\."' gone hang.pid -# A program that ignores Ctrl-C runs until the SIGKILL of core's cleanup, 4 seconds in the -# test binary, and the folder of the worker is gone before that SIGKILL +# Core's reservation force-stops this resistant worker after 2 seconds in the +# test binary. The worker's folder is gone before that force-stop. rm hang.pid child.pid worker.path env STUB=stubborn exec exitcode exit.txt ncly hello count input.md --json & diff --git a/testdata/extensions/standalone-named/go.mod b/testdata/extensions/standalone-named/go.mod index 942d75e5..d48d03da 100644 --- a/testdata/extensions/standalone-named/go.mod +++ b/testdata/extensions/standalone-named/go.mod @@ -51,6 +51,7 @@ require ( github.com/yuin/goldmark-emoji v1.0.5 // indirect github.com/zalando/go-keyring v0.2.8 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect + golang.org/x/mod v0.41.0 // indirect golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.23.0 // indirect golang.org/x/sys v0.48.0 // indirect diff --git a/testdata/extensions/standalone/go.mod b/testdata/extensions/standalone/go.mod index 44198562..0c2c0443 100644 --- a/testdata/extensions/standalone/go.mod +++ b/testdata/extensions/standalone/go.mod @@ -51,6 +51,7 @@ require ( github.com/yuin/goldmark-emoji v1.0.5 // indirect github.com/zalando/go-keyring v0.2.8 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect + golang.org/x/mod v0.41.0 // indirect golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.23.0 // indirect golang.org/x/sys v0.48.0 // indirect diff --git a/testdata/script/extension_cancel_stubborn.txtar b/testdata/script/extension_cancel_stubborn.txtar index ea962b49..5e5865a9 100644 --- a/testdata/script/extension_cancel_stubborn.txtar +++ b/testdata/script/extension_cancel_stubborn.txtar @@ -14,6 +14,23 @@ answer stderr INTERRUPTED stderr '"output_dir":"/Users/me/out"' gone fixture.pid +# The fixture captures its cleanup child's wait diagnostic separately. +# It keeps a valid JSON answer and returns before the final force-stop. +rm fixture.pid +exec exitcode exit.txt ncly hello cleanup-child --json & +waitfile fixture.pid +kill -INT +wait +grep '^130$' exit.txt +! stdout . +answer stderr INTERRUPTED +stderr '"output_dir":"/Users/me/out"' +! stderr Killed +exec cat cleanup-child.exit +stdout '^137\n$' +gone cleanup-child.pid +gone fixture.pid + # An extension that keeps running after Ctrl-C got the signal, and SIGKILL ends it once # cleanup ends. A second Ctrl-C during cleanup changes nothing: it never reaches the # extension, and core still answers INTERRUPTED @@ -45,9 +62,25 @@ gone fixture.pid -- bin/ncly-hello -- #!/bin/sh +cleanup_child() { + # A shell can print its wait diagnostic itself, so redirect the whole group. + { + sh cleanup-child.sh & + child=$! + wait "$child" + result=$? + } 2> "$WORK/cleanup-child.stderr" + printf '%s\n' "$result" > "$WORK/cleanup-child.exit" + echo '{"ok":true,"contract_version":1,"output_dir":"/Users/me/out"}' + exit 0 +} case "$1" in late) - trap 'sleep 3; echo "{\"ok\":true,\"contract_version\":1,\"output_dir\":\"/Users/me/out\"}"; exit 0' INT TERM + # Delay in the handler itself: core may force-stop descendant processes first. + exec python3 late.py + ;; +cleanup-child) + trap cleanup_child INT TERM ;; stubborn) trap 'echo INT >> "$WORK/signals.txt"' INT @@ -61,6 +94,34 @@ esac echo $$ > "$WORK/fixture.tmp" mv "$WORK/fixture.tmp" "$WORK/fixture.pid" while :; do sleep 0.1; done +-- cleanup-child.sh -- +echo $$ > "$WORK/cleanup-child.tmp" +mv "$WORK/cleanup-child.tmp" "$WORK/cleanup-child.pid" +exec sleep 30 +-- late.py -- +import json +import os +import signal +import time +from pathlib import Path + + +def stop(_signal: int, _frame: object) -> None: + time.sleep(3) + print( + json.dumps({"ok": True, "contract_version": 1, "output_dir": "/Users/me/out"}) + ) + raise SystemExit(0) + + +signal.signal(signal.SIGINT, stop) +signal.signal(signal.SIGTERM, stop) +folder = Path(os.environ["WORK"]) +temporary = folder / "fixture.tmp" +temporary.write_text(f"{os.getpid()}\n", encoding="utf-8") +temporary.rename(folder / "fixture.pid") +while True: + signal.pause() -- bin/ncly-hello.toml -- format_version = 1 contract_version = 1 @@ -74,6 +135,10 @@ summary = "hello.summary" path = "hello late" summary = "hello.late.summary" +[[commands]] +path = "hello cleanup-child" +summary = "hello.cleanup_child.summary" + [[commands]] path = "hello stubborn" summary = "hello.stubborn.summary" @@ -90,6 +155,10 @@ other = "Greet people" description = "Summary of hello late." other = "Answer 3 seconds after a signal" +["hello.cleanup_child.summary"] +description = "Summary of hello cleanup-child." +other = "Answer after the cleanup child stops" + ["hello.stubborn.summary"] description = "Summary of hello stubborn." other = "Keep running after a signal" diff --git a/testdata/script/transcript_named_busy.txtar b/testdata/script/transcript_named_busy.txtar new file mode 100644 index 00000000..3f6fe89c --- /dev/null +++ b/testdata/script/transcript_named_busy.txtar @@ -0,0 +1,18 @@ +# A named run takes its lock before the invalid output and missing key checks. +env LANG= LC_ALL= LC_MESSAGES= NCLY_LANG= +env DEEPGRAM_API_KEY= +env NCLY_TEST_UV_CACHE=$WORK/cache +env NCLY_TEST_LAUNCHER=$WORK/unused.py +onlyncly transcript-external holdlock +install transcript +env RUNS=$WORK/.local/state/nicely/runs +mkdir $RUNS +! exec holdlock $RUNS/busy.lock held & +waitfile held +exits 75 ncly transcript run youtube --url https://youtu.be/abc --output $WORK/missing --run-id busy --json +answer stderr TEMPORARY +stderr '"run_id":"busy"' +kill +wait +exists $RUNS/busy.lock +! exists $RUNS/busy.json diff --git a/testdata/script/transcript_named_replay.txtar b/testdata/script/transcript_named_replay.txtar new file mode 100644 index 00000000..c509dc42 --- /dev/null +++ b/testdata/script/transcript_named_replay.txtar @@ -0,0 +1,32 @@ +# A named completed run still replays without an output check, programs, or key. +env LANG= LC_ALL= LC_MESSAGES= NCLY_LANG= +env DEEPGRAM_API_KEY= +env NCLY_TEST_UV_CACHE=$WORK/cache +env NCLY_TEST_LAUNCHER=$WORK/unused.py +onlyncly transcript-external +install transcript +env RUNS=$WORK/.local/state/nicely/runs +mkdir $RUNS $WORK/out/saved $WORK/.config/nicely +cp words.txt $WORK/out/saved/raw_transcript.txt +cp words.txt $WORK/out/saved/raw_sentences.txt +cp paragraphs.json $WORK/out/saved/raw_transcript.json +cp config.toml $WORK/.config/nicely/config.toml +exec /bin/sh -c '/usr/bin/sed "s|ROOT|$WORK|g" saved.json > "$RUNS/saved.json"' +cp $RUNS/saved.json before.json +exec ncly transcript run youtube --url https://youtu.be/abc --run-id saved --json +answer stdout ok +stdout '"run_id":"saved"' +stdout '"output_dir":"'${WORK@R}'/out/saved"' +stdout '"transcript":"'${WORK@R}'/out/saved/raw_transcript\.txt"' +cmp $RUNS/saved.json before.json +exists $RUNS/saved.lock + +-- config.toml -- +[transcript.youtube] +output = "missing-output" +-- words.txt -- +hello +-- paragraphs.json -- +[] +-- saved.json -- +{"format_version":1,"run_id":"saved","path":"transcript run youtube","status":"completed","started_at":"2026-10-03T09:00:00Z","updated_at":"2026-10-03T09:02:00Z","ncly_version":"v0.26.0","attempts":1,"inputs":{"source":"youtube","output":null},"resolved":{"output":"ROOT/out","browser":null},"tools":{"transcript":"1.0.0","yt-dlp":"2026.1.1"},"items":[{"key":"https://youtu.be/abc","output_dir":"ROOT/out/saved","steps":[{"id":"transcribe","effect":"paid","status":"completed","started_at":"2026-10-03T09:00:01Z","finished_at":"2026-10-03T09:02:00Z","artifacts":[{"path":"ROOT/out/saved/raw_transcript.txt","size_bytes":6,"sha256":"5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"},{"path":"ROOT/out/saved/raw_sentences.txt","size_bytes":6,"sha256":"5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"},{"path":"ROOT/out/saved/raw_transcript.json","size_bytes":3,"sha256":"37517e5f3dc66819f61f5a7bb8ace1921282415f10551d2defa5c3eb0985b570"}],"error":null,"evidence":{"format_version":1,"warnings":[]}}]}]} diff --git a/testdata/script/transcript_no_id_key.txtar b/testdata/script/transcript_no_id_key.txtar new file mode 100644 index 00000000..831e4f27 --- /dev/null +++ b/testdata/script/transcript_no_id_key.txtar @@ -0,0 +1,30 @@ +# A missing key without --run-id leaves the existing run files unchanged. +env LANG= LC_ALL= LC_MESSAGES= NCLY_LANG= +env DEEPGRAM_API_KEY= +env NCLY_TEST_UV_CACHE=$WORK/cache +env NCLY_TEST_LAUNCHER=$WORK/unused.py +mkdir bin +chmod 755 bin/uv +chmod 755 bin/python3 +onlyncly transcript-external +install transcript +env RUNS=$WORK/.local/state/nicely/runs +mkdir $RUNS +cp retained.lock $RUNS/retained.lock +exec /bin/ls -A $RUNS +cp stdout before.txt +exits 78 ncly transcript run youtube --url https://youtu.be/abc --json +answer stderr KEY_NOT_GRANTED +! stderr '"run_id"' +exec /bin/ls -A $RUNS +cmp stdout before.txt +cmp retained.lock $RUNS/retained.lock + +-- retained.lock -- +keep this existing lock file +-- bin/uv -- +#!/bin/sh +echo 'uv 0.13.1' +-- bin/python3 -- +#!/bin/sh +echo 'Python 3.13.1' diff --git a/testdata/script/transcript_no_id_output.txtar b/testdata/script/transcript_no_id_output.txtar new file mode 100644 index 00000000..c919306c --- /dev/null +++ b/testdata/script/transcript_no_id_output.txtar @@ -0,0 +1,21 @@ +# An unusable output without --run-id leaves the existing run files unchanged. +env LANG= LC_ALL= LC_MESSAGES= NCLY_LANG= +env DEEPGRAM_API_KEY= +env NCLY_TEST_UV_CACHE=$WORK/cache +env NCLY_TEST_LAUNCHER=$WORK/unused.py +onlyncly transcript-external +install transcript +env RUNS=$WORK/.local/state/nicely/runs +mkdir $RUNS +cp retained.lock $RUNS/retained.lock +exec /bin/ls -A $RUNS +cp stdout before.txt +exits 2 ncly transcript run youtube --url https://youtu.be/abc --output $WORK/missing --json +answer stderr USAGE_INVALID +! stderr '"run_id"' +exec /bin/ls -A $RUNS +cmp stdout before.txt +cmp retained.lock $RUNS/retained.lock + +-- retained.lock -- +keep this existing lock file diff --git a/tools/cmd/plan/revision_index_test.go b/tools/cmd/plan/revision_index_test.go new file mode 100644 index 00000000..cea94b06 --- /dev/null +++ b/tools/cmd/plan/revision_index_test.go @@ -0,0 +1,26 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestRuleIndexResolvesSelectedTemplateRevision(t *testing.T) { + root := t.TempDir() + write(t, filepath.Join(root, "sdk/extensionrepo/gate.go"), "package extensionrepo\n") + file := filepath.Join(root, filepath.FromSlash(templateIndex)) + text := "# Rules\n\n| Rule | Enforced by |\n|---|---|\n| Check the gate | [Gate][gate] |\n\n[gate]: https://github.com/pascalandy/nicely/blob/{{ _commit }}/sdk/extensionrepo/gate.go\n" + write(t, file, text) + if problems, err := checkRuleIndexes(root); err != nil || len(problems) != 0 { + t.Fatalf("selected revision link failed: %v %q", err, problems) + } + write(t, file, strings.ReplaceAll(text, "/gate.go", "/missing.go")) + problems, err := checkRuleIndexes(root) + if err != nil { + t.Fatal(err) + } + if len(problems) == 0 || !strings.Contains(strings.Join(problems, "\n"), "reaches no file") { + t.Fatalf("missing target passed: %q", problems) + } +} diff --git a/tools/cmd/plan/rules.go b/tools/cmd/plan/rules.go index f817f35a..acb55ad3 100644 --- a/tools/cmd/plan/rules.go +++ b/tools/cmd/plan/rules.go @@ -128,6 +128,10 @@ func markdownFiles(root, dir, suffix string) ([]string, error) { // their enforcers link to named, and returns its problems. ids maps each ID // of a core rule to the file that holds it. func readRules(root, file, text string, index *ruleIndex, named map[string]bool, ids map[string]string) []string { + // Resolve template links against this checkout before Markdown parses their spaces. + if strings.HasPrefix(file, "template/") { + text = strings.ReplaceAll(text, "{{ _commit }}", "main") + } var problems []string refs := definitions(text) var header []string diff --git a/tools/cmd/plan/template_links_test.go b/tools/cmd/plan/template_links_test.go new file mode 100644 index 00000000..bc180c85 --- /dev/null +++ b/tools/cmd/plan/template_links_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + "testing" +) + +// Source links cannot silently opt a pinned extension into main's contract. +func TestPinnedDocumentationSources(t *testing.T) { + for _, rel := range []string{"template", "extensions/skill/nicely-extensions"} { + err := filepath.WalkDir(filepath.Join("../../..", rel), func(file string, entry fs.DirEntry, err error) error { + if err != nil || entry.IsDir() { + return err + } + data, err := os.ReadFile(file) + if err != nil { + return err + } + for n, line := range strings.Split(string(data), "\n") { + if strings.Contains(line, "https://github.com/pascalandy/nicely/blob/main/") { + t.Errorf("%s:%d: link Nicely documentation to the selected revision instead of main", file, n+1) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } +}