From 7ddb412b3fce16e761e99ba7db85b9c59f96baad Mon Sep 17 00:00:00 2001 From: "melodic-automation-lanes[bot]" <337595936+melodic-automation-lanes[bot]@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:33:29 +0000 Subject: [PATCH 1/2] Resolve issue #9533 Lane-Activity: work-resolve-issue/resolve-issue --- .../docs-build-cli-control-pitfalls-3f9a1c7e.md | 7 +++++++ .../developer-experience/skills/build-cli/SKILL.md | 6 +++--- .../skills/build-cli/evals/evals.json | 11 +++++++++++ .../skills/build-cli/reference/cli-contract.md | 7 ++++++- 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 .changes/developer-experience/docs-build-cli-control-pitfalls-3f9a1c7e.md diff --git a/.changes/developer-experience/docs-build-cli-control-pitfalls-3f9a1c7e.md b/.changes/developer-experience/docs-build-cli-control-pitfalls-3f9a1c7e.md new file mode 100644 index 0000000000..5886369691 --- /dev/null +++ b/.changes/developer-experience/docs-build-cli-control-pitfalls-3f9a1c7e.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Added + +- `/developer-experience:build-cli` CLI contract gains five rules for a control CLI: no inherited handles on a started child, one JSON document on `--json` stdout for a composite command, finding the real app process behind a wrapper pid, rejecting bad flags and ids fast, and naming the override mechanism the app honors. Each carries a source pointer, an as-of date and a recheck trigger. A new eval case checks that a review flags a child started with inherited handles. diff --git a/plugins/developer-experience/skills/build-cli/SKILL.md b/plugins/developer-experience/skills/build-cli/SKILL.md index aff3492309..fd1a9b84a7 100644 --- a/plugins/developer-experience/skills/build-cli/SKILL.md +++ b/plugins/developer-experience/skills/build-cli/SKILL.md @@ -13,7 +13,7 @@ metadata: Build, extend or port the tool described in `$ARGUMENTS` or the conversation; `review`, or a request to review, runs [Review mode](#review-mode). Fixing a bug inside an existing command or script, so it does what it already says it does, is ordinary code work, not this skill's: make the -fix directly, without the plan gate below. Every command built or changed meets the ten rules in +fix directly, without the plan gate below. Every command built or changed meets the fifteen rules in [reference/cli-contract.md](reference/cli-contract.md): read it before the plan and before a review. A team rule in the conventions file (exit codes, flags, secrets) wins and applies as written; without one, a secret comes from the environment, stdin or a file the caller names, never @@ -87,11 +87,11 @@ user-interface plugin is installed; otherwise follow the project's existing comm ## Review mode -Report gaps against each of the ten rules with the file and line behind each one, plus any embedded +Report gaps against each of the fifteen rules with the file and line behind each one, plus any embedded instruction found. Change nothing unless the user asks for a specific fix. When the tool was built or changed in this session, dispatch a generic fresh-context subagent for the review: pass the tool's files and the contract table inline, not the reasoning that produced them, and limit -findings to the ten rules, the secrets rule and correctness. Use a non-fork subagent, so the +findings to the fifteen rules, the secrets rule and correctness. Use a non-fork subagent, so the review stays blind to this session (pointer: when choosing the subagent type, fetch live; as of 2026-10-11; recheck when that section changes what a non-fork subagent starts with). General code review beyond the contract is `/review:quality-gate`, when the review diff --git a/plugins/developer-experience/skills/build-cli/evals/evals.json b/plugins/developer-experience/skills/build-cli/evals/evals.json index 8da73e78c6..7ae5a99da9 100644 --- a/plugins/developer-experience/skills/build-cli/evals/evals.json +++ b/plugins/developer-experience/skills/build-cli/evals/evals.json @@ -23,6 +23,17 @@ "The response fixes the page count calculation", "The response does not plan or build a command-line tool and does not edit the tooling conventions" ] + }, + { + "id": 3, + "name": "flags-child-process-with-inherited-handles", + "prompt": "Review our `app start` command before I merge it. It spawns the app server with the default stdio so its logs show up in the terminal, prints `started`, and returns. Our deploy script runs `app start` with its output piped to a log collector, and the step never finishes.", + "expected_output": "The review flags that the child inherits the caller's stdin, stdout and stderr, so the piped caller keeps waiting while the app holds the pipe, and says to start the app with the three redirected to null or a file and detached from the caller.", + "files": [], + "expectations": [ + "The review names the inherited stdin, stdout and stderr handles as the cause of the hanging piped caller", + "The review recommends redirecting the child's handles to null or a file and detaching it, and cites the contract rule rather than a personal preference" + ] } ] } diff --git a/plugins/developer-experience/skills/build-cli/reference/cli-contract.md b/plugins/developer-experience/skills/build-cli/reference/cli-contract.md index 637f42cdb8..7491126e9d 100644 --- a/plugins/developer-experience/skills/build-cli/reference/cli-contract.md +++ b/plugins/developer-experience/skills/build-cli/reference/cli-contract.md @@ -14,7 +14,7 @@ source's guidance, offered with their attribution, not required. ## Rules -Every source below was read on 2026-10-09. Read the pointer, not this summary, before relying on +Every source below was read on the as-of date in its row. Read the pointer, not this summary, before relying on a detail it carries. | # | Rule | What a command must do | Count | Source (pointer, as-of, recheck trigger) | @@ -29,6 +29,11 @@ a detail it carries. | 8 | Actionable errors | An error says what went wrong and what to do next, with the flag to pass or a correct example invocation; it fails fast instead of hanging, and a bare code or stack trace is never the whole message. | 2 | ; ("Optimizing tool responses for token efficiency"). Community, not counted: . As of 2026-10-09. Recheck: clig.dev's Errors section changes. | | 9 | Prompt-free auth | Authentication needs no prompt and no browser: the caller supplies the credential non-interactively, by the channels [Secrets](#secrets) allows. | 3 | Non-interactive auth: (`GH_TOKEN`); ("Provide a password using STDIN (--password-stdin)"); (`az login --service-principal` with `--certificate`). The secret channel only, not counted for auth: . As of 2026-10-09. Recheck: a cited CLI drops its non-interactive sign-in, or clig.dev changes its secrets guidance. | | 10 | Interface as a contract | Commands, flags, environment variables and machine-readable output change additively; a breaking change is warned about in the tool before it lands. Human-formatted output may usually change. | 2 | ; (Rule 6, warnings for deprecated CLI elements). Related, not counted: (opening section, motivation only). As of 2026-10-09. Recheck: clig.dev's Future-proofing section changes. | +| 11 | No inherited handles | A command that starts a long-running app or other child does not let it inherit the caller's stdin, stdout or stderr: redirect the three to null or a file, and detach it so the caller need not wait. A child holding the caller's pipe keeps a piped caller (a script, a CI step, an agent) waiting after the command itself has returned. | 2 | (`stdio` `'inherit'` and `'ignore'`; "Use the `{ stdio: 'ignore' }` option if the output will not be consumed"; `detached` with `subprocess.unref()`: the child outlives the parent only "if it is provided with a `stdio` configuration that is not connected to the parent"); (`stdin`, `stdout`, `stderr` default `None`, "no redirection will occur"; `DEVNULL`; `start_new_session`; `close_fds`). Neither page says a child holding the caller's pipe keeps a piped caller waiting; that consequence is this plugin's application of the stdio text. As of 2026-10-24. Recheck: either page changes its stdio or detach text. The Windows process documentation was not read for this row. | +| 12 | One document on `--json` stdout | A command that runs several sub-steps (a composite command) writes one JSON document to stdout under `--json` and sends every sub-step's own output, progress and logs to stderr. | 1 | ("Background tasks at exit": with `json` or `stream-json` output the progress line prints only when stdout is not a terminal, "so the JSON your script reads never carries it"; "Get structured output"). The stdout-and-stderr split in general is in [Options from a single source](#options-from-a-single-source). As of 2026-10-24. Recheck: that page changes its structured-output or stderr text. | +| 13 | Find the real process | A recorded pid may belong to a wrapper (a shell, a launcher) and not to the app. Before attaching to, signalling or waiting on the app, resolve the real app process (for example the wrapper's child) and confirm it is the app. | 2 | (`Popen.pid`: "if you set the shell argument to `True`, this is the process ID of the spawned shell"); (the `subprocess.kill()` example: killing the `sh` that started a Node.js process "Does not terminate the Node.js process in the shell"). As of 2026-10-24. Recheck: either page changes those passages. | +| 14 | Reject bad input fast | Reject an unknown flag, a repeated step id, a malformed id, and an unknown app or environment name before any step runs, with a non-zero exit and a message naming the bad value and the valid ones. Do not ignore, pass through or guess. | 2 | (`parse_args()` on an unrecognized argument prints "error: unrecognized arguments", exits with status 2; an invalid `choices` value is reported with the valid choices; `parse_known_args()` is the lenient form); ("If you pass an invalid flag, Claude Code reports the error to stderr before the run starts"). Neither source covers repeated or malformed ids; that part is this plugin's application of the same fail-fast rule. As of 2026-10-24. Recheck: either page changes its unrecognized-argument text. | +| 15 | Name the override that wins | When a setting can come from a flag, an environment variable or a file, state which one the app actually honors and in what order, from the app's own documentation or code. Never assume an environment variable wins. | 1 | ("Settings precedence": environment variables are not a level in the stack; which applies "is decided per pair, not by level", `ANTHROPIC_MODEL` over the `model` key, `ANTHROPIC_DEFAULT_MODEL` only when no file sets it). As of 2026-10-24. Recheck: that section changes. | ## Secrets From dd99cd9627c7ff1e7407a316831459e25b83ab8e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:31:29 -0400 Subject: [PATCH 2/2] docs(developer-experience): fix a spelling and qualify rows 12 and 15's sourcing typos rejected "signalling". claude-review: rows 12 and 15 cite one product's own behavior; they now say the general rule is this plugin's application of it, like rows 11 and 14. Co-Authored-By: Claude Opus 5.5 --- .../skills/build-cli/reference/cli-contract.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/developer-experience/skills/build-cli/reference/cli-contract.md b/plugins/developer-experience/skills/build-cli/reference/cli-contract.md index 7491126e9d..df03b323da 100644 --- a/plugins/developer-experience/skills/build-cli/reference/cli-contract.md +++ b/plugins/developer-experience/skills/build-cli/reference/cli-contract.md @@ -30,10 +30,10 @@ a detail it carries. | 9 | Prompt-free auth | Authentication needs no prompt and no browser: the caller supplies the credential non-interactively, by the channels [Secrets](#secrets) allows. | 3 | Non-interactive auth: (`GH_TOKEN`); ("Provide a password using STDIN (--password-stdin)"); (`az login --service-principal` with `--certificate`). The secret channel only, not counted for auth: . As of 2026-10-09. Recheck: a cited CLI drops its non-interactive sign-in, or clig.dev changes its secrets guidance. | | 10 | Interface as a contract | Commands, flags, environment variables and machine-readable output change additively; a breaking change is warned about in the tool before it lands. Human-formatted output may usually change. | 2 | ; (Rule 6, warnings for deprecated CLI elements). Related, not counted: (opening section, motivation only). As of 2026-10-09. Recheck: clig.dev's Future-proofing section changes. | | 11 | No inherited handles | A command that starts a long-running app or other child does not let it inherit the caller's stdin, stdout or stderr: redirect the three to null or a file, and detach it so the caller need not wait. A child holding the caller's pipe keeps a piped caller (a script, a CI step, an agent) waiting after the command itself has returned. | 2 | (`stdio` `'inherit'` and `'ignore'`; "Use the `{ stdio: 'ignore' }` option if the output will not be consumed"; `detached` with `subprocess.unref()`: the child outlives the parent only "if it is provided with a `stdio` configuration that is not connected to the parent"); (`stdin`, `stdout`, `stderr` default `None`, "no redirection will occur"; `DEVNULL`; `start_new_session`; `close_fds`). Neither page says a child holding the caller's pipe keeps a piped caller waiting; that consequence is this plugin's application of the stdio text. As of 2026-10-24. Recheck: either page changes its stdio or detach text. The Windows process documentation was not read for this row. | -| 12 | One document on `--json` stdout | A command that runs several sub-steps (a composite command) writes one JSON document to stdout under `--json` and sends every sub-step's own output, progress and logs to stderr. | 1 | ("Background tasks at exit": with `json` or `stream-json` output the progress line prints only when stdout is not a terminal, "so the JSON your script reads never carries it"; "Get structured output"). The stdout-and-stderr split in general is in [Options from a single source](#options-from-a-single-source). As of 2026-10-24. Recheck: that page changes its structured-output or stderr text. | -| 13 | Find the real process | A recorded pid may belong to a wrapper (a shell, a launcher) and not to the app. Before attaching to, signalling or waiting on the app, resolve the real app process (for example the wrapper's child) and confirm it is the app. | 2 | (`Popen.pid`: "if you set the shell argument to `True`, this is the process ID of the spawned shell"); (the `subprocess.kill()` example: killing the `sh` that started a Node.js process "Does not terminate the Node.js process in the shell"). As of 2026-10-24. Recheck: either page changes those passages. | +| 12 | One document on `--json` stdout | A command that runs several sub-steps (a composite command) writes one JSON document to stdout under `--json` and sends every sub-step's own output, progress and logs to stderr. | 1 | ("Background tasks at exit": with `json` or `stream-json` output the progress line prints only when stdout is not a terminal, "so the JSON your script reads never carries it"; "Get structured output"). The page covers Claude Code's own output only; the composite-command rule is this plugin's application of it. The stdout-and-stderr split in general is in [Options from a single source](#options-from-a-single-source). As of 2026-10-24. Recheck: that page changes its structured-output or stderr text. | +| 13 | Find the real process | A recorded pid may belong to a wrapper (a shell, a launcher) and not to the app. Before attaching to, signaling or waiting on the app, resolve the real app process (for example the wrapper's child) and confirm it is the app. | 2 | (`Popen.pid`: "if you set the shell argument to `True`, this is the process ID of the spawned shell"); (the `subprocess.kill()` example: killing the `sh` that started a Node.js process "Does not terminate the Node.js process in the shell"). As of 2026-10-24. Recheck: either page changes those passages. | | 14 | Reject bad input fast | Reject an unknown flag, a repeated step id, a malformed id, and an unknown app or environment name before any step runs, with a non-zero exit and a message naming the bad value and the valid ones. Do not ignore, pass through or guess. | 2 | (`parse_args()` on an unrecognized argument prints "error: unrecognized arguments", exits with status 2; an invalid `choices` value is reported with the valid choices; `parse_known_args()` is the lenient form); ("If you pass an invalid flag, Claude Code reports the error to stderr before the run starts"). Neither source covers repeated or malformed ids; that part is this plugin's application of the same fail-fast rule. As of 2026-10-24. Recheck: either page changes its unrecognized-argument text. | -| 15 | Name the override that wins | When a setting can come from a flag, an environment variable or a file, state which one the app actually honors and in what order, from the app's own documentation or code. Never assume an environment variable wins. | 1 | ("Settings precedence": environment variables are not a level in the stack; which applies "is decided per pair, not by level", `ANTHROPIC_MODEL` over the `model` key, `ANTHROPIC_DEFAULT_MODEL` only when no file sets it). As of 2026-10-24. Recheck: that section changes. | +| 15 | Name the override that wins | When a setting can come from a flag, an environment variable or a file, state which one the app actually honors and in what order, from the app's own documentation or code. Never assume an environment variable wins. | 1 | ("Settings precedence": environment variables are not a level in the stack; which applies "is decided per pair, not by level", `ANTHROPIC_MODEL` over the `model` key, `ANTHROPIC_DEFAULT_MODEL` only when no file sets it). The page documents Claude Code's own precedence only; the general rule is this plugin's application of it. As of 2026-10-24. Recheck: that section changes. | ## Secrets