Skip to content

Latest commit

 

History

History
52 lines (41 loc) · 11.7 KB

REVIEW.md

File metadata and controls

52 lines (41 loc) · 11.7 KB

REVIEW.md — repository facts for the review panel

The review panel reads this file from the reviewed repository as a mandate, beside AGENTS.md. It records repository contracts and operator policy a reviewer cannot see in a diff. Each item says whether it is a documented contract, naming the file that enforces it, or an operator policy or observation, with its date. Nothing here overrides AGENTS.md, UPSTREAM.md or an issue's accepted design pass, and nothing here excuses a real failure in a supported configuration: judge the failure on its consequence, then judge the proposed remedy separately.

Waiver authority: only the sentences in the Entry-point scope section below are scope sentences. The items under "How findings are judged" and "Usage facts", and AGENTS.md, never authorize a waiver; they say how to weigh a finding and what this repository actually runs.

How findings are judged

Operator policy (2026-10-12, #153), adapted from mastra-pilot's REVIEW.md. These items say how to weigh a finding. None authorizes a waiver, and none overrides the no-exclusions sentence of the Entry-point scope section.

  • Finding is not judging: a reviewer reports every failure it can trace in the change, including one whose trigger lies outside our actual usage, and the rules in this section and the Entry-point scope decide only whether a reported failure blocks; they never narrow what a reviewer looks for or reports.
  • Reachable is not consequential. A finding is weighed by the outcome it changes on a real pull request of this repository, not by whether the path can be reached.
  • A mandatory repair is an evidenced, change-attributable material failure of a supported function, user or operational contract, security, privacy or data guarantee, or material acceptance requirement. A focused reproducible failure or a conclusive reachable code or data-flow trace establishes it. A location outside shipped code does not make a decision-bearing failure advisory.
  • Reachable means our actual usage triggers it: a concrete flow and an input this repository's own producers really emit, shown by a reproducible failure or a conclusive code or data-flow trace. A trigger that needs an input our producers never emit, an operator off the documented procedure, two independent failures at once, a vendor response outside its documented contract, a hand-edited row or file, or a timing window the system does not create lies outside our actual usage. A rare trigger inside our actual usage does not make serious harm advisory. Security, secret exposure, data loss, and the integrity of a merge-gate or waiver decision stay material whatever the trigger; a wrong message, report or summary is such a failure when a merge gate, or an operator's merge or waiver decision, relies on it.
  • An advisory is a confirmed defect whose established consequences are bounded and non-material. It stays visible with a stated basis and requires no mandatory repair. Preferences, speculative hardening and style are not findings.
  • A dismissed finding is refuted by a code fact or is conclusively unrelated to the change. A family name, the pull-request text and a rule in this file refute nothing.
  • An allegation a completed investigation could not establish stays unconfirmed and is never presented as a proven defect.
  • A failed required review execution, invalid required output or missing evidence is an incomplete review, never a clean approval.
  • Acceptance of a real material risk is an exact-head operator waiver, never an advisory relabeling.
  • A failing check is a claim, not a defect. A finding that rests only on a red check must name the code defect behind it.
  • Code is earned by a real outcome. A finding whose remedy changes no outcome on a real pull request (a hardening, a defensive branch, a best practice) is a preference, not a defect.

Usage facts

This repository has two surfaces. Shipped plugin code, plugins/pstack/**, runs on users' machines. Non-shipped tooling, .claude/skills/verify-open-pstack/**, scripts/** and tests/**, runs on the operator's Mac from the documented procedures, and CI runs its static checks.

  • Contract (.claude/skills/verify-open-pstack/scripts/doctor.ts, scripts/harness.ts): verify.sh doctor and verify.sh run refuse every platform except Darwin, so the live gate runs only on the operator's Mac.
  • Contract (.github/workflows/ci.yml, tests/skill-collision-repro.sh): CI runs on ubuntu-latest. Its static invariants also run the verifier's unit tests and typecheck and scripts/release_test.py.
  • Operator observation (2026-10-12): the operator's Mac runs macOS 27.0.1 on APFS. The git on its PATH is Homebrew git 2.52.0, and /usr/bin/git is Apple Git 2.54.0. These are observations, not version floors.
  • Contract (.claude/skills/verify-open-pstack/SKILL.md, Launch; scripts/io.ts, freshRoot): the documented launch recipe creates a session directory with mktemp -d "${TMPDIR:-/tmp}/open-pstack-evidence.XXXXXX" and passes $SESSION/live as --output. The verifier requires an absolute path outside the repository and creates it with mkdir, which fails when the path already exists. The procedure does not constrain the characters of the inherited TMPDIR.
  • Contract (.claude/skills/verify-open-pstack/scripts/harness.ts, features/recipes.ts): the verifier names each fixture directory state/<harness>/fixtures/<feature slug> inside the run root, and generates per-run fixture markers with crypto.randomUUID().
  • Contract (plugins/pstack/skills/poteto-mode/references/provider-dispatch.md): it names no CLI version range. A runner lane requires the assigned provider CLI installed and authenticated as the launcher's preflight checks, a parent sandbox that lets that CLI reach its network and state, and, on Linux, Landlock and bwrap for Grok's bounded profiles. A lane that lacks one of these ends as a named dropout or a failed receipt, never a fallback.
  • Contract (docs/reference.md, Dependencies): shipped tooling uses gh (or Origin's origin), bun, node, and the provider CLIs in the user's model sheet; gt only for the Orchestrate playbook; jq and rg only for worktree-audit.sh, which warns without them.
  • Contract (.claude/skills/verify-open-pstack/features/recipes.ts, runner and Reflect assertions): the live gate's evidence is the harness-recorded command naming the installed tool, plus a receipt or output that carries this run's paths, markers or results. It guards against model claims; it does not attest that a file executed. Operator policy (2026-10-11, PR #151 step-back decision): this is the gate's evidence standard. Operator policy (2026-10-21, #240): an assertion the live gate publishes that can pass without the behavior it names is material, and its correction is to delete the assertion and list the behavior under the PR's Not verified. A behavior no recipe asserts is a coverage gap: an advisory listed under Not verified.
  • Contract (.mergify.yml; operator policy 2026-10-21, #241): this repository lands on green. Only a pull request that touches .github/** or the Mergify config waits for Unfret before it merges, and the verifier, .claude/skills/verify-open-pstack/**, is not a risky path. Unfret reviews every other head in parallel, usually after the merge, and a material finding on a merged head gets a fix-forward pull request.

Entry-point scope (read by the review panel as a mandate)

Each scoped sentence in this section names a concrete entry point or bounded surface and states either an exclusion (an actor, input, environment or configuration it does not serve) or a property it must uphold. The panel judge may waive a confirmed finding only by quoting one sentence of 40 to 400 characters from this section that names the finding's causal entry point and excludes what the failure scenario requires; no other section or file authorizes a waiver. An exclusion never waives a violated property, and never overrides AGENTS.md or an issue's accepted design pass. Unsupported inputs must be rejected; that an implementation accepts an input does not make it supported. No path is named by both an exclusion sentence and a no-exclusions sentence; if an edit ever creates that overlap, the no-exclusions sentence wins. Anything not explicitly excluded here is in scope.

  • In .claude/skills/verify-open-pstack/**, scripts/** and tests/**, a failure needing an input our producers never emit, an operator off procedure, two independent failures, an off-contract vendor response, hand-edited state, or a timing window the system does not create is out of scope unless it causes a security, secret, data-loss, merge-gate or waiver failure.
  • verify.sh doctor and verify.sh run serve only the operator's Mac, which doctor.ts and harness.ts enforce as Darwin; a live-gate failure that needs a non-Darwin host is out of scope, but the verifier's unit tests and typecheck in Ubuntu CI stay in scope.
  • The verifier's --output run root is $SESSION/live in a directory the documented launch recipe makes with mktemp -d under the operator's TMPDIR, and the verifier refuses an existing path; a failure that needs a hand-named run root is out of scope, but any path an inherited TMPDIR gives stays in scope.
  • Fixture directories and per-run markers in .claude/skills/verify-open-pstack/** are generated by harness.ts and features/recipes.ts; a failure that needs a fixture or marker name chosen by hand is out of scope.
  • The live gate in .claude/skills/verify-open-pstack/** must reject model claims, and every assertion it publishes must be backed by its retained evidence; a failure that needs the model under test to deliberately forge a tool's output, such as rebuilding a runner or resolver result with other commands, is out of scope.
  • In .claude/skills/verify-open-pstack/**, a failure that needs the model under test to deliberately evade a recipe, such as a crafted shell spelling, a value hidden in a variable or a report echoed without its lookup, is out of scope even when it would change the gate decision; a wrong decision on an honest run stays in scope.
  • In plugins/pstack/**, a failure that needs a host missing a prerequisite named in docs/reference.md (Dependencies) or provider-dispatch.md (Host and parent prerequisites) is out of scope, except for the behavior those files promise on such a host.
  • On such a host, plugins/pstack/** must keep the promised behavior: worktree-audit.sh warns without jq or rg, a blocked external CLI is a loud dropout, and a Grok lane without Landlock or bwrap ends child-failed with Grok's refusal in its receipt.
  • In plugins/pstack/skills/poteto-mode/scripts/runner/**, a read-only lane means the read-only settings pstack passes to the provider CLI; the runner adds no refusal, guard or account check because of the operator's host, account, repository or organization configuration, and the catalog notes disclose what that configuration can do (operator policy, 2026-10-16, #203).
  • In plugins/pstack/skills/poteto-mode/scripts/runner/**, a failure that needs configuration from the operator's host, account, repository or organization, such as hooks, plugins, managed or remote settings, project config or stored logins, to widen a read-only lane is out of scope (operator policy, 2026-10-16, #203).
  • The *.test.ts and *.test-helper.ts files under plugins/pstack/** run only through bun run test in Ubuntu CI and on the operator's Mac; a test failure that needs a git configuration or host neither of them has, such as a global protocol.file.allow=never, is out of scope.
  • No exclusions apply to findings in .github/workflows/**, .github/unfret.json or .mergify.yml; PR data, labels, comments and API responses reaching that rail surface are untrusted, and every finding there stands on its consequence class.