mirror of
https://github.com/SCST-project/scst.git
synced 2026-08-22 15:16:33 +00:00
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.
This commit is contained in:
@@ -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: <SUCCESS|BLOCKED>
|
||||
```
|
||||
|
||||
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`.
|
||||
@@ -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
|
||||
@@ -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: <PASS|FAIL|BLOCKED>
|
||||
CHECKPATCH: <PASS|WAIVED|FAIL|NOT_APPLICABLE|BLOCKED>
|
||||
CODE_STYLE: <PASS|FAIL|NOT_APPLICABLE|BLOCKED>
|
||||
COMMIT_STYLE: <PASS|FAIL|BLOCKED>
|
||||
LINT_BASELINE: <CLEAN|PRESENT|BLOCKED>
|
||||
LINT_COVERAGE: <COMPLETE|BLOCKED>
|
||||
LINT_STATUS: <PASS|FAIL|BLOCKED>
|
||||
```
|
||||
|
||||
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.
|
||||
@@ -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
|
||||
@@ -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: <loaded|unavailable|not-applicable>
|
||||
TASK_CONTRACT: <applied|default|ambiguous>
|
||||
REVIEW_COVERAGE: <COMPLETE|BLOCKED>
|
||||
REVIEW_STATUS: <PASS|FAIL|BLOCKED>
|
||||
```
|
||||
|
||||
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.
|
||||
@@ -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
|
||||
@@ -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: <loaded|unavailable|not-applicable>
|
||||
TASK_CONTRACT: <applied|default|ambiguous>
|
||||
REVIEW_COVERAGE: <COMPLETE|BLOCKED>
|
||||
REVIEW_STATUS: <PASS|FAIL|BLOCKED>
|
||||
|
||||
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.
|
||||
"""
|
||||
Reference in New Issue
Block a user