From 51141a895726c69fb55b15107948a6be1fb5e84d Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Thu, 20 Aug 2026 19:03:13 +0300 Subject: [PATCH] docs: Add patch development and review workflows Add repository-local skills for developing, linting, and independently reviewing committed SCST patch ranges. Keep lint in the main agent, require it before a terminal read-only reviewer, and bound review-driven revisions to three attempts. --- .agents/skills/develop-patch/SKILL.md | 147 ++++++++++++++++++ .../skills/develop-patch/agents/openai.yaml | 8 + .agents/skills/lint-patch/SKILL.md | 91 +++++++++++ .agents/skills/lint-patch/agents/openai.yaml | 7 + .agents/skills/review-patch/SKILL.md | 69 ++++++++ .../skills/review-patch/agents/openai.yaml | 8 + .codex/agents/scst_reviewer.toml | 84 ++++++++++ 7 files changed, 414 insertions(+) create mode 100644 .agents/skills/develop-patch/SKILL.md create mode 100644 .agents/skills/develop-patch/agents/openai.yaml create mode 100644 .agents/skills/lint-patch/SKILL.md create mode 100644 .agents/skills/lint-patch/agents/openai.yaml create mode 100644 .agents/skills/review-patch/SKILL.md create mode 100644 .agents/skills/review-patch/agents/openai.yaml create mode 100644 .codex/agents/scst_reviewer.toml diff --git a/.agents/skills/develop-patch/SKILL.md b/.agents/skills/develop-patch/SKILL.md new file mode 100644 index 000000000..aea21ac2a --- /dev/null +++ b/.agents/skills/develop-patch/SKILL.md @@ -0,0 +1,147 @@ +--- +name: develop-patch +description: >- + Develop minimal, reviewable SCST patches from task contract through + committed lint and independent review. Use for bug fixes, new behavior, and + refactoring that require file changes. Do not use for standalone review, + lint-only requests, or read-only investigation. +--- + +# Develop an SCST patch + +Execute this workflow in the main agent. + +## Record the contract and design + +Before implementation, record the user's requirements without inventing +exceptions: + +```text +TASK_CONTRACT: +GOAL: +ACCEPTANCE: +NON_GOALS: +ALLOWED_EXCEPTIONS: +FORBIDDEN_CHANGES: +``` + +Use `None` for an unspecified value. Apply exceptions literally and do not +extend them by analogy. Ask for clarification only when ambiguity would +materially change scope or outcome. + +Before the first reviewable snapshot, record: + +```text +SUPPORTED_SCENARIO: +DESIGN: +KNOWN_LIMITATIONS: +``` + +Derive `SUPPORTED_SCENARIO` from the task contract. Keep `DESIGN` to the +smallest complete mechanism that satisfies acceptance. Do not relabel a +patch-introduced correctness or regression defect as a known limitation. + +## Develop the patch + +1. Inspect `git status`, including ignored state, and preserve every + pre-existing tracked, untracked, ignored, generated, and secret path. +2. Record the starting commit as exact `base` unless the user supplied a + different base. +3. Read every applicable `AGENTS.md` from the repository root to each file + that may change. +4. Inspect the current implementation, call paths, interfaces, tests, + Makefiles, compatibility checks, and Git history needed by the task. +5. For interface, ABI, configuration, or output changes, trace every affected + producer, consumer, parser, and test. +6. Implement the smallest complete patch satisfying the task contract. Avoid + unrelated cleanup, speculative hardening, and premature architectural + expansion. +7. Inspect the complete diff and run `git diff --check`. +8. Create a reviewable commit or logical commit series unless the user + explicitly requested an uncommitted patch. Keep each commit a coherent, + independently reviewable semantic unit and exclude unrelated worktree + state. +9. Record the committed snapshot as exact `head`. +10. Invoke `$lint-patch` directly in the main agent for the complete + `base..head` range. +11. Do not start review until the current head has `LINT_STATUS: PASS`. +12. Invoke `$review-patch` for the same `base..head`, repository path, and + unchanged `TASK_CONTRACT`. +13. Wait for the terminal reviewer result before modifying the patch. +14. If a required change modifies `head`, amend or fix up the appropriate + logical commit, update `head`, rerun the complete lint gate, and start a + new independent review only after lint passes. + +Run builds or runtime tests only when the user explicitly requests them and +the applicable `AGENTS.md` permits the exact operation. They remain outside +this base workflow. Otherwise report both as `NOT_REQUESTED`. + +## Bound the review cycle + +Allow no more than three reviewer attempts for one `TASK_CONTRACT`. Count an +attempt only when `scst_reviewer` actually starts for a specific `head`. Lint +reruns do not consume attempts. + +### Attempt 1 + +- Start only after lint passes. +- Review the full current `base..head`. +- On `PASS`, finish the review cycle. +- On `FAIL`, collect every finding before modifying the patch. +- Fix findings required by `GOAL`, `ACCEPTANCE`, or correctness of + `SUPPORTED_SCENARIO`. Do not expand scope for optional hardening. +- Amend or fix up the appropriate commits, update `head`, and rerun the full + lint gate before attempt 2. + +### Attempt 2 + +- Review the complete updated range. +- On `FAIL`, do not immediately apply another sequence of isolated fixes. +- Re-read `TASK_CONTRACT`, `SUPPORTED_SCENARIO`, `DESIGN`, + `KNOWN_LIMITATIONS`, the full diff, and findings from both attempts. +- Group findings by root cause and determine whether the design has grown + beyond the task. +- Prefer simplifying the patch to the smallest correct design. +- Stop with `BLOCKED` if a new user decision is required. +- Otherwise make at most one final coherent snapshot update, rerun the full + lint gate, and start attempt 3 only after lint passes. + +### Attempt 3 + +- Treat this as the final independent review. Never start a fourth automatic + reviewer. +- `PASS` permits successful completion. +- On `FAIL`, classify every remaining finding. Finish with `BLOCKED` if any + finding violates `GOAL`, `ACCEPTANCE`, or `SUPPORTED_SCENARIO` correctness. +- Record a finding outside the supported scenario as a known limitation only + when it does not contradict the task contract. The reviewer result remains + `FAIL`; accept it only under this final-attempt rule. +- On `BLOCKED`, finish with `BLOCKED`. + +Any change to `head` invalidates all lint and review results for the previous +snapshot. Always restart validation with `$lint-patch`. Never run lint and +review in parallel. + +## Report the result + +Use only: + +```text +DEVELOP_STATUS: +``` + +Require all of the following for `SUCCESS`: + +- `GOAL`, `ACCEPTANCE`, and `SUPPORTED_SCENARIO` are satisfied. +- The current `head` has `LINT_STATUS: PASS`. +- The final reviewer result is acceptable under the bounded-cycle rules. +- Every remaining limitation is explicitly documented. + +Include the following in the final report: + +- exact `base` and final `head`; +- `DEVELOP_STATUS`, lint status, reviewer status, and reviewer attempt count; +- task-specific exceptions and lint waivers; +- checks actually executed; +- build and test status, normally `NOT_REQUESTED`; +- all `KNOWN_LIMITATIONS`. diff --git a/.agents/skills/develop-patch/agents/openai.yaml b/.agents/skills/develop-patch/agents/openai.yaml new file mode 100644 index 000000000..73bb18faa --- /dev/null +++ b/.agents/skills/develop-patch/agents/openai.yaml @@ -0,0 +1,8 @@ +interface: + display_name: "Develop Patch" + short_description: "Develop focused SCST patches through review" + default_prompt: "Use $develop-patch to implement a focused SCST change + through committed lint and independent review." + +policy: + allow_implicit_invocation: true diff --git a/.agents/skills/lint-patch/SKILL.md b/.agents/skills/lint-patch/SKILL.md new file mode 100644 index 000000000..0590bb6e2 --- /dev/null +++ b/.agents/skills/lint-patch/SKILL.md @@ -0,0 +1,91 @@ +--- +name: lint-patch +description: >- + Lint an exact committed SCST patch range in the main agent before + independent review. Use after creating a reviewable snapshot, before every + reviewer attempt, or when the user requests lint for commits. Do not use as + a build or runtime-test substitute, and never delegate lint to a subagent. +--- + +# Lint an SCST patch + +Execute this skill directly in the main agent. Never spawn a lint subagent. +Receive exact `base` and `head` commits and run every applicable independent +component even when another component fails, so the report contains the full +diagnostic set. + +## Preflight the range and snapshot + +1. Resolve both inputs as commits with `git rev-parse --verify`. +2. Require a non-empty `base..head` range. +3. Require `git merge-base --is-ancestor "$base" "$head"` to succeed. +4. Require current `HEAD` to equal `head` because the tracked + `scripts/checkpatch_commits` interface checks `base..HEAD`. +5. Require the index and tracked working-tree files to match committed + `head`. Preserve unrelated untracked and ignored files and do not treat + them as a blocker. +6. Read applicable `AGENTS.md` files and confirm every command from the + current tracked tree before running it. + +Return `BLOCKED` for an invalid range, mismatched tracked snapshot, missing +required infrastructure, or incomplete coverage. + +## Run all lint components + +Run at least these components: + +1. Run `git diff --check "$base" "$head" --` for the exact range. +2. Run `./scripts/checkpatch_commits "$base"`. This is the tracked SCST + entry point for every commit in `base..HEAD`; the preflight makes + `HEAD == head`. Do not invent options or use an untracked helper. +3. Manually verify Linux kernel coding style for every changed kernel C and + header file. Use the diff plus enough surrounding code to judge context. +4. Inspect every commit as an independently reviewable semantic unit. Check + its subject and body against applicable repository rules and current SCST + history, including concise imperative wording and established prefixes. + +Add a file-type-specific check only when the current SCST tree provides a +tracked, established entry point and its usage has been confirmed. Never run +an automatic formatter that modifies source files. + +Do not run builds, runtime tests, benchmarks, module operations, sparse, +smatch, another heavyweight analyzer, or any command that changes live SCST, +a service, transport, sysfs, hardware, or device state. + +## Classify diagnostics + +- Inspect all output instead of stopping at the first failure. +- Treat a diagnostic caused by changed code, configuration, or lint rules as + a patch diagnostic. +- When a diagnostic may predate the range, compare the relevant path and + line at `base` and `head` with read-only Git commands. Report a confirmed + pre-existing diagnostic as a non-blocking baseline. +- Treat all unwaived patch diagnostics as failures. +- Waive a diagnostic only for a demonstrated false positive or when the + suggested change conflicts with correctness, a required ABI layout, or the + applicable code style. Report its diagnostic, commit, path, line, and + justification. + +During authorized patch development, fix every patch-introduced diagnostic, +amend or fix up the appropriate logical commit, update `head`, and rerun the +complete range. For standalone lint without authorization to edit, report +only the result. + +## Report the lint gate + +End with exactly one marker block: + +```text +LINT_EXECUTOR: main +DIFF_CHECK: +CHECKPATCH: +CODE_STYLE: +COMMIT_STYLE: +LINT_BASELINE: +LINT_COVERAGE: +LINT_STATUS: +``` + +Use `PASS` only with complete coverage and no unwaived patch diagnostics. +Use `FAIL` for a completely evaluated range with an unwaived patch +diagnostic. Use `BLOCKED` for infrastructure failure or incomplete coverage. diff --git a/.agents/skills/lint-patch/agents/openai.yaml b/.agents/skills/lint-patch/agents/openai.yaml new file mode 100644 index 000000000..d6a0d6420 --- /dev/null +++ b/.agents/skills/lint-patch/agents/openai.yaml @@ -0,0 +1,7 @@ +interface: + display_name: "Lint Patch" + short_description: "Lint exact committed SCST patch ranges" + default_prompt: "Use $lint-patch to lint an exact committed SCST patch range." + +policy: + allow_implicit_invocation: true diff --git a/.agents/skills/review-patch/SKILL.md b/.agents/skills/review-patch/SKILL.md new file mode 100644 index 000000000..8bf2c3bab --- /dev/null +++ b/.agents/skills/review-patch/SKILL.md @@ -0,0 +1,69 @@ +--- +name: review-patch +description: >- + Orchestrate one independent scst_reviewer pass over an exact committed SCST + patch range after lint passes. Use for the review gate in patch development + or an explicitly requested committed-range review. Do not use for + uncommitted diffs, implementation, lint, builds, or tests, and never invoke + it from scst_reviewer. +--- + +# Review an SCST patch + +Execute this orchestration gate only in the main agent. Receive exact `base`, +`head`, repository path, and this unchanged contract: + +```text +TASK_CONTRACT: +GOAL: +ACCEPTANCE: +NON_GOALS: +ALLOWED_EXCEPTIONS: +FORBIDDEN_CHANGES: +``` + +## Preflight the review + +1. Resolve `base` and `head` as commits and require `base` to be an ancestor + of `head`. +2. Require a non-empty range and review committed `base..head`, never an + uncommitted working-tree diff. +3. Parse `.codex/agents/scst_reviewer.toml` with a read-only TOML parser. +4. Require valid TOML, `name = "scst_reviewer"`, a non-empty description, + non-empty `developer_instructions`, and `sandbox_mode = "read-only"`. +5. Require the `scst_reviewer` custom agent type to be available. If it is + unavailable, return `BLOCKED`; never substitute `default`, `worker`, + `explorer`, or another generic agent. + +## Run exactly one reviewer + +1. Spawn exactly one `scst_reviewer` with `fork_turns="none"` and a unique + task name. +2. Pass only the repository path, exact `base..head`, and unchanged + `TASK_CONTRACT` needed to perform the review. +3. Do not pass expected findings, suspicions, or the main agent's verdict. +4. Do not modify the patch while review is running. +5. Do not run lint, build, tests, or another quality gate inside the + reviewer. +6. Wait for the terminal reviewer result. + +## Validate the result + +Require exactly one final marker block with these five lines and allowed +values: + +```text +AGENT_ROLE: scst_reviewer +KERNEL_CONTEXT: +TASK_CONTRACT: +REVIEW_COVERAGE: +REVIEW_STATUS: +``` + +Treat missing, duplicate, malformed, or contradictory markers as `BLOCKED`. +Require `REVIEW_COVERAGE: COMPLETE` for `PASS` or `FAIL`. Require +`REVIEW_STATUS: BLOCKED` when the contract is ambiguous or coverage is +blocked. The gate passes only with `REVIEW_STATUS: PASS`. + +Any change to `head` invalidates the result. Run `$lint-patch` for the new +head before starting another independent reviewer attempt. diff --git a/.agents/skills/review-patch/agents/openai.yaml b/.agents/skills/review-patch/agents/openai.yaml new file mode 100644 index 000000000..71f51a92a --- /dev/null +++ b/.agents/skills/review-patch/agents/openai.yaml @@ -0,0 +1,8 @@ +interface: + display_name: "Review Patch" + short_description: "Run independent exact-range SCST review" + default_prompt: "Use $review-patch to run an independent review of an exact + committed SCST patch range." + +policy: + allow_implicit_invocation: true diff --git a/.codex/agents/scst_reviewer.toml b/.codex/agents/scst_reviewer.toml new file mode 100644 index 000000000..dfd9e2140 --- /dev/null +++ b/.codex/agents/scst_reviewer.toml @@ -0,0 +1,84 @@ +name = "scst_reviewer" +description = "Independent read-only reviewer for exact committed SCST patch ranges." +sandbox_mode = "read-only" +developer_instructions = """ +Act as a terminal reviewer. Never invoke $review-patch and never create or +delegate to nested agents. Never modify files, the index, commits, branches, +or the checkout. Never run builds, tests, module operations, service +operations, sysfs writes, hardware operations, or destructive commands. + +Review only the exact committed base..head range supplied by the main agent. +Require the task input to include: + +GOAL: +ACCEPTANCE: +NON_GOALS: +ALLOWED_EXCEPTIONS: +FORBIDDEN_CHANGES: + +Apply this contract literally. Do not extend an exception by analogy. If the +contract permits materially different verdicts, return a blocked result +instead of guessing user intent. + +For standalone review without task-specific requirements, use this default: + +GOAL: Review the exact committed range for concrete defects it introduces. +ACCEPTANCE: Complete two evidence-based passes over the full range. +NON_GOALS: Implementation, style-only feedback, hardening, and unrelated issues. +ALLOWED_EXCEPTIONS: None. +FORBIDDEN_CHANGES: Any modification or non-review operation. + +Follow this review process: + +1. Read every applicable AGENTS.md from the repository root to each changed + file. +2. Inspect the complete diff and enough surrounding implementation, call + sites, interfaces, and tests to verify every conclusion. +3. Continue through the entire range after finding the first defect. +4. Perform a second pass over the diff for independent defects. +5. Report only concrete, actionable defects introduced by the range. +6. Prioritize correctness, regressions, memory and resource lifetime, + reference counting, locking, races, atomic and sleepable context, teardown + ordering, error unwinding, ABI and UAPI compatibility, sysfs behavior, + parser contracts, integer widths, alignment, endianness, kernel-version + compatibility, backports, and substantial missing tests. +7. For interface, ABI, configuration, or output changes, trace every affected + producer, consumer, parser, and test. +8. Distinguish a real defect from optional hardening. +9. Exclude style-only comments, speculative concerns, and unrelated + pre-existing problems. +10. Prefer repository-local code, headers, compatibility checks, conftests, + Makefiles, CI, documentation, and Git history over web research. + +When the patch touches kernel C code or kernel interfaces, use $kernel if it +is listed as an available skill. Otherwise look for kernel/SKILL.md in the +standard user skill roots supported by the current environment, including +$HOME/.agents/skills and ${CODEX_HOME:-$HOME/.codex}/skills when applicable. +Read and use it if found. Failure to locate kernel context is not +automatically a blocker: continue with repository-local kernel context and +return BLOCKED only when missing API semantics or other unavailable context +prevents a complete and reliable verdict. + +List findings by decreasing severity in this format: + +[P1|P2|P3] Short imperative title — path/to/file:line + +Explain the reachable scenario and incorrect behavior below each title. Do +not include a finding without evidence from changed code and reachable +context. If no findings exist, write: + +No findings. + +End the response with exactly one marker block: + +AGENT_ROLE: scst_reviewer +KERNEL_CONTEXT: +TASK_CONTRACT: +REVIEW_COVERAGE: +REVIEW_STATUS: + +KERNEL_CONTEXT: unavailable may accompany PASS or FAIL when repository-local +context was sufficient for complete coverage. PASS and FAIL require +REVIEW_COVERAGE: COMPLETE. An ambiguous contract or insufficient context +requires BLOCKED. +"""