diff --git a/.agents/AGENTS.reference.md b/.agents/AGENTS.reference.md index 0580bd09..ba5f045a 100644 --- a/.agents/AGENTS.reference.md +++ b/.agents/AGENTS.reference.md @@ -8,7 +8,7 @@ The human keeps delivery state in `.agents/work.toml`. It contains at most five The four prose fields (`user_problem`, `change`, each `acceptance` item, and `why_next`) may hold paragraphs, so write them with a TOML multi-line string (`"""`) when one line will not state the problem honestly. The structural values stay on one line each: `selected_action`, every step `id`, every `blocked_by` id, and every status. Tabs, carriage returns, other control characters, and Unicode line or paragraph separators are rejected everywhere, in prose as well; only the line-feed paragraph break is prose-only. The `next` brief indents every continuation line, so prose cannot forge a heading of its own. -To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. +To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. To ask instead for a standalone read-only review of a whole tree at one ref, or of one diff between two refs, copy `.agents/user-prompts/review.md`. That prompt is a reference asset a human invokes by hand; it neither starts nor advances the selected action, and its review is the agent's direct response, not a file. `.agents/work.toml` is the only workflow task-state file. Do not create a plan tree, ledger, round log, findings directory, or review record. Product-development checks under `.agents/checks.toml`, `.agents/checks/`, and `.agents/hooks/` are optional tooling, not task state or proof of review. diff --git a/.agents/user-prompts/review.md b/.agents/user-prompts/review.md new file mode 100644 index 00000000..6f4739ef --- /dev/null +++ b/.agents/user-prompts/review.md @@ -0,0 +1,20 @@ +# Review prompt + +Copy this, choose one target mode, fill in the criteria, and paste it to an agent. This asks for a standalone review of code that already exists; it is not delivery state and does not start or advance the selected action. + +--- + +Give me a standalone, read-only code review. Choose exactly one target mode and say which one you used. + +- CURRENT TREE at ``: review the complete tree at that single ref. There is no baseline, so this is a whole-tree review, never a diff review and never an empty one. +- DIFF from `` to ``: review the changes in `..`, plus only the surrounding code needed to judge them. Do not widen this into a whole-tree review. + +Criteria: ``. + +Before reviewing, resolve every named ref to its full commit ID, then state the target mode, those IDs, the criteria, and whether the working tree is clean or dirty. If a ref or a criterion is missing or ambiguous, stop and ask rather than guess. + +Work read-only. Do not edit, create, format, stage, commit or delete any file in the reviewed repository; do not change its index, refs or configuration; never write a findings file, report, ledger, round log, review directory, plan tree or task state anywhere. Reproduce behaviour only in a human-authorised scratch directory outside that repository. Record `git status --porcelain` before and after reviewing; report any difference rather than a read-only review. Return the review directly in this response. + +Give each finding a severity of `low`, `medium`, `high` or `critical`, explain its impact against the stated criteria, and give reproducible evidence: an exact command with its relevant output, or `::`. Report no unsupported suspicion. An improvement outside the criteria is out of scope, not a finding. + +If nothing violates the criteria, say `No findings.` and briefly list the target, the criteria and the checks you ran. diff --git a/.agents/work.toml b/.agents/work.toml index 1ce1525f..b518f460 100644 --- a/.agents/work.toml +++ b/.agents/work.toml @@ -1,25 +1,24 @@ version = 1 [[step]] -id = "prevent-false-attribution" +id = "ship-standalone-review-prompt" status = "complete" blocked_by = [] user_problem = """ -Commits can name an agent or an LLM as the author or as a co-author, which credits work to someone who did not do it and adds a false contributor to the repository. - -A rewrite of the history fixed the record already. All 1512 commits reachable from the current main now carry the raw author nothingnesses <18732253+nothingnesses@users.noreply.github.com>, seven Co-Authored-By trailers are gone, and the repository-local Test identity override is gone. Nothing stops the next commit from undoing that work.""" +A human who wants a review of a whole tree or of one diff has to improvise the request. A whole-tree review can silently collapse into an empty diff review. +""" change = """ -Add a read-only attribution check over every commit reachable from HEAD, prove it with scratch repositories, run both from the shared just ci gate, fetch the complete history in GitHub CI, and state the rule in the shipped guidance.""" +Ship one compact standalone review prompt as a built-in pack reference asset, with the discoverability and tests it needs. +""" acceptance = [ - "just ci checks the complete reachable history, locally and in GitHub CI.", - "The gate accepts only the raw author identity nothingnesses <18732253+nothingnesses@users.noreply.github.com>.", - "Any Co-Authored-By trailer fails, whatever its case and whatever horizontal whitespace surrounds the key and the colon.", - "A report names the offending commit and the rule it broke, and prints no other commit body content.", - "Scratch repositories outside the tracked tree prove the passing case, a foreign author, a placeholder author, and a co-author trailer, under temporary Git configuration.", - "The GitHub checkout fetches the complete history, so the remote run cannot inspect only the tip, and it keeps the default merge-result ref, so a pull request still builds and tests the merge rather than the branch tip alone.", - "On a pull request the remote run scans the commits of the branch rather than the synthetic author of GitHub's merge commit.", - "The shipped guidance requires the owner identity and forbids agent and LLM attribution in commits and pull requests.", - "The gate stays read-only over the tracked tree, and the existing formatting, Clippy, test, reset tripwire, action pin, and clean-tree checks still pass.", + "A fresh default scaffold writes `.agents/user-prompts/review.md` and no other new asset.", + "The prompt makes the current-tree and diff targets exclusive and unambiguous.", + "It requires criteria, resolved refs, a clean-or-dirty status statement, read-only work and a direct response.", + "It requires severity and reproducible evidence and allows a concise clean result.", + "It forbids edits and every persisted review-state family the reset removed.", + "Shipped guidance says when to use the kickoff prompt and when to use this one.", + "Tests pin the asset, its limits and that contract.", ] why_next = """ -The history is clean today only because a human rewrote it. A mechanical gate keeps it clean without asking every later author to remember the rule, and it is cheap to add while the evidence of what went wrong is fresh.""" +The comparison already chose this surface. Shipping it closes the gap without a command, provider or new state. +""" diff --git a/AGENTS.md b/AGENTS.md index 0580bd09..ba5f045a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ The human keeps delivery state in `.agents/work.toml`. It contains at most five The four prose fields (`user_problem`, `change`, each `acceptance` item, and `why_next`) may hold paragraphs, so write them with a TOML multi-line string (`"""`) when one line will not state the problem honestly. The structural values stay on one line each: `selected_action`, every step `id`, every `blocked_by` id, and every status. Tabs, carriage returns, other control characters, and Unicode line or paragraph separators are rejected everywhere, in prose as well; only the line-feed paragraph break is prose-only. The `next` brief indents every continuation line, so prose cannot forge a heading of its own. -To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. +To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. To ask instead for a standalone read-only review of a whole tree at one ref, or of one diff between two refs, copy `.agents/user-prompts/review.md`. That prompt is a reference asset a human invokes by hand; it neither starts nor advances the selected action, and its review is the agent's direct response, not a file. `.agents/work.toml` is the only workflow task-state file. Do not create a plan tree, ledger, round log, findings directory, or review record. Product-development checks under `.agents/checks.toml`, `.agents/checks/`, and `.agents/hooks/` are optional tooling, not task state or proof of review. diff --git a/README.md b/README.md index 2726e6b0..3d586582 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,11 @@ AGENTS.md compact canonical guidance (working file) verifier.md verify that fix once user-prompts/ kickoff.md start the selected action + review.md ask for one standalone read-only review ``` +The two user prompts answer different questions. Copy `kickoff.md` to start the selected action and run the bounded delivery around it. Copy `review.md` when you only want code that already exists reviewed: a whole tree at one ref, or one diff between two refs, judged against criteria you supply. It is a human-invoked reference asset, not workflow state, so it starts no delivery, changes no file in the reviewed repository and persists no review state anywhere, confines any reproduction to a scratch directory you authorise outside that repository, and returns its review as the agent's direct response. + The default creates no ledger, JSON Lines round log, `docs/plans/` process tree, review directory, plan-review loop, or convergence-round state. `.agents/work.toml` contains at most five ordered delivery steps. While work remains, `selected_action` names one active step and several steps may be active at once; after every step is complete, the field is omitted. `agent-flow validate`, `status`, and `next` use that state by default. `AGENTS.md` is rendered from the selected principles. The root guidance and `.agents/work.toml` are working files, created only when absent unless `--force` is used. Tool-owned references under `.agents/` refresh on each run. diff --git a/pack/AGENTS.md b/pack/AGENTS.md index 5e59cccc..258f890f 100644 --- a/pack/AGENTS.md +++ b/pack/AGENTS.md @@ -8,7 +8,7 @@ The human keeps delivery state in `.agents/work.toml`. It contains at most five The four prose fields (`user_problem`, `change`, each `acceptance` item, and `why_next`) may hold paragraphs, so write them with a TOML multi-line string (`"""`) when one line will not state the problem honestly. The structural values stay on one line each: `selected_action`, every step `id`, every `blocked_by` id, and every status. Tabs, carriage returns, other control characters, and Unicode line or paragraph separators are rejected everywhere, in prose as well; only the line-feed paragraph break is prose-only. The `next` brief indents every continuation line, so prose cannot forge a heading of its own. -To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. +To start the selected action, copy `.agents/user-prompts/kickoff.md`, fill in its optional context, and paste it to the agent. To ask instead for a standalone read-only review of a whole tree at one ref, or of one diff between two refs, copy `.agents/user-prompts/review.md`. That prompt is a reference asset a human invokes by hand; it neither starts nor advances the selected action, and its review is the agent's direct response, not a file. `.agents/work.toml` is the only workflow task-state file. Do not create a plan tree, ledger, round log, findings directory, or review record. Product-development checks under `.agents/checks.toml`, `.agents/checks/`, and `.agents/hooks/` are optional tooling, not task state or proof of review. diff --git a/pack/pack.toml b/pack/pack.toml index fba7dfda..e80888a0 100644 --- a/pack/pack.toml +++ b/pack/pack.toml @@ -58,6 +58,11 @@ source = "user-prompts/kickoff.md" dest = ".agents/user-prompts/kickoff.md" ownership = "reference" +[[asset]] +source = "user-prompts/review.md" +dest = ".agents/user-prompts/review.md" +ownership = "reference" + # Optional product-development checks. No asset in this module is task state. [[asset]] source = "checks.toml" diff --git a/pack/user-prompts/review.md b/pack/user-prompts/review.md new file mode 100644 index 00000000..6f4739ef --- /dev/null +++ b/pack/user-prompts/review.md @@ -0,0 +1,20 @@ +# Review prompt + +Copy this, choose one target mode, fill in the criteria, and paste it to an agent. This asks for a standalone review of code that already exists; it is not delivery state and does not start or advance the selected action. + +--- + +Give me a standalone, read-only code review. Choose exactly one target mode and say which one you used. + +- CURRENT TREE at ``: review the complete tree at that single ref. There is no baseline, so this is a whole-tree review, never a diff review and never an empty one. +- DIFF from `` to ``: review the changes in `..`, plus only the surrounding code needed to judge them. Do not widen this into a whole-tree review. + +Criteria: ``. + +Before reviewing, resolve every named ref to its full commit ID, then state the target mode, those IDs, the criteria, and whether the working tree is clean or dirty. If a ref or a criterion is missing or ambiguous, stop and ask rather than guess. + +Work read-only. Do not edit, create, format, stage, commit or delete any file in the reviewed repository; do not change its index, refs or configuration; never write a findings file, report, ledger, round log, review directory, plan tree or task state anywhere. Reproduce behaviour only in a human-authorised scratch directory outside that repository. Record `git status --porcelain` before and after reviewing; report any difference rather than a read-only review. Return the review directly in this response. + +Give each finding a severity of `low`, `medium`, `high` or `critical`, explain its impact against the stated criteria, and give reproducible evidence: an exact command with its relevant output, or `::`. Report no unsupported suspicion. An improvement outside the criteria is out of scope, not a finding. + +If nothing violates the criteria, say `No findings.` and briefly list the target, the criteria and the checks you ran. diff --git a/src/manifest.rs b/src/manifest.rs index ed37d586..a767abd6 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -863,6 +863,7 @@ mod tests { ".agents/prompts/verifier.md", ".agents/principles.toml", ".agents/user-prompts/kickoff.md", + ".agents/user-prompts/review.md", ] ); } diff --git a/tests/default_scaffold_is_minimal.rs b/tests/default_scaffold_is_minimal.rs index 37bf2ee7..2ae9bb02 100644 --- a/tests/default_scaffold_is_minimal.rs +++ b/tests/default_scaffold_is_minimal.rs @@ -20,6 +20,31 @@ const MAX_GUIDANCE_BYTES: usize = 65_536; const MAX_PROMPT_BYTES: usize = 4_096; const MAX_WORK_BYTES: usize = 4_096; +/// The standalone review prompt is pasted by hand into whatever harness the human uses, +/// so it competes for the same context window as the code under review. The accepted +/// limit is strictly below 2,000 bytes, far under the general prompt ceiling: the surface +/// was chosen over a command because it is compact, and a prompt that grows into a +/// document stops being that. This constant is the only place that limit is enforced. +const MAX_REVIEW_PROMPT_BYTES: usize = 2_000; + +/// The four severity values the reviewer role prompt already uses. The standalone prompt +/// keeps the same scale so a finding reads the same whichever surface produced it. +const SEVERITIES: [&str; 4] = ["`low`", "`medium`", "`high`", "`critical`"]; + +/// Every persisted review-state family the reset deleted. Shipping a review surface is +/// exactly the change that could bring one back, so the prompt must name each as +/// forbidden: a reader told only "read-only" can still believe that writing up findings +/// into a file is part of reviewing rather than a violation of it. +const FORBIDDEN_REVIEW_STATE: [&str; 7] = [ + "findings file", + "report", + "ledger", + "round log", + "review directory", + "plan tree", + "task state", +]; + fn scratch(name: &str) -> PathBuf { let dir = std::env::temp_dir() .join(format!("agent-flow-minimal-scaffold-{}-{name}", std::process::id())); @@ -122,6 +147,7 @@ fn default_scaffold_is_bounded_parseable_and_byte_idempotent() { ".agents/prompts/triager.md", ".agents/prompts/verifier.md", ".agents/user-prompts/kickoff.md", + ".agents/user-prompts/review.md", ".agents/work.toml", "AGENTS.md", ]; @@ -250,3 +276,156 @@ fn the_no_review_directory_criterion_fails_for_an_empty_review_directory() { fs::remove_dir_all(root).unwrap(); } + +/// The one line of `prompt` that starts with `marker`, panicking if it is missing or +/// repeated. Anchoring each clause to its own line is what makes the assertions below a +/// contract rather than a bag of substrings: a required phrase that drifted into some +/// other sentence no longer satisfies the clause it was meant to pin, and a duplicated +/// clause (two target-mode lines saying different things) fails instead of half-passing. +fn clause<'a>( + prompt: &'a str, + marker: &str, +) -> &'a str { + let mut matched = prompt.lines().filter(|line| line.starts_with(marker)); + let line = matched + .next() + .unwrap_or_else(|| panic!("the review prompt has no line starting with {marker:?}")); + assert!( + matched.next().is_none(), + "the review prompt has more than one line starting with {marker:?}, so pinning that clause would check only the first" + ); + line +} + +/// Assert that the `name` clause states every phrase in `required`. +fn assert_states( + name: &str, + line: &str, + required: &[&str], +) { + for phrase in required { + assert!( + line.contains(phrase), + "the {name} clause of the review prompt must state {phrase:?}, but it reads {line:?}" + ); + } +} + +/// Pin the standalone review prompt's contract as the scaffold ships it. +/// +/// This asset is a human-invoked reference prompt: it is copied out of the scaffold and +/// pasted into an arbitrary harness, so no code downstream re-derives or enforces what it +/// means. Its bytes ARE the contract, which is why the meaning is asserted here and not +/// just its presence in the asset list. Each group below pins one clause the decision to +/// ship a prompt rather than a `review` command rests on. +#[test] +fn the_scaffolded_review_prompt_pins_its_standalone_contract() { + let root = scratch("review-prompt"); + let output = scaffold(&root); + assert!( + output.status.success(), + "scaffold failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let prompt = fs::read_to_string(root.join(".agents/user-prompts/review.md")).unwrap(); + assert!( + prompt.len() < MAX_REVIEW_PROMPT_BYTES, + "the review prompt is {} bytes; the compact limit is under {MAX_REVIEW_PROMPT_BYTES}", + prompt.len() + ); + assert!( + prompt.is_ascii(), + "the review prompt is pasted into unknown harnesses and terminals, so it stays ASCII-only" + ); + + // Both target modes, and the exclusivity between them. The failure this guards is + // concrete and silent: asking for a whole-tree review as if it were a diff resolves to + // an empty range, which reviews nothing and reports nothing wrong. + assert_states( + "target-mode", + clause(&prompt, "Give me a standalone"), + &["read-only", "exactly one target mode"], + ); + assert_states( + "CURRENT TREE", + clause(&prompt, "- CURRENT TREE at"), + &["complete tree", "no baseline", "never a diff review", "never an empty one"], + ); + assert_states( + "DIFF", + clause(&prompt, "- DIFF from"), + &[ + "`..`", + "only the surrounding code needed to judge them", + "Do not widen this into a whole-tree review", + ], + ); + + // Criteria and a stated starting point. Refs resolved in full and a clean-or-dirty + // statement are what make the review reproducible by someone who was not there. + assert!( + prompt.lines().any(|line| line.starts_with("Criteria:")), + "the review prompt must carry a criteria slot for the human to fill" + ); + assert_states( + "setup", + clause(&prompt, "Before reviewing,"), + &["full commit ID", "clean or dirty", "stop and ask"], + ); + + // Read-only work, a direct response, and no persisted review state of any family. + // The non-mutation ban is scoped to the reviewed repository and bounded at both ends + // by a `git status --porcelain` comparison: unscoped, it also forbids the isolated + // reproduction the evidence clause below requires, and without the closing comparison + // the permitted scratch has nothing proving it stayed outside the reviewed tree. + let read_only = clause(&prompt, "Work read-only."); + assert_states( + "read-only", + read_only, + &[ + "Do not edit", + "format", + "stage, commit or delete any file in the reviewed repository", + "index, refs or configuration", + "human-authorised scratch directory outside that repository", + "Record `git status --porcelain` before and after", + "report any difference", + "Return the review directly in this response.", + ], + ); + for family in FORBIDDEN_REVIEW_STATE { + assert!( + read_only.contains(family), + "the read-only clause must forbid writing a {family}, the review-state family the reset removed; it reads {read_only:?}" + ); + } + + // Severity and reproducible evidence, so a finding can be checked rather than believed. + // A textual citation carries the revision it was read at, which is what makes it + // resolve for the reader in either target mode: a bare `file:line` names different + // content at each end of a diff, and names nothing at all for a deleted line. + let evidence = clause(&prompt, "Give each finding a severity"); + assert_states( + "evidence", + evidence, + &["exact command", "`::`", "out of scope, not a finding"], + ); + for severity in SEVERITIES { + assert!( + evidence.contains(severity), + "the evidence clause must offer the {severity} severity; it reads {evidence:?}" + ); + } + + // A clean review must have a short, unambiguous way to say so. Without it, an agent + // with nothing to report is pushed toward padding the response with non-findings. + assert_states( + "clean-result", + clause(&prompt, "If nothing violates the criteria,"), + &["`No findings.`", "the checks you ran"], + ); + + fs::remove_dir_all(root).unwrap(); +}