Separate tutorial and Skill review gates

This commit is contained in:
LinuxDev9002
2026-07-18 20:23:00 -07:00
parent b523765944
commit a77417dbc7
6 changed files with 136 additions and 7 deletions

View File

@@ -17,7 +17,7 @@ This Skill owns the writing process, not writing style. Invoke `$bpf-tutorial-wr
- Resolve every valid **Must fix** finding. Record a concrete disposition for every **Should fix** and **Consider** finding, then run an external re-review of the final diff.
- Do not call the prose finished while any valid Must-fix remains. If no different model family is available, report `cross-model review unavailable`.
- Use `scripts/run-external-review.sh` for Grok 4.5 or OpenCode GLM 5.2 reviews. Choose one reviewer for a normal pass and at most two when the first review finds factual uncertainty or substantial bilingual problems. Do not run every available model by default. The wrapper is read-only and preserves the complete prompt, trace, and manifest under `~/.local/state/bpf-tutorial-reviews/runs/`.
- Keep `scripts/run-claude-writer.sh` only as an optional, trace-verified `claude-opus-4-6[1m]` authoring channel. It requires a clean checkout, authors inside a bubblewrap-protected isolated worktree, verifies the exact two-file diff, preserves a prompt, trace, manifest, and patch, then applies that patch to the real checkout.
- Keep `scripts/run-claude-writer.sh` only as an optional, trace-verified `claude-opus-4-6[1m]` authoring channel. It requires a clean checkout, authors inside a bubblewrap-protected isolated worktree, verifies the exact two-file diff, preserves the worktree, prompt, trace, manifest, and patch, then applies that patch to the real checkout.
- Never delete, truncate, rewrite, or clean any writer or reviewer trace directory. Preserve failed and partial runs too.
## Establish the tutorial contract
@@ -31,7 +31,7 @@ Before coding, write one sentence naming:
Reject proposals whose only result is “the program loaded,” “the helper returned success,” or “the upstream selftest passed.”
Read [references/tutorial-acceptance.md](references/tutorial-acceptance.md) before selecting scope or placement. Read [references/repository-precedents.md](references/repository-precedents.md) and the required precedent lessons in full before drafting. Read [references/drafting-process.md](references/drafting-process.md) before creating the task file. Read [references/review-checklist.md](references/review-checklist.md) before external review. Read every reference required by `$bpf-tutorial-writing-style` before drafting and again before final review.
Read [references/tutorial-acceptance.md](references/tutorial-acceptance.md) before selecting scope or placement. Read [references/repository-precedents.md](references/repository-precedents.md) and the required precedent lessons in full before drafting. Read [references/drafting-process.md](references/drafting-process.md) before creating the task file. Read [references/review-checklist.md](references/review-checklist.md) before a tutorial review and [references/skill-review-checklist.md](references/skill-review-checklist.md) before a repository-local Skill review. Read every reference required by `$bpf-tutorial-writing-style` before drafting and again before final review.
## Implement and verify

View File

@@ -0,0 +1,26 @@
# Independent repository-local Skill review checklist
Use this checklist only for `--scope skill`. Review the supplied immutable snapshot without editing files or running commands. The author and reviewer must belong to different model families. The snapshot must include both repository-local writing Skill entrypoints, every changed Skill reference or script, every changed CI hook, and any neighboring Skill or workflow needed to verify an interface claim. Fail the process as incomplete when a changed public surface or required dependency is absent.
Classify every finding as **Must fix**, **Should fix**, or **Consider**. For each finding, name the file and line or heading, quote the smallest useful fragment, explain the problem, and propose a concrete correction. Do not turn a Skill review into a tutorial prose review.
Give explicit verdicts for:
1. whether workflow ownership and writing-style ownership are separated cleanly, with correct trigger descriptions and cross-Skill routing;
2. whether the complete project-provided advanced tutorial guidelines are preserved and every optional author or reviewer receives the full applicable rulebook;
3. whether agents must read the required completed tutorial pairs and may select other finished tutorials as useful precedents rather than following a closed allowlist;
4. whether source synchronization enforces containment, complete inventory, exact accepted payloads, bilingual pairing, idempotence, and safe check/write behavior;
5. whether writer and reviewer wrappers preserve prompts, snapshots, traces, manifests, patches, failed runs, partial runs, and real agent behavior without deleting or rewriting prior records;
6. whether author isolation, path/tool restrictions, model proof, read-only review, prompt-injection boundaries, interruption handling, and final gate status match the documented contract;
7. whether unit tests and CI cover the important contracts without requiring BPF runtime on the host;
8. whether every added public surface is authorized or strictly necessary, and whether the two-Skill design is simpler than a combined or duplicated workflow;
9. whether the final Skill can guide a fresh agent from evidence through bilingual drafting, KVM verification, independent review, PR preparation, and CI closure without relying on a global Skill.
The tutorial-specific precedent snapshot requirement and nine lesson verdicts in `review-checklist.md` do not apply to `--scope skill`. A Skill review checks that those tutorial rules are encoded correctly; it does not pretend that the Skill itself is a runnable lesson.
Finish with a count by severity and exactly one final gate result:
- `GATE: PASS` when no valid Must-fix item remains.
- `GATE: FAIL` when one or more Must-fix items remain.
Model identity documents review independence. It is not evidence that the workflow is correct.

View File

@@ -426,7 +426,5 @@ git -C "$repo" apply --check "$patch_file"
git -C "$repo" apply "$patch_file"
applied=true
git -C "$repo" worktree remove --force "$author_repo"
worktree_preserved=false
run_state=complete
echo "manifest=$manifest"

View File

@@ -5,7 +5,8 @@ readonly SKILL_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
readonly PROCESS_SKILL="$SKILL_DIR/SKILL.md"
readonly ACCEPTANCE="$SKILL_DIR/references/tutorial-acceptance.md"
readonly WRITING_GUIDE="$SKILL_DIR/references/drafting-process.md"
readonly REVIEW_GUIDE="$SKILL_DIR/references/review-checklist.md"
readonly TUTORIAL_REVIEW_GUIDE="$SKILL_DIR/references/review-checklist.md"
readonly SKILL_REVIEW_GUIDE="$SKILL_DIR/references/skill-review-checklist.md"
readonly PRECEDENT_GUIDE="$SKILL_DIR/references/repository-precedents.md"
readonly VERIFY="$SKILL_DIR/scripts/verify_external_review.py"
readonly STYLE_DIR="$SKILL_DIR/../bpf-tutorial-writing-style"
@@ -75,13 +76,20 @@ declare -a policy_files=(
"$PROCESS_SKILL"
"$ACCEPTANCE"
"$WRITING_GUIDE"
"$REVIEW_GUIDE"
"$PRECEDENT_GUIDE"
"$STYLE_SKILL"
"$STYLE_GUIDELINES"
"$STYLE_REPOSITORY"
"$STYLE_PROSE"
)
case "$scope" in
tutorial)
policy_files+=("$TUTORIAL_REVIEW_GUIDE")
;;
skill)
policy_files+=("$SKILL_REVIEW_GUIDE")
;;
esac
for required_file in "${policy_files[@]}" "$VERIFY"; do
[[ -r $required_file ]] || {
echo "repository-local Skill dependency is missing: $required_file" >&2
@@ -365,7 +373,7 @@ finalize() {
run_state=failed_or_interrupted
fi
write_manifest
if ((exit_status != 0)); then
if ((exit_status != 0)) && [[ $run_state != complete ]]; then
echo "review did not complete; prompt, trace, and manifest were preserved under $run_dir" >&2
fi
}
@@ -444,3 +452,7 @@ fi
run_state=complete
echo "gate=$gate"
echo "manifest=$manifest"
if [[ $gate == FAIL ]]; then
echo "review completed with GATE: FAIL; prompt, trace, snapshot, and manifest were preserved under $run_dir" >&2
exit 3
fi

View File

@@ -0,0 +1,92 @@
#!/usr/bin/env python3
"""Integration tests for external-review gate exit behavior."""
from __future__ import annotations
import json
import os
import pathlib
import subprocess
import tempfile
import unittest
SCRIPT = pathlib.Path(__file__).with_name("run-external-review.sh")
REPO = SCRIPT.parents[4]
class RunExternalReviewTest(unittest.TestCase):
def setUp(self) -> None:
self.temporary = tempfile.TemporaryDirectory()
self.root = pathlib.Path(self.temporary.name)
binary = self.root / "bin"
binary.mkdir()
grok = binary / "grok"
grok.write_text(
"#!/usr/bin/env python3\n"
"import json, os\n"
"print(json.dumps({'type': 'text', 'data': 'Synthetic review.\\nGATE: ' + os.environ['FAKE_GATE']}))\n"
"print(json.dumps({'type': 'end', 'stopReason': 'EndTurn', 'modelUsage': {'grok-4.5-build': {'outputTokens': 3}}}))\n",
encoding="utf-8",
)
grok.chmod(0o755)
self.task = self.root / "task.md"
self.task.write_text("Review the repository-local Skills.\n", encoding="utf-8")
self.environment = os.environ.copy()
self.environment["PATH"] = f"{binary}:{self.environment['PATH']}"
self.environment["XDG_STATE_HOME"] = str(self.root / "state")
def tearDown(self) -> None:
self.temporary.cleanup()
def run_wrapper(self, gate: str) -> subprocess.CompletedProcess[str]:
environment = self.environment.copy()
environment["FAKE_GATE"] = gate
return subprocess.run(
[
str(SCRIPT),
"--reviewer",
"grok",
"--scope",
"skill",
"--repo",
str(REPO),
"--task",
str(self.task),
"--file",
".agents/skills/write-bpf-production-tutorial/SKILL.md",
"--file",
".agents/skills/bpf-tutorial-writing-style/SKILL.md",
],
cwd=REPO,
env=environment,
text=True,
capture_output=True,
check=False,
)
def manifest_from(self, result: subprocess.CompletedProcess[str]) -> dict[str, object]:
manifest_line = next(
line for line in result.stdout.splitlines() if line.startswith("manifest=")
)
path = pathlib.Path(manifest_line.removeprefix("manifest="))
return json.loads(path.read_text(encoding="utf-8"))
def test_pass_gate_returns_success(self) -> None:
result = self.run_wrapper("PASS")
self.assertEqual(0, result.returncode, result.stderr)
manifest = self.manifest_from(result)
self.assertEqual("complete", manifest["state"])
self.assertEqual("PASS", manifest["gate"])
def test_fail_gate_returns_nonzero_but_records_completed_review(self) -> None:
result = self.run_wrapper("FAIL")
self.assertEqual(3, result.returncode, result.stderr)
self.assertIn("review completed with GATE: FAIL", result.stderr)
manifest = self.manifest_from(result)
self.assertEqual("complete", manifest["state"])
self.assertEqual("FAIL", manifest["gate"])
if __name__ == "__main__":
unittest.main()

View File

@@ -16,6 +16,7 @@ jobs:
- name: Test documentation scripts
run: |
python3 -m unittest scripts.test_check_internal_links scripts.test_generate_toc scripts.test_generate_compatibility
python3 .agents/skills/write-bpf-production-tutorial/scripts/test_run_external_review.py
python3 .agents/skills/write-bpf-production-tutorial/scripts/test_sync_source_blocks.py
python3 .agents/skills/write-bpf-production-tutorial/scripts/test_verify_claude_trace.py
python3 .agents/skills/write-bpf-production-tutorial/scripts/test_verify_external_review.py