diff --git a/docs/TESTING.md b/docs/TESTING.md index cb4c6d0de..1708e1d1c 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -59,7 +59,7 @@ Every new test follows these rules. Some of them name a path, fixture or check t ### Which change gets a test - A change that adds or changes what the code does gets a test. Write it first, and see it fail on the base. A fix pull request cites its `devops ci fail-to-pass --base ` run. -- Every other change gets no test: a document, a manifest, a workflow, a deletion or a refactor that keeps behavior, an edit of tests only, or data. It names one command that can run again and gives the same answer, such as `uv run devops ci docs`, `uv run devops openspec check`, `! git grep -n src/ tests/` (a command that exits 0 when it finds nothing), or `uv run devops k8s image-pins`. When no command exists, it gives the steps of a repeatable manual check. The `! git grep` form has a fixed shape, because the step runs it as a list of words and never as a shell: the options `-n -I -F -i -w -l -c -q`, one plain literal and relative paths. A pipe, `;`, `&&`, a redirect and any other option are refused. +- Every other change gets no test: a document, a manifest, a workflow, a deletion or a refactor that keeps behavior, an edit of tests only, or data. It names one command that can run again and gives the same answer, such as `uv run devops ci docs`, `uv run devops openspec check`, `! git grep -n src/ tests/` (a command that exits 0 when it finds nothing), or `uv run devops k8s image-pins`. When no command exists, it gives the steps of a repeatable manual check. The `! git grep` form has a fixed shape. The step runs it as a list of words, never as a shell. It takes the options `-n -I -F -i -w -l -c -q`, then one plain literal and relative paths. Or it takes one or more `-e` options, each with a plain literal, then `--` before the relative paths. A literal after `-e` must not start with `-`. A pipe, `;`, `&&`, a redirect, `-E`, `-P` and any other option are refused. - A task group of a plan has the same two kinds, `behavior` and `other` (`GroupKind`). The stage job refuses a `test` on an `other` group, and refuses a `behavior` group that changes no path under `src/`. A test compares the prompts of the job with these kinds. ### Doubles diff --git a/openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md b/openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md index c48a14679..2703177f5 100644 --- a/openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md +++ b/openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md @@ -27,7 +27,7 @@ Reads: `src/devops_cli/k8s/image_pins.py`, `src/devops_cli/commands/k8s/image_pi - [ ] 3.2 In `src/devops_cli/k8s/image_pins.py`, give `resolve_image` and `resolve_image_pins` a client and a timeout. `resolve_image` calls `tag_digest` and records `RegistryReadError`, `SSRFBlockedError` and `httpx2.HTTPError` in `PinResolution.error`. Delete `inspect_command`, the process call, the `planned` field of `PinResolution`, and the imports and constants that this leaves unused. In `src/devops_cli/commands/k8s/image_pins.py`, open `new_http_client(level=EgressLevel.PUBLIC)` with `DEFAULT_HTTP_TIMEOUT_SECONDS` only outside a dry run, remove the "planned" text from `_result_text`, and change the dry-run message in `src/devops_cli/lang/en/messages.py` to name the HEAD request. Write the docstring of the module and the help text of the command as they read now: a HEAD request that Docker Hub does not count - Verification: `uv run devops ci test tests/k8s/test_image_pins.py tests/docker/test_registry_tag_digest.py tests/argo/test_image_pins.py` passes - [ ] 3.3 Delete the four files of the imagetools fake, its recording, its record and its contract suite - - Verification: `! git grep -n -E "ImagetoolsFake|inspect_command|imagetools_recording|imagetools_runs" -- src tests` prints nothing, and `uv run devops ci test tests/k8s tests/docker tests/argo` passes + - Verification: `! git grep -n -e "ImagetoolsFake" -e "inspect_command" -e "imagetools_recording" -e "imagetools_runs" -- src tests` prints nothing, and `uv run devops ci test tests/k8s tests/docker tests/argo` passes ## 4. Fix the documents that name the old resolver diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/.openspec.yaml b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/.openspec.yaml new file mode 100644 index 000000000..1491b921e --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-05 diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/design.md b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/design.md new file mode 100644 index 000000000..d0646c003 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/design.md @@ -0,0 +1,42 @@ +# Design + +## Context + +The negative search is `! git grep [options] [--] ...` (#3263). The step reads the words in `read_negative_search` (`src/devops_cli/roadmap/implement_shell_policy.py`). The stage job reads the same function through `negative_search_parts` (`src/devops_cli/roadmap/stage_form.py`) and `_literal` (`src/devops_cli/roadmap/stage_move.py`). One literal is the only form the reader accepts. + +## Decisions + +### Repeat `-e`, and keep the positional literal + +Git matches a line that holds any of several `-e` patterns, with no `--and` and no `--all-match` (checked on git 2.55.0). A group with four names then needs one line, not four tasks. + +- Rejected: one `! git grep` line for each name. Each name adds a line or a task, and the plan loses its one-line check of a group. The two-name search of the first acceptance criterion cannot run as one line. +- Rejected: replace the positional literal with `-e` only. Committed plans, such as task 1.1 of #2610, use the positional form. Dropping it refuses every committed line of that shape. + +### Each `-e` value is a plain literal + +Each value passes the same check as the positional literal: the signs of `NEGATIVE_SEARCH_LITERAL_SIGNS` and no leading `-`. A value that starts with `-` is refused with its own reason, so `-E`, `-P` and an option word stay refused as a value. The value `--` gets its own reason: it is a missing literal. + +- Rejected: let a value start with `-`. Git would read it as a pattern, but the grammar could no longer tell an option word from a literal, and `-E` as a value would pass unnoticed. + +### `--` is required with `-e`, and no word may come before it + +With `-e`, git reads a positional word as a path. The search would then cover other paths, and the plan would not say so. The reader requires `--` after the last `-e` value and refuses any word between them. + +- Rejected: let git read the word as a path. The search covers paths that the plan does not name. +- Rejected: accept a mixed form with a positional literal and `-e`. A plain word is both a literal and a path, so the reader cannot tell which the plan meant. + +### The stage job checks each literal + +`record_search_evidence` records the staged hits for each literal. `search_problems` refuses each literal with no staged hit, and each literal that a created module keeps. A literal with no staged hit already passes before the change, so the check holds for each literal alone. + +- Rejected: check only the first literal. A second literal with no hit proves nothing, and the stage job would accept it. + +### Move groups check each literal + +`move_verify_problems` refuses a move group when any literal names one module of the moved prefix. + +## Rejected for the scope of this change + +- A test over every committed `tasks.md` (AC3 of #3276). Three active plans hold refused commands that this change does not fix. #3187 holds regex lines with boundaries that the grammar cannot express, and its plan needs a rewrite by its owner. The test waits for that rewrite (see Workflow follow-up in the proposal). +- A fix of the prose code spans of `step_files.py`. It is a separate cause, so it gets its own issue. diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/proposal.md b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/proposal.md new file mode 100644 index 000000000..a8400383c --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/proposal.md @@ -0,0 +1,56 @@ +# Proposal + +## Why + +The negative search of the implement step takes one literal and no `-e` option (#3263). A group that removes several names cannot check them in one command. The plan of #2610 has a group 3 line with `-E` and four names (task 3.3). The step refuses that line with "the option `-E` is not allowed", so the group cannot run (#3276). + +## What Changes + +- `read_negative_search` in `src/devops_cli/roadmap/implement_shell_policy.py` accepts one or more `-e ` options. Each literal is a plain literal, as the positional literal is. The search takes `--` before the paths when it has `-e` options. The positional literal form stays. +- `NegativeSearch` holds a tuple of literals and a flag that marks the `-e` form. The `argv` of the search passes each literal after its own `-e`. The field of one literal goes away. +- The stage job checks each literal of a negative search. `record_search_evidence` and `search_problems` in `src/devops_cli/roadmap/stage_form.py` and `move_verify_problems` in `src/devops_cli/roadmap/stage_move.py` read every literal. +- `src/devops_cli/lang/en/messages.py` holds three new reasons: an `-e` option with no literal, a word that comes after an `-e` literal, and an `-e` value that starts with `-`. +- `src/devops_cli/ai/tasks/roadmap_stage_change.md` names the `-e` form, and `tests/roadmap/test_roadmap_implement_negative_search.py` asserts it. +- `docs/TESTING.md` states the shape of the `! git grep` form: one plain literal, or one or more `-e` literals and `--`, with the refused tokens. +- Task 3.3 of `openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md` uses four `-e` literals in place of `-E` and the pipe. +- The change fits one pull request. The invariant test of every committed plan is not part of it (see Workflow follow-up). + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `roadmap-implement-step`: A negative search takes one literal or repeated `-e` literals. Each literal keeps the plain-literal rule. +- `roadmap-stage`: The stage job records the hits of each literal of a negative search and checks each one. + +## Impact + +- Code: `src/devops_cli/roadmap/implement_shell_policy.py`, `src/devops_cli/roadmap/stage_form.py`, `src/devops_cli/roadmap/stage_move.py`, `src/devops_cli/lang/en/messages.py` +- Documents: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `docs/TESTING.md`, `openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md` +- Tests: `tests/roadmap/test_roadmap_implement_negative_search.py`, `tests/roadmap/test_roadmap_implement_shell_policy.py`, `tests/roadmap/test_roadmap_stage_form.py`, `tests/roadmap/test_roadmap_stage_plan.py` + +## Feasibility + +- `git grep` accepts `-e` more than once. A scratch repository at git 2.55.0 gave exit 0 with the matching line for `git grep -n -e alpha -e zzz -- src`, and exit 1 for `git grep -n -e zzz -e qqq -- src`. Exit 1 also holds when the path matches no file (`git grep -n -e zzz -- nothere`). +- The premise holds at origin/main (d254410c). `NEGATIVE_SEARCH_OPTION_LETTERS = "nIFiwlcq"` in `src/devops_cli/roadmap/implement_shell_policy.py` excludes `e`. The test `test_a_negative_search_outside_the_fixed_shape_is_refused_and_starts_no_process` pins `! git grep -e name src/` as refused. +- `read_verification` from `src/devops_cli/roadmap/implement_shell_policy.py` gives the refusal "the option `-E` is not allowed" for the task 3.3 line of #2610. The same reader run over every active `tasks.md` gave 16 refused commands in three plans: #2610 (group 2 prose spans and the `-E` line), #2441 (`helm` and a prose span) and #3187 (`test -z` lines and three `-E` regex lines with boundaries). Only the `-E` lines of #2610 and #3187 are negative searches, and the change fixes only #2610, because #3187 needs a regex that the grammar cannot express. +- Group 2 of #2610 also stops the step before group 3. Its verification line holds prose code spans (`tagged_image`, `nginx:1.27`) that the step reads as commands. That cause is outside this change. + +## Existing Tool Considered + +`git grep` covers the search. It matches any of several `-e` patterns in one command, so the change keeps git as the program that searches. Git has no option policy: it accepts `-E`, `-P` and a regex pattern. The step keeps its allowlist and its plain-literal check, and it requires `--` before the paths when `-e` is used. The change builds no search engine. The grammar in the step adds about 40 lines. + +Internal code that the change deletes: the one-literal field of `NegativeSearch` (`literal`), the comment that refuses `-e` (`implement_shell_policy.py`, lines 150-151), the refusal case `! git grep -e name src/` in `tests/roadmap/test_roadmap_implement_negative_search.py`. + +## Dependencies + +- #3263 added the negative search. The change extends it. +- The plan of #2610 is edited in this change. #3490 also lists #2610, so its edit of that plan must follow this change or merge into it. + +## Workflow follow-up + +- #3579 tracks the test over every active plan (acceptance criterion 3 of #3276). This change does not add that test. After this change, 15 refused Verification commands remain in three active plans, and #3579 holds the list. +- #3596 tracks the step that reads every code span of a Verification line as a command (`src/devops_cli/roadmap/step_files.py`). diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-implement-step/spec.md b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-implement-step/spec.md new file mode 100644 index 000000000..4e1fb8642 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-implement-step/spec.md @@ -0,0 +1,40 @@ +## MODIFIED Requirements + +### Requirement: A negative search is a verification command in a fixed shape +The step SHALL run a verification command that starts with `!` only as `! git grep [options] [--] ...` or as `! git grep [options] -e [-e ]... -- ...`, with at least one path and with the options `-n -I -F -i -w -l -c -q` in words of those letters. The step SHALL start no process for a command outside the shape, and the reason of the stop SHALL say which rule of the shape the command breaks. + +#### Scenario: A command that is not the fixed shape +- **WHEN** a Verification line names `! rm -rf src/`, `! git grep -n name src/ | cat`, `! git grep -n name src/ ; ls`, `! git grep -n name src/ && ls`, `! git grep -n name src/ > out.txt`, `! git grep -n '$(id)' src/`, `! git grep -O name src/`, `! git grep --no-index -n name src/`, `! git grep -f patterns.txt src/`, `! git grep --open-files-in-pager=cat name src/`, `! git grep -n name /etc` or `! git grep -n name ../other` +- **THEN** the step stops with the reason that says which rule of the shape the command breaks, and starts no process + +#### Scenario: A letter outside ASCII +- **WHEN** a Verification line names a literal with a fullwidth `ｆ` or an accented `é`, or a path with one +- **THEN** the step stops with the reason that the text is not a literal, or that the word is not a path, and starts no process + +## ADDED Requirements + +### Requirement: A negative search takes repeated -e literals +The step SHALL read each `-e` option of a negative search as the option that takes the literal after it. Each such literal SHALL hold only the characters of a positional literal and SHALL NOT start with `-`. The step SHALL run `git grep` with each literal after its own `-e`, and SHALL pass the search when git exits 1. + +#### Scenario: Two names that do not occur +- **WHEN** a Verification line names `! git grep -n -e "ImagetoolsFake" -e "inspect_command" -- src tests` and no file under `src` or `tests` holds either name +- **THEN** the step runs `git grep -n -e ImagetoolsFake -e inspect_command -- src tests` and the verification command passes + +#### Scenario: One of the names occurs +- **WHEN** a Verification line names `! git grep -n -e hello -e absent_name -- src/ tests/` and `src/sample.py` holds `hello` +- **THEN** the step stops with the matching lines that git printed, and starts no other process + +#### Scenario: A value that is not a plain literal +- **WHEN** a Verification line names `! git grep -n -e 'a|b' -- src/`, `! git grep -n -e 'a;b' -- src/` or `! git grep -n -e -- src/` +- **THEN** the step stops with the reason that names the text, or that the `-e` option has no literal, and starts no process + +#### Scenario: A value that starts with - +- **WHEN** a Verification line names `! git grep -n -e -E -- src/` or `! git grep -n -e --addons -- src/` +- **THEN** the step stops with the reason that the text starts with `-`, and starts no process + +### Requirement: A negative search with -e puts -- before its paths +A negative search with `-e` options SHALL have `--` after its last literal and before its paths. It SHALL NOT have a word between the last literal and `--`. + +#### Scenario: Both forms in one line +- **WHEN** a Verification line names `! git grep -e name literal -- src/`, or `! git grep -e name src/` +- **THEN** the step stops with the reason that names both forms of the search, and starts no process diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-stage/spec.md b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-stage/spec.md new file mode 100644 index 000000000..0a689e8f0 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/specs/roadmap-stage/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: The stage job checks each literal of a negative search +The stage job SHALL record the staged hits of each `-e` literal of a negative-search verify command, and check_proposal SHALL apply the rules of the negative search to each literal, the rule of the staged hit and the rule of the created module alike. + +#### Scenario: One literal has no staged hit +- **WHEN** A group gives the verify command `! git grep -n -e old_name -e gone_name -- src/ tests/`, and the staged tree holds `old_name` but no `gone_name`. +- **THEN** check_proposal returns one problem that names the literal `gone_name` and says that the search already passes before the change. + +#### Scenario: A move group names a module with a second literal +- **WHEN** A move group for prefix `stage` gives the verify command `! git grep -n -e devops_cli.roadmap.stage_ready -e devops_cli.roadmap.stage -- src/ tests/`. +- **THEN** check_proposal returns two problems: the problem of the move and the problem of the created module, and each names the literal `devops_cli.roadmap.stage`. diff --git a/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/tasks.md b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/tasks.md new file mode 100644 index 000000000..bc530fb87 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3276-negative-search-accepts-repeated-e/tasks.md @@ -0,0 +1,50 @@ +# Tasks + +## 1. Repeated -e literals in the negative search + +Reads: `src/devops_cli/roadmap/implement_shell_policy.py`, `src/devops_cli/lang/en/messages.py`, `tests/roadmap/test_roadmap_implement_negative_search.py`, `tests/roadmap/test_roadmap_implement_shell_policy.py`. Changes: `src/devops_cli/roadmap/implement_shell_policy.py`, `src/devops_cli/lang/en/messages.py`, `tests/roadmap/test_roadmap_implement_negative_search.py`, `tests/roadmap/test_roadmap_implement_shell_policy.py`. + +- [x] 1.1 Write the failing tests in `tests/roadmap/test_roadmap_implement_negative_search.py`. A search `! git grep -n -e ImagetoolsFake -e inspect_command -e imagetools_recording -e imagetools_runs -- src tests` runs `git grep` with one `-e` for each name, passes on exit 1, and stops with the matching lines on exit 0. A pipe, a shell sign, `-E` or `-P` in an `-e` value is refused, a value that starts with `-` gets the reason that states this rule, and so is an `-e` with no literal, an `-e` followed by `--`, and a word between the last `-e` value and `--`. Remove the `-e name` case from the refusal list + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_negative_search.py` fails on the base, because `read_negative_search` refuses `-e` with "the option `-e` is not allowed" +- [x] 1.2 In `src/devops_cli/roadmap/implement_shell_policy.py`, make `NegativeSearch` hold `literals` and the flag `by_e`, and read the `-e` form in `read_negative_search`. `argv` passes each literal after its own `-e`. Delete the field `literal`, and the comment that refuses `-e`. Add the reasons `search_reason_e_value`, `search_reason_e_dash`, `search_reason_e_forms` to `src/devops_cli/lang/en/messages.py` + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_negative_search.py` passes +- [x] 1.3 Update `tests/roadmap/test_roadmap_implement_shell_policy.py` to the new shape of `NegativeSearch`: the positional form holds one literal in `literals`, and a case reads the `-e` form + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_shell_policy.py` passes + +## 2. The stage job checks each literal + +Reads: `src/devops_cli/roadmap/stage_form.py`, `src/devops_cli/roadmap/stage_move.py`, `tests/roadmap/test_roadmap_stage_form.py`, `tests/roadmap/test_roadmap_stage_plan.py`. Changes: `src/devops_cli/roadmap/stage_form.py`, `src/devops_cli/roadmap/stage_move.py`, `tests/roadmap/test_roadmap_stage_form.py`, `tests/roadmap/test_roadmap_stage_plan.py`. + +- [x] 2.1 Write the failing tests `test_each_e_literal_has_its_own_hit_check` in `tests/roadmap/test_roadmap_stage_form.py` and `test_a_move_group_checks_each_e_literal` in `tests/roadmap/test_roadmap_stage_plan.py`. The first gives a group with two `-e` literals, where one has a staged hit and one has none, and expects one problem that names the literal with no hit. The second gives a move group whose second `-e` literal is the bare prefix, and expects two problems that name this literal: the problem of the move and the problem of the created module + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_form.py tests/roadmap/test_roadmap_stage_plan.py` fails on the base, because `read_negative_search` refuses `-e`, so `negative_search_parts` returns None and the stage job checks no literal +- [x] 2.2 In `src/devops_cli/roadmap/stage_form.py`, `record_search_evidence` and `search_problems` check each literal of `NegativeSearch.literals`. In `src/devops_cli/roadmap/stage_move.py`, `_literals` returns the literals, and `move_verify_problems` checks each one + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_form.py tests/roadmap/test_roadmap_stage_plan.py` passes + +## 3. The stage prompt names the -e form + +Reads: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_implement_negative_search.py`. Changes: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_implement_negative_search.py`. + +- [x] 3.1 Extend `test_the_stage_prompt_gives_the_options_and_the_signs_that_the_step_reads` in `tests/roadmap/test_roadmap_implement_negative_search.py` to assert that the prompt names the form `-e`, the rule that `--` comes before the paths, and the rule that a literal after `-e` must not start with `-` + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_negative_search.py` fails on the base +- [x] 3.2 In `src/devops_cli/ai/tasks/roadmap_stage_change.md`, state the `-e` form in the sentence of the negative search: one literal, or one or more `-e` literals and then `--` before the paths. A literal after `-e` must not start with `-` + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_negative_search.py` passes + +## 4. The plan of #2610 uses the allowed form + +Reads: `openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md`. Changes: `openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md`. + +- [x] 4.1 In task 3.3 of `openspec/changes/2610-devops-k8s-image-pins-resolves-with/tasks.md`, replace `-E "ImagetoolsFake|inspect_command|imagetools_recording|imagetools_runs"` with four `-e` literals: `ImagetoolsFake`, `inspect_command`, `imagetools_recording` and `imagetools_runs`. Keep the rest of the line + - Verification: `uv run devops openspec check` passes + +## 5. Document the negative-search form + +Reads: `docs/TESTING.md`. Changes: `docs/TESTING.md`. + +- [x] 5.1 In the negative-search paragraph of `docs/TESTING.md`, state the form: one plain literal, or one or more `-e` literals with `--` before the paths. A literal after `-e` must not start with `-`. A pipe, `;`, `&&`, a redirect, `-E`, `-P` and any other option stay refused + - Verification: `uv run devops ci docs` passes + +## Workflow follow-up + +- #3579 tracks the test over every active plan (acceptance criterion 3 of #3276). This change does not add that test. +- #3596 tracks the step that reads every code span of a Verification line as a command. +- Pending a person: review the diff in tuicr (tuicr -r ..HEAD). diff --git a/openspec/specs/roadmap-implement-step/spec.md b/openspec/specs/roadmap-implement-step/spec.md index 2530fa96f..d6271a94b 100644 --- a/openspec/specs/roadmap-implement-step/spec.md +++ b/openspec/specs/roadmap-implement-step/spec.md @@ -897,7 +897,7 @@ The settings `roadmap.implement.request_timeout_seconds`, `roadmap.implement.com - **THEN** the watch waits at most 123 seconds, and the reason of the stop names 123 seconds ### Requirement: A negative search is a verification command in a fixed shape -The step SHALL run a verification command that starts with `!` only as `! git grep [options] [--] ...`, with at least one path and with the options `-n -I -F -i -w -l -c -q` in words of those letters. The step SHALL start no process for a command outside the shape, and the reason of the stop SHALL say which rule of the shape the command breaks. +The step SHALL run a verification command that starts with `!` only as `! git grep [options] [--] ...` or as `! git grep [options] -e [-e ]... -- ...`, with at least one path and with the options `-n -I -F -i -w -l -c -q` in words of those letters. The step SHALL start no process for a command outside the shape, and the reason of the stop SHALL say which rule of the shape the command breaks. #### Scenario: A command that is not the fixed shape - **WHEN** a Verification line names `! rm -rf src/`, `! git grep -n name src/ | cat`, `! git grep -n name src/ ; ls`, `! git grep -n name src/ && ls`, `! git grep -n name src/ > out.txt`, `! git grep -n '$(id)' src/`, `! git grep -O name src/`, `! git grep --no-index -n name src/`, `! git grep -f patterns.txt src/`, `! git grep --open-files-in-pager=cat name src/`, `! git grep -n name /etc` or `! git grep -n name ../other` @@ -1256,3 +1256,29 @@ When the agent ends its run, each verification command passes, and `ruff check`, #### Scenario: A hook that the agent cannot fix - **WHEN** the `pre-commit` stage exits 1 after every turn, and `repair_rounds` is 3 - **THEN** the step reports stopped with the reason that holds the output of the hook, the model got three extra turns, `repair_rounds` is 3, and the checkout is reset + +### Requirement: A negative search takes repeated -e literals +The step SHALL read each `-e` option of a negative search as the option that takes the literal after it. Each such literal SHALL hold only the characters of a positional literal and SHALL NOT start with `-`. The step SHALL run `git grep` with each literal after its own `-e`, and SHALL pass the search when git exits 1. + +#### Scenario: Two names that do not occur +- **WHEN** a Verification line names `! git grep -n -e "ImagetoolsFake" -e "inspect_command" -- src tests` and no file under `src` or `tests` holds either name +- **THEN** the step runs `git grep -n -e ImagetoolsFake -e inspect_command -- src tests` and the verification command passes + +#### Scenario: One of the names occurs +- **WHEN** a Verification line names `! git grep -n -e hello -e absent_name -- src/ tests/` and `src/sample.py` holds `hello` +- **THEN** the step stops with the matching lines that git printed, and starts no other process + +#### Scenario: A value that is not a plain literal +- **WHEN** a Verification line names `! git grep -n -e 'a|b' -- src/`, `! git grep -n -e 'a;b' -- src/` or `! git grep -n -e -- src/` +- **THEN** the step stops with the reason that names the text, or that the `-e` option has no literal, and starts no process + +#### Scenario: A value that starts with - +- **WHEN** a Verification line names `! git grep -n -e -E -- src/` or `! git grep -n -e --addons -- src/` +- **THEN** the step stops with the reason that the text starts with `-`, and starts no process + +### Requirement: A negative search with -e puts -- before its paths +A negative search with `-e` options SHALL have `--` after its last literal and before its paths. It SHALL NOT have a word between the last literal and `--`. + +#### Scenario: Both forms in one line +- **WHEN** a Verification line names `! git grep -e name literal -- src/`, or `! git grep -e name src/` +- **THEN** the step stops with the reason that names both forms of the search, and starts no process diff --git a/openspec/specs/roadmap-stage/spec.md b/openspec/specs/roadmap-stage/spec.md index e540cec2d..dc7179fe1 100644 --- a/openspec/specs/roadmap-stage/spec.md +++ b/openspec/specs/roadmap-stage/spec.md @@ -1992,3 +1992,14 @@ The job SHALL start the branch of a staged item at the staged commit of its roun #### Scenario: origin moves after the round read the default branch - **WHEN** `origin/main` gains a commit after the round read it, and the job stages an item - **THEN** the parent of the commit of the new branch is the staged commit of the round, and the new commit of `origin/main` is not an ancestor of the new branch + +### Requirement: The stage job checks each literal of a negative search +The stage job SHALL record the staged hits of each `-e` literal of a negative-search verify command, and check_proposal SHALL apply the rules of the negative search to each literal, the rule of the staged hit and the rule of the created module alike. + +#### Scenario: One literal has no staged hit +- **WHEN** A group gives the verify command `! git grep -n -e old_name -e gone_name -- src/ tests/`, and the staged tree holds `old_name` but no `gone_name`. +- **THEN** check_proposal returns one problem that names the literal `gone_name` and says that the search already passes before the change. + +#### Scenario: A move group names a module with a second literal +- **WHEN** A move group for prefix `stage` gives the verify command `! git grep -n -e devops_cli.roadmap.stage_ready -e devops_cli.roadmap.stage -- src/ tests/`. +- **THEN** check_proposal returns two problems: the problem of the move and the problem of the created module, and each names the literal `devops_cli.roadmap.stage`. diff --git a/src/devops_cli/ai/tasks/roadmap_stage_change.md b/src/devops_cli/ai/tasks/roadmap_stage_change.md index 07c2f1e42..455409ff0 100644 --- a/src/devops_cli/ai/tasks/roadmap_stage_change.md +++ b/src/devops_cli/ai/tasks/roadmap_stage_change.md @@ -15,7 +15,7 @@ The rules of the project: - In `decisions`, record each design decision with its reason. In `rejected`, record each alternative that you rejected with the evidence that rejected it. - Write `groups` in the order of the work. Each group holds the work of one step of an implementation, and fits in the `budget` with its files: a step reads the instructions of the group, the `AGENTS.md` of the repository and the sections of the documents that it routes the group to, the issue, the change and the files of the group. Keep each group small. A group has these keys: `title`; `kind`, `behavior` or `other`; `reads`, the repository paths that the step reads, an empty list when it reads none; `changes`, the repository paths that the step changes or creates, with at least one path, because every group changes a file; `creates`, the paths of `changes` that do not exist yet, an empty list when it creates none; `creates_lines`, an object that gives the estimated number of lines of each path in `creates`, such as `{"tests/roadmap/test_sample.py": 40}`; `test`, the failing test to write first, and `null` for an `other` group; `implementation`, the work of the step; `documentation`, one text that starts with an imperative verb, such as `Edit`, and names each document in a code span, or `null` when the step changes no document; `verify`, one command that checks the group and gives the same answer on each run, such as `uv run devops ci test tests/roadmap/test_feature.py`, or `null`; and `process`, the steps of a manual check, or `null`. Every task has a text: `title` and `implementation` are never blank, and no other key that you give is blank. - The stage counts 32 tokens for each estimated line of a path in `creates_lines`, so the estimate of a new file counts toward the `budget` of the step. Give a whole number of lines of at least 1 for each path in `creates`. A path in `creates` with no estimate is refused. -- Choose the `kind` of each group by what the group does. A `behavior` group adds or changes what the code does. It changes a path under `src/`, it has a `test`, its `verify` runs that test, and it has no `process`. It is written test first. Write tests only for new behavior. Every other group is `other`: a document, a manifest, a workflow, a deletion or a refactor that keeps behavior, an edit of tests only, or data. An `other` group has no `test`, so set `test` to `null`. It has exactly one of `verify` and `process`. `verify` is one command that can run again, such as `uv run devops ci docs`, `uv run devops openspec check`, `! git grep -n src/ tests/` (a command that exits 0 when it finds nothing), or `uv run devops k8s image-pins`. A `verify` command starts with `uv run devops`, and the `! git grep` form is the one other form. The `! git grep` form is not a shell command: it takes the options `-n -I -F -i -w -l -c -q`, then one literal of letters, digits, spaces and the signs `_ . : / = @ + , # % ~ -`, then relative paths, and a pipe, `;`, `&&`, a redirect or any other option is refused. `uv run pytest` is refused. `process` is the steps of a repeatable manual check, and you write it only when no command exists. A reply that breaks these rules comes back to you with the problem. +- Choose the `kind` of each group by what the group does. A `behavior` group adds or changes what the code does. It changes a path under `src/`, it has a `test`, its `verify` runs that test, and it has no `process`. It is written test first. Write tests only for new behavior. Every other group is `other`: a document, a manifest, a workflow, a deletion or a refactor that keeps behavior, an edit of tests only, or data. An `other` group has no `test`, so set `test` to `null`. It has exactly one of `verify` and `process`. `verify` is one command that can run again, such as `uv run devops ci docs`, `uv run devops openspec check`, `! git grep -n src/ tests/` (a command that exits 0 when it finds nothing), or `uv run devops k8s image-pins`. A `verify` command starts with `uv run devops`, and the `! git grep` form is the one other form. The `! git grep` form is not a shell command: it takes the options `-n -I -F -i -w -l -c -q`, then one literal of letters, digits, spaces and the signs `_ . : / = @ + , # % ~ -`, then relative paths. Or it takes one or more `-e` options, each followed by its literal, then `--` before the relative paths. A literal after `-e` must not start with `-`. A pipe, `;`, `&&`, a redirect or any other option is refused. `uv run pytest` is refused. `process` is the steps of a repeatable manual check, and you write it only when no command exists. A reply that breaks these rules comes back to you with the problem. - A group that moves a set of modules by name prefix lists every module of that prefix in `changes`, and not the entry module only. Its `verify` command covers the whole prefix: run the layout test of the plan, or search the dotted prefix with the separator (`devops_cli.roadmap.stage_`), and never one sampled module. Before you give a `! git grep` command, call `search_code` with its literal on the staged commit and quote the hit count in `feasibility`. A literal that the plan creates a module for, such as a bare package prefix or a name that no file holds, is no check: the search then proves nothing before the change or can never pass after it. - A new test module in `creates` goes in the folder under `tests/` that mirrors the package of the code it tests, such as `tests/roadmap/test_feature.py` for code in `src/devops_cli/roadmap/`, or in `tests/invariants/`. The check refuses a new test module that sits flat under `tests/` when it tests code of a package. - A path is relative to the repository root. It has no `..` and does not start with `/`. A sample file that a test writes at run time, and that no group creates, gets a bare name with no folder, such as `test_sample.py`. Put no folder before such a name: the check reads a name with a folder as a path of the repository. diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index 1f54f40b7..00ac07ffe 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -2507,6 +2507,12 @@ class RoadmapMessages: ) search_reason_no_path: str = "it gives no path" search_reason_path: str = "the word `{word}` is not a path of the checkout" + search_reason_e_value: str = "the option `-e` has no literal after it" + search_reason_e_dash: str = "the text `{word}` starts with `-`, so it is not an `-e` literal" + search_reason_e_forms: str = ( + "the word `{word}` is not allowed with `-e`, so use one literal, or `-e` literals and then " + "`--`" + ) verify_reason_unparsed: str = "it is not a valid command line" verify_reason_bytes: str = "a word holds a NUL byte or a character that cannot be encoded" verify_reason_long: str = "it is longer than {limit} characters" diff --git a/src/devops_cli/roadmap/implement_shell_policy.py b/src/devops_cli/roadmap/implement_shell_policy.py index 44f4b9da5..06654ce5c 100644 --- a/src/devops_cli/roadmap/implement_shell_policy.py +++ b/src/devops_cli/roadmap/implement_shell_policy.py @@ -147,14 +147,16 @@ class ShellRefusal: # The negative search: `! git grep [options] [--] ...`. The step runs the words # after `!` and the search passes when git exits 1, which means no match. An option is one word of # these letters: line numbers, no binary files, fixed strings, any case, whole words, file names, -# counts and a quiet run. No option reads a file (`-f`), opens a program (`-O`), leaves the -# repository (`--no-index`) or takes a value (`-e`), so every other option is refused. +# counts and a quiet run. The option `-e` takes one literal, and a search may repeat it: git matches +# any of the literals. No other option reads a file (`-f`), opens a program (`-O`), leaves the +# repository (`--no-index`) or takes a value, so every other option is refused. NEGATIVE_SEARCH_START: tuple[str, ...] = (NEGATIVE_SEARCH_WORD, BIN_GIT, "grep") NEGATIVE_SEARCH_OPTION_LETTERS = "nIFiwlcq" NEGATIVE_SEARCH_LITERAL_SIGNS = "_ . : / = @ + , # % ~ -" _OPTION = re.compile(rf"-[{NEGATIVE_SEARCH_OPTION_LETTERS}]+") _LITERAL = re.compile(r"[\w .:/=@+,#%~-]+", re.ASCII) _PATH = re.compile(r"[\w.+@~*/-]+", re.ASCII) +_E_OPTION = "-e" _PATHS_MARK = "--" _SHOWN_CHARS = 60 @@ -227,17 +229,24 @@ def _is_path(word: str) -> bool: @dataclass(frozen=True) class NegativeSearch: - """A negative search in the fixed shape, read from its words.""" + """A negative search in the fixed shape, read from its words. `literals` holds one literal, or + each `-e` literal when `by_e` is set.""" options: tuple[str, ...] - literal: str + literals: tuple[str, ...] paths: tuple[str, ...] + by_e: bool = False @property def argv(self) -> list[str]: - """The command that the step runs: the paths follow `--`, so git never reads one as a - revision.""" - return [BIN_GIT, "grep", *self.options, self.literal, _PATHS_MARK, *self.paths] + """The command that the step runs: each literal follows its own `-e` when `by_e` is set, + and the paths follow `--`, so git never reads one as a revision.""" + patterns = ( + [word for literal in self.literals for word in (_E_OPTION, literal)] + if self.by_e + else list(self.literals) + ) + return [BIN_GIT, "grep", *self.options, *patterns, _PATHS_MARK, *self.paths] def shown(text: str, limit: int = _SHOWN_CHARS) -> str: @@ -249,22 +258,56 @@ def shown(text: str, limit: int = _SHOWN_CHARS) -> str: return escaped if len(escaped) <= limit else f"{escaped[:limit]}..." +def _read_e_literals(options: tuple[str, ...], rest: list[str]) -> NegativeSearch | str: + """The negative search whose literals are `-e` options, read from `rest`, which starts with the + first `-e`. Each value is a plain literal and never a word that starts with `-`. `--` comes + after the last value and before the paths, and no word may stand between them.""" + texts = MESSAGES.roadmap + literals: list[str] = [] + while rest[:1] == [_E_OPTION]: + rest.pop(0) + if not rest or rest[0] == _PATHS_MARK: + return texts.search_reason_e_value + value = rest.pop(0) + if value.startswith("-"): + return texts.search_reason_e_dash.format(word=shown(value)) + if _LITERAL.fullmatch(value) is None: + return texts.search_reason_literal.format( + word=shown(value), signs=NEGATIVE_SEARCH_LITERAL_SIGNS + ) + literals.append(value) + if not rest: + return texts.search_reason_no_path + if rest[0] != _PATHS_MARK: + return texts.search_reason_e_forms.format(word=shown(rest[0])) + del rest[0] + if not rest: + return texts.search_reason_no_path + bad = next((word for word in rest if not _is_path(word)), None) + if bad is not None: + return texts.search_reason_path.format(word=shown(bad)) + return NegativeSearch(options, tuple(literals), tuple(rest), by_e=True) + + def read_negative_search(words: Sequence[str]) -> NegativeSearch | str: """The negative search in `words`, or the reason that the step refuses it. The words are `!`, - `git`, `grep`, options of `NEGATIVE_SEARCH_OPTION_LETTERS`, one literal, an optional `--` and at - least one path. The reason is a sentence for the reason of the step and for the gate.""" + `git`, `grep`, options of `NEGATIVE_SEARCH_OPTION_LETTERS`, then either one literal or one or + more `-e` literals, an optional `--` (required with `-e`) and at least one path. The reason is a + sentence for the reason of the step and for the gate.""" texts = MESSAGES.roadmap if tuple(words[:3]) != NEGATIVE_SEARCH_START: return texts.search_reason_start rest = list(words[3:]) options: list[str] = [] - while rest and rest[0].startswith("-"): + while rest and rest[0].startswith("-") and rest[0] != _E_OPTION: if _OPTION.fullmatch(rest[0]) is None: return texts.search_reason_option.format( word=shown(rest[0]), letters=" ".join(f"-{c}" for c in NEGATIVE_SEARCH_OPTION_LETTERS), ) options.append(rest.pop(0)) + if rest[:1] == [_E_OPTION]: + return _read_e_literals(tuple(options), rest) if not rest: return texts.search_reason_no_text literal = rest.pop(0) @@ -279,7 +322,7 @@ def read_negative_search(words: Sequence[str]) -> NegativeSearch | str: bad = next((word for word in rest if not _is_path(word)), None) if bad is not None: return texts.search_reason_path.format(word=shown(bad)) - return NegativeSearch(tuple(options), literal, tuple(rest)) + return NegativeSearch(tuple(options), (literal,), tuple(rest)) @dataclass(frozen=True) diff --git a/src/devops_cli/roadmap/stage_form.py b/src/devops_cli/roadmap/stage_form.py index 88008fbff..a22758480 100644 --- a/src/devops_cli/roadmap/stage_form.py +++ b/src/devops_cli/roadmap/stage_form.py @@ -145,9 +145,12 @@ def record_search_evidence( if group.verify is None: continue found = negative_search_parts(group.verify) - if found is None or found.literal in evidence: + if found is None: continue - evidence[found.literal] = tuple(sorted({path for path in search(found.literal) if path})) + for literal in found.literals: + if literal in evidence: + continue + evidence[literal] = tuple(sorted({path for path in search(literal) if path})) return evidence @@ -184,24 +187,25 @@ def search_problems( found = negative_search_parts(group.verify) if found is None: continue - kept = next(((path, name) for path, name in created if found.literal in name), None) - if kept is not None: - path, import_path = kept - problems.append( - texts.stage_problem_search_created.format( - group=number, - command=group.verify, - literal=found.literal, - path=path, - import_path=import_path, + for literal in found.literals: + kept = next(((path, name) for path, name in created if literal in name), None) + if kept is not None: + path, import_path = kept + problems.append( + texts.stage_problem_search_created.format( + group=number, + command=group.verify, + literal=literal, + path=path, + import_path=import_path, + ) ) - ) - if evidence is not None and not evidence.get(found.literal): - problems.append( - texts.stage_problem_search_no_hit.format( - group=number, command=group.verify, literal=found.literal + if evidence is not None and not evidence.get(literal): + problems.append( + texts.stage_problem_search_no_hit.format( + group=number, command=group.verify, literal=literal + ) ) - ) return problems diff --git a/src/devops_cli/roadmap/stage_move.py b/src/devops_cli/roadmap/stage_move.py index 33aa893b9..1660ccd1a 100644 --- a/src/devops_cli/roadmap/stage_move.py +++ b/src/devops_cli/roadmap/stage_move.py @@ -114,37 +114,38 @@ def move_prefix_problems(groups: Sequence[StageGroup], tree: StagedTree) -> list ] -def _literal(command: str) -> str | None: - """The literal of a valid `! git grep` command, or None for another command.""" +def _literals(command: str) -> tuple[str, ...]: + """The literals of a valid `! git grep` command, or none for another command.""" try: words = shlex.split(command) except ValueError: - return None + return () if words[:1] != [NEGATIVE_SEARCH_WORD]: - return None + return () found = read_negative_search(words) - return None if isinstance(found, str) else found.literal + return () if isinstance(found, str) else found.literals def move_verify_problems(groups: Sequence[StageGroup], tree: StagedTree) -> list[str]: - """One sentence for each move group whose `! git grep` literal names one module of the - prefix, the bare prefix included.""" + """One sentence for each move group whose `! git grep` literal, each `-e` literal included, + names one module of the prefix, the bare prefix included.""" texts = MESSAGES.roadmap problems: list[str] = [] for move in _moves(groups, tree): group = groups[move.number - 1] - if group.verify is None or (literal := _literal(group.verify)) is None: + if group.verify is None: continue stems = {PurePosixPath(path).stem for path in move.modules} | {move.prefix} named = {f"{move.import_path}.{stem}" for stem in stems} - if literal in named: - problems.append( - texts.stage_problem_move_verify.format( - group=move.number, - prefix=move.prefix, - command=group.verify, - literal=literal, - expected=f"{move.import_path}.{move.prefix}_", - ) + problems.extend( + texts.stage_problem_move_verify.format( + group=move.number, + prefix=move.prefix, + command=group.verify, + literal=literal, + expected=f"{move.import_path}.{move.prefix}_", ) + for literal in _literals(group.verify) + if literal in named + ) return problems diff --git a/tests/roadmap/test_roadmap_implement_negative_search.py b/tests/roadmap/test_roadmap_implement_negative_search.py index 79864b89e..9169da18d 100644 --- a/tests/roadmap/test_roadmap_implement_negative_search.py +++ b/tests/roadmap/test_roadmap_implement_negative_search.py @@ -130,7 +130,6 @@ def _path(word: str) -> str: ("! git grep --no-index -n name src/", f"the option `--no-index` is not allowed {OPTIONS}"), ("! git grep -f patterns.txt src/", f"the option `-f` is not allowed {OPTIONS}"), ("! git grep --ext-grep name src/", f"the option `--ext-grep` is not allowed {OPTIONS}"), - ("! git grep -e name src/", f"the option `-e` is not allowed {OPTIONS}"), ("! git grep -nP name src/", f"the option `-nP` is not allowed {OPTIONS}"), ("! git grep -n -- name src/", f"the option `--` is not allowed {OPTIONS}"), ("! git grep -n", "it gives no text to search"), @@ -261,6 +260,9 @@ def test_the_stage_prompt_gives_the_options_and_the_signs_that_the_step_reads() True, True, ) + assert "`-e`" in prompt + assert "then `--` before the relative paths" in prompt + assert "A literal after `-e` must not start with `-`." in prompt def test_a_negative_search_in_a_verification_line_is_a_command_of_the_group_and_the_step_runs_it( @@ -272,3 +274,64 @@ def test_a_negative_search_in_a_verification_line_is_a_command_of_the_group_and_ assert group.verification_commands == (GONE,) assert commands_failure(checkout, group.verification_commands) is None + + +TWO_ABSENT = "! git grep -n -e absent_name -e other_absent -- src/ tests/" +HELLO_OR_ABSENT = "! git grep -n -e hello -e absent_name -- src/ tests/" +E_NO_LITERAL = "the option `-e` has no literal after it" + + +def _e_dash(word: str) -> str: + return f"the text `{word}` starts with `-`, so it is not an `-e` literal" + + +def _e_forms(word: str) -> str: + return f"the word `{word}` is not allowed with `-e`, so use one literal, or `-e` literals and then `--`" + + +def test_a_search_for_two_names_passes_when_neither_name_occurs( + checkout: Path, programs: ProcessFake +) -> None: + assert commands_failure(checkout, [TWO_ABSENT]) is None + assert programs_run(programs, "git") == [ + "git grep -n -e absent_name -e other_absent -- src/ tests/" + ] + + +def test_a_search_for_two_names_stops_with_the_matching_lines( + checkout: Path, programs: ProcessFake +) -> None: + assert commands_failure(checkout, [HELLO_OR_ABSENT]) == ( + f"{HELLO_OR_ABSENT} found a match, so the name is not gone.\n" + "src/sample.py:4:def hello(name: str) -> str:\n" + 'src/sample.py:6: return f"hello {name}"' + ) + + +@pytest.mark.parametrize( + ("command", "reason"), + [ + ("! git grep -n -e 'a|b' -- src/", _literal("a|b")), + ("! git grep -n -e 'a;b' -- src/", _literal("a;b")), + ("! git grep -n -e '$(id)' -- src/", _literal("$(id)")), + ("! git grep -n -e -E -- src/", _e_dash("-E")), + ("! git grep -n -e -P -- src/", _e_dash("-P")), + ("! git grep -n -e --addons -- src/", _e_dash("--addons")), + ("! git grep -n -e -- src/", E_NO_LITERAL), + ("! git grep -n -e name -e -- src/", E_NO_LITERAL), + ("! git grep -e", E_NO_LITERAL), + ("! git grep -e name", "it gives no path"), + ("! git grep -e name literal -- src/", _e_forms("literal")), + ("! git grep -e name src/", _e_forms("src/")), + ("! git grep -n -e name -n -- src/", _e_forms("-n")), + ("! git grep -E -e name -- src/", f"the option `-E` is not allowed {OPTIONS}"), + ("! git grep -n -- -e name src/", f"the option `--` is not allowed {OPTIONS}"), + ], +) +def test_an_e_search_outside_the_form_is_refused_and_starts_no_process( + checkout: Path, programs: ProcessFake, command: str, reason: str +) -> None: + assert commands_failure(checkout, [command]) == ( + f"The verification command {command} is refused: {reason}." + ) + assert programs_run(programs, "git") == [] diff --git a/tests/roadmap/test_roadmap_implement_shell_policy.py b/tests/roadmap/test_roadmap_implement_shell_policy.py index 91ab2e3c7..172936956 100644 --- a/tests/roadmap/test_roadmap_implement_shell_policy.py +++ b/tests/roadmap/test_roadmap_implement_shell_policy.py @@ -340,7 +340,7 @@ def test_the_options_of_a_negative_search_are_these_letters() -> None: def test_each_option_of_a_negative_search_is_read(letter: str) -> None: found = read_negative_search(["!", "git", "grep", f"-{letter}", "name", "src/"]) - assert found == NegativeSearch((f"-{letter}",), "name", ("src/",)) + assert found == NegativeSearch((f"-{letter}",), ("name",), ("src/",)) def test_a_negative_search_is_read_into_its_options_literal_and_paths() -> None: @@ -348,11 +348,31 @@ def test_a_negative_search_is_read_into_its_options_literal_and_paths() -> None: ["!", "git", "grep", "-nI", "-w", "old name", "--", "src/", "tests/*.py"] ) - assert found == NegativeSearch(("-nI", "-w"), "old name", ("src/", "tests/*.py")) + assert found == NegativeSearch(("-nI", "-w"), ("old name",), ("src/", "tests/*.py")) assert isinstance(found, NegativeSearch) assert found.argv == ["git", "grep", "-nI", "-w", "old name", "--", "src/", "tests/*.py"] +def test_a_negative_search_with_e_options_is_read_into_one_literal_for_each() -> None: + found = read_negative_search( + ["!", "git", "grep", "-n", "-e", "old", "-e", "new name", "--", "src/"] + ) + + assert found == NegativeSearch(("-n",), ("old", "new name"), ("src/",), by_e=True) + assert isinstance(found, NegativeSearch) + assert found.argv == [ + "git", + "grep", + "-n", + "-e", + "old", + "-e", + "new name", + "--", + "src/", + ] + + def test_the_paths_of_the_run_follow_a_mark_also_when_the_command_has_none() -> None: found = read_negative_search(["!", "git", "grep", "name", "src/"]) diff --git a/tests/roadmap/test_roadmap_stage_form.py b/tests/roadmap/test_roadmap_stage_form.py index 83a16d736..de5d7c3ea 100644 --- a/tests/roadmap/test_roadmap_stage_form.py +++ b/tests/roadmap/test_roadmap_stage_form.py @@ -256,7 +256,7 @@ def test_negative_search_parts_reads_the_literal_and_the_paths() -> None: found = negative_search_parts(DOTTED) assert found is not None - assert (found.literal, found.paths) == ("devops_cli.roadmap.stage_", ("src/", "tests/")) + assert (found.literals, found.paths) == (("devops_cli.roadmap.stage_",), ("src/", "tests/")) @pytest.mark.parametrize("command", ["uv run devops ci", "! git grep -n", "!"]) @@ -311,6 +311,36 @@ def test_search_problems_records_no_hit_only_when_the_caller_gives_evidence() -> assert search_problems(groups) == [] +E_TWO = ( + "! git grep -n -e devops_cli.roadmap.stage_ -e devops_cli.roadmap.stage_missing -- src/ tests/" +) + + +def test_each_e_literal_has_its_own_hit_check() -> None: + evidence = {"devops_cli.roadmap.stage_": ("src/a.py",), "devops_cli.roadmap.stage_missing": ()} + + (problem,) = search_problems([_search_group(E_TWO)], evidence) + + assert "already passes before the change and proves nothing" in problem + assert "devops_cli.roadmap.stage_missing" in problem + + +def test_record_search_evidence_records_each_e_literal() -> None: + seen: list[str] = [] + + def search(literal: str) -> tuple[str, ...]: + seen.append(literal) + return ("src/a.py",) if literal == "devops_cli.roadmap.stage_" else () + + evidence = record_search_evidence([_search_group(E_TWO)], search) + + assert seen == ["devops_cli.roadmap.stage_", "devops_cli.roadmap.stage_missing"] + assert evidence == { + "devops_cli.roadmap.stage_": ("src/a.py",), + "devops_cli.roadmap.stage_missing": (), + } + + # ── The examples of the prompt and of the rules ─────────────────────────────── _TEST_PATH = re.compile(r"tests/[\w/<>-]*test_[\w<>-]*\.py") diff --git a/tests/roadmap/test_roadmap_stage_plan.py b/tests/roadmap/test_roadmap_stage_plan.py index 9aaa7040a..1be5800d3 100644 --- a/tests/roadmap/test_roadmap_stage_plan.py +++ b/tests/roadmap/test_roadmap_stage_plan.py @@ -1889,6 +1889,28 @@ def test_check_proposal_accepts_a_recorded_hit_that_survives_the_change() -> Non assert _search_problems(_move_plan(DOTTED), {"devops_cli.roadmap.stage_": (STAGE_OLD,)}) == [] +E_MOVE = ( + "! git grep -n -e devops_cli.roadmap.stage_ready -e devops_cli.roadmap.stage -- src/ tests/" +) + + +def test_a_move_group_checks_each_e_literal() -> None: + evidence = { + "devops_cli.roadmap.stage_ready": (STAGE_OLD,), + "devops_cli.roadmap.stage": (STAGE_OLD,), + } + + assert _search_problems(_move_plan(E_MOVE), evidence) == [ + f"Group 1 gives the `verify` command `{E_MOVE}` for the moved prefix `stage`, and its " + "literal `devops_cli.roadmap.stage` names one module of the moved prefix. Search the " + "dotted prefix, such as `devops_cli.roadmap.stage_`, so the check covers every module.", + f"Group 1 gives the `verify` command `{E_MOVE}` with the literal " + f"`devops_cli.roadmap.stage`, and the plan creates `{STAGE_PACKAGE}`, whose import path " + "`devops_cli.roadmap.stage.ready` contains the literal. The search can never pass on the " + "produced tree. Search a name that the plan does not keep.", + ] + + # ── The requirement that the base spec already has (#3013) ──────────────────── DUPLICATE_PROBLEM = (