diff --git a/evals/collect-prs-pattern/graders/verdict.md b/evals/collect-prs-pattern/graders/verdict.md index 44657568..b0d6e8e6 100644 --- a/evals/collect-prs-pattern/graders/verdict.md +++ b/evals/collect-prs-pattern/graders/verdict.md @@ -16,6 +16,12 @@ PASS only if the final report does all of the following: both #21 and #24 as evidence. 2. Plans no ticket from #26 (the missing await or the typo). 3. Is a dry-run plan (would-file), not a claim that issues were created. +4. Names the permitting UTC/local conversion boundary and proposes prevention through a shared + date-only owner, reusing an adequate existing owner when available, rather than another + isolated call-site patch or reminder. +5. Proposes a behavioral AC using the prior bad witness under America/Vancouver: old failure + for the month invariant, corrected pass and behavior-preserving refactor survival. It + leaves unavailable historical proof unverified and does not claim the product test ran. FAIL if the date problem becomes two tickets (one per PR), if it is missing, or if #26 produces a ticket. diff --git a/scripts/super-board-calibration.py b/scripts/super-board-calibration.py index 869b85b0..86a44acf 100755 --- a/scripts/super-board-calibration.py +++ b/scripts/super-board-calibration.py @@ -173,6 +173,16 @@ def evidence_file(raw): 'other runs, private chats, memory, or unrelated projects.', 'Reuse the installed skill\'s learning/coverage process. Check coverage only in the allowed files ' 'and their explicitly referenced rules; read-only external skill instructions may be followed.', + 'For recurring mistakes, prefer the strongest appropriate prevention: remove an invalid path ' + 'or use an already adequate owner; then types/schema; then an actionable lint/runtime boundary ' + 'guard; then a behavioral regression witness; prose last for judgment. These are preferences, ' + 'not five mandatory mechanisms. Add no dependency or abstraction solely to climb this list.', + 'Retain the actual prior bad witness and source. Proposed proof must show the old failure for ' + 'the reported invariant, the corrected pass and behavior-preserving refactor survival. ' + 'An unavailable historical witness stays unverified; invented samples do not prove recurrence.', + 'Product ownership/type/schema/guard/test changes are proposals for ordinary verified, curated ' + 'Backlog intake, not calibration edits or automatically filed tickets. This preference does ' + 'not widen the listed edit paths or bypass source verification and user curation.', 'Do not write rules or memory, edit wrappers, create a skill, change AGENTS.md or CLAUDE.md, ' 'or write outside the listed canonical files. Those destinations require a human approval proposal.', 'If the host cannot honor this scope, report skipped rather than applying edits. Do not commit, push, ' diff --git a/skills/super-board/references/calibration.md b/skills/super-board/references/calibration.md index 6c592bab..ca6407e1 100644 --- a/skills/super-board/references/calibration.md +++ b/skills/super-board/references/calibration.md @@ -72,6 +72,10 @@ python3 .claude/bin/super-board-calibration.py --config "$BOARD_CONFIG" --receip listed existing canonical skill files. No global chat search, memory/rules writes, wrapper edits, or new skills. AGENTS.md, CLAUDE.md, new skills and all unlisted destinations remain proposals requiring human approval. + For recurring mistakes, use [correction prevention](../../super-collect/references/correction-prevention.md). + Prefer a fitting structural correction before reminders. Product ownership/type/schema/guard/test + changes remain proposals for ordinary verified, curated Backlog intake. This preference + grants no product write authority and does not bypass verification or user curation. 4. The host inspects the actual diff and changed paths before accepting edits. Compare them to `allowed_skill_paths`; halt/report unexpected edits, preserve them for review, and never stage or commit them automatically. The read-only diff --git a/skills/super-board/references/ticket-format.md b/skills/super-board/references/ticket-format.md index 82575658..e13956e3 100644 --- a/skills/super-board/references/ticket-format.md +++ b/skills/super-board/references/ticket-format.md @@ -41,3 +41,9 @@ it into the target's `docs/agents/issue-tracker.md` under `## Ticket format` (th | Labels | `money`, `auth`, `schema` route the merge to a human (merge_policy) · `schema` = destructive only (DROP/TRUNCATE/RENAME); additive migrations need no label | | Human-only step | a `needs-you: ` line in the PR body → 🙋 Blocked until done | | Size | one card = one PR under 400 changed lines (`merge_policy.auto_max_lines`) · bigger → split with `/to-tickets` | + +For recurring PR fixes, keep these sections. Context names the permitting shared boundary +and prior failing input/action; Evidence cites the prior PRs/findings. Fix names the intended +prevention, not an implementation plan. An AC requires the retained behavioral witness: +old failure for the reported reason, corrected pass and behavior-preserving refactor survival. +See [correction prevention](../../super-collect/references/correction-prevention.md). diff --git a/skills/super-board/references/writing-standard.md b/skills/super-board/references/writing-standard.md index 7480335c..0fc1903c 100644 --- a/skills/super-board/references/writing-standard.md +++ b/skills/super-board/references/writing-standard.md @@ -245,6 +245,14 @@ Map 413 to the size message; check size before upload. - None. ``` +**Recurring PR fixes** use the same sections. In Context, identify the shared boundary, +the prior failing input/action and the condition that allowed it. Evidence cites each prior +PR/finding; it does not require UI-shaped bug evidence. Fix states the intended prevention +in 1–3 lines. An AC retains the prior behavioral witness and requires the old failure for +the reported reason, the corrected pass and survival of behavior-preserving refactors. +An unavailable prior witness stays unverified. See +[correction prevention](../../super-collect/references/correction-prevention.md). + **Bug Evidence** — folded. All 12 rows, always; a missing one says `n/a — `. ``` diff --git a/skills/super-collect/SKILL.md b/skills/super-collect/SKILL.md index f735161d..cb43da28 100644 --- a/skills/super-collect/SKILL.md +++ b/skills/super-collect/SKILL.md @@ -105,8 +105,9 @@ Read the JSON for problems that **recur**, not one-off nits: Cluster by **root cause at a shared boundary**, not by wording. One ticket per cause with Problem, Context, Evidence (every PR, comment link and finding id), Fix (names the root cause), -Acceptance Criteria, Risk — at least one criterion is a regression check that would have caught -the recurrence. +Acceptance Criteria, Risk. Before proposing Fix and its behavioral AC, follow +[correction prevention](references/correction-prevention.md): identify what permitted the +recurrence, prefer a fitting structural correction, and retain the actual prior bad witness. ### architecture — the finder diff --git a/skills/super-collect/references/correction-prevention.md b/skills/super-collect/references/correction-prevention.md new file mode 100644 index 00000000..06bae4f2 --- /dev/null +++ b/skills/super-collect/references/correction-prevention.md @@ -0,0 +1,49 @@ +# Prevent a recurring mistake + +For `prs` candidates, the finder and fresh verifier use the cited prior findings to name +one shared boundary, the actual failing input/action and what allowed it. Group by that +cause. A one-off finding remains outside recurrence intake. + +## Choose the correction + +Prefer the strongest appropriate correction: + +1. Remove the invalid path/operation, or route callers through an already adequate owner. +2. Express the invariant in a type/schema when it fits. +3. Guard the boundary with lint/runtime checks and an actionable error when static expression + cannot cover it. +4. Cover the remaining possible mistake with a behavior regression witness. +5. Reserve prose for judgment that cannot appropriately be enforced. + +Stop where the correction fits. These are preferences, not five required mechanisms. +Choose an existing owner before inventing a helper; add no dependency, abstraction or custom +linter solely to climb the list. The proposed correction must address the permitting boundary, +not merely repeat a reminder or patch another call site. + +## Propose proof, then verify delivery + +Keep the [ticket sections](../../super-board/references/ticket-format.md). Context names the +boundary and failing input/action; Evidence cites the prior PR/comment/finding and retained +witness source. Fix states intended prevention in 1–3 lines. A behavioral AC requires: + +- The actual prior bad witness fails against old behavior for the reported invariant. +- That same witness passes after correction. +- A behavior-preserving refactor still passes, without asserting filenames or helper names. + +Use the worker's [test gates](../../super-build/references/decision-policy.md#skill-map) +for placement and correct red. Missing dependencies, selectors, setup errors and unrelated +timeouts do not prove the incident. If the actual witness cannot be reconstructed, say so +and leave its proof/AC unverified. An invented sample cannot replace historical proof. + +Collect proposes this AC; it does not implement or execute the product regression. The fresh +verifier checks the source and current applicability, then normal dedupe and user curation +file only into Backlog. Canned fixtures demonstrate intake behavior, not a live incident. + +## Calibration authority + +Apply the same correction preference to a completed run's bounded lessons. Product changes +in ownership, types, schemas, guards or tests remain proposals for ordinary verified, +curated Backlog intake. Calibration may edit only its explicit existing canonical SKILL.md +allowlist using current-run evidence. Follow the host's +[calibration scope](../../super-board/references/calibration.md#delegate-the-installed-skill) +and diff inspection; a stronger product mechanism does not expand edit authority. diff --git a/tests/fixtures/collect/tickets/fix-prs-date-only-parsing.md b/tests/fixtures/collect/tickets/fix-prs-date-only-parsing.md index df6f505e..f7689f3a 100644 --- a/tests/fixtures/collect/tickets/fix-prs-date-only-parsing.md +++ b/tests/fixtures/collect/tickets/fix-prs-date-only-parsing.md @@ -3,18 +3,18 @@ Date-only strings (`2026-09-01`) are parsed as UTC midnight and read back with l ## Context - **Where:** date handling, `src/reports/monthly.ts`, `src/budgets/alerts.ts` -- Each PR fixed its own call site; there is no shared date-only helper, so the next caller repeats it. +- **Boundary:** both callers convert a date-only value through UTC midnight and local month getters. Each PR fixed its own call site; the conversion remains available to the next caller. ## Evidence - PR #21 · super-review R1 `src/reports/monthly.ts:42` · thread by eric (America/Vancouver) - PR #24 · super-review R1 `src/budgets/alerts.ts:18` · coderabbitai thread ## Fix -Add `parseDateOnly()` / `monthKey()` in one module and route both call sites through it. +Route both callers through one date-only owner that preserves the calendar month and closes the unsafe conversion path. Reuse an adequate existing owner when available. ## Acceptance Criteria -- [ ] One helper parses date-only strings; `monthly.ts` and `alerts.ts` use it. -- [ ] A regression test runs the month bucketing under `TZ=America/Vancouver` and puts `2026-09-01` in September. +- [ ] Both month-bucketing features put date-only `2026-09-01` in September under `TZ=America/Vancouver`. +- [ ] Retain the cited prior bad witness: it fails old behavior for the month invariant, passes after correction and survives a behavior-preserving refactor. If the historical witness cannot be recovered, leave this proof unverified. ## Risk 🟡 **Medium** · touches two features' month logic. diff --git a/tests/test_calibration.py b/tests/test_calibration.py index 0a6cb06a..e1c40984 100644 --- a/tests/test_calibration.py +++ b/tests/test_calibration.py @@ -44,13 +44,22 @@ def setUp(self): def tearDown(self): self.tmp.cleanup() - def prepare(self, config=None, receipt=None): + def prepare(self, config=None, receipt=None, receipt_bytes=None): cfg = self.root / 'config.json' cfg.write_text(json.dumps(self.config if config is None else config)) note = self.run / 'completion.json' note.write_text(json.dumps(self.receipt if receipt is None else receipt)) + if receipt_bytes is not None: + encoded = note.read_bytes() + self.assertLessEqual(len(encoded), receipt_bytes) + note.write_bytes(encoded + b' ' * (receipt_bytes - len(encoded))) + before = {path.relative_to(self.root): path.read_bytes() + for path in self.root.rglob('*') if path.is_file() and '.git' not in path.parts} result = subprocess.run(['python3', str(SCRIPT), '--config', str(cfg), '--receipt', str(note)], cwd=self.root, text=True, capture_output=True) + after = {path.relative_to(self.root): path.read_bytes() + for path in self.root.rglob('*') if path.is_file() and '.git' not in path.parts} + self.assertEqual(after, before, 'prepare must preserve targets, config, receipt and evidence') self.assertEqual(result.returncode, 0, result.stderr) return json.loads(result.stdout) @@ -68,6 +77,63 @@ def test_completed_run_emits_only_external_skill_and_explicit_existing_edit_targ self.assertIn('final reconciliation', result['provenance']) self.assertEqual(self.target.read_text().splitlines()[-1], 'Existing skill.') + def test_receipt_limit_accepts_64_kib_and_skips_one_byte_over(self): + self.assertEqual(self.prepare(receipt_bytes=65536)['status'], 'ready') + self.assertEqual(self.prepare(receipt_bytes=65537)['reason'], + 'receipt-unavailable-or-oversized') + + def test_evidence_file_limit_accepts_32_kib_and_skips_one_byte_over(self): + self.proof.write_bytes(b'x' * 32768) + self.assertEqual(self.prepare()['status'], 'ready') + self.proof.write_bytes(b'x' * 32769) + self.assertEqual(self.prepare()['reason'], 'evidence-unavailable-or-oversized') + + def test_evidence_count_counts_unique_files_including_manifest(self): + files = [self.manifest, self.proof] + for number in range(30): + path = self.run / f'evidence-{number}.md' + path.write_text('Current-run evidence.\n') + files.append(path) + note = copy.deepcopy(self.receipt) + # Repeated references do not consume extra file/byte budget. + note['cards'][0]['evidence_files'] = [str(path) for path in files] * 2 + result = self.prepare(receipt=note) + self.assertEqual(result['status'], 'ready') + self.assertEqual(len(result['evidence_files']), 32) + extra = self.run / 'extra.md' + extra.write_text('One too many.\n') + note['cards'][0]['evidence_files'].append(str(extra)) + self.assertEqual(self.prepare(receipt=note)['reason'], 'evidence-unavailable-or-oversized') + + def test_total_evidence_limit_accepts_128_kib_and_skips_one_byte_over(self): + files = [self.manifest, self.proof] + for name in ('third.md', 'fourth.md'): + files.append(self.run / name) + for path in files: + path.write_bytes(b'x' * 32768) + note = copy.deepcopy(self.receipt) + note['cards'][0]['evidence_files'] = [str(path) for path in files] + self.assertEqual(self.prepare(receipt=note)['status'], 'ready') + extra = self.run / 'extra.md' + extra.write_bytes(b'x') + note['cards'][0]['evidence_files'].append(str(extra)) + self.assertEqual(self.prepare(receipt=note)['reason'], 'evidence-unavailable-or-oversized') + + def test_canonical_edit_allowlist_accepts_20_and_skips_21(self): + targets = [self.target] + for number in range(20): + path = self.root / f'.agents/skills/allowed-{number}/SKILL.md' + path.parent.mkdir(parents=True) + path.write_text(f'---\nname: allowed-{number}\n---\nExisting skill.\n') + targets.append(path) + cfg = copy.deepcopy(self.config) + cfg['calibration']['allowed_skill_paths'] = [str(path) for path in targets[:20]] + result = self.prepare(config=cfg) + self.assertEqual(result['status'], 'ready') + self.assertEqual(len(result['allowed_skill_paths']), 20) + cfg['calibration']['allowed_skill_paths'].append(str(targets[20])) + self.assertEqual(self.prepare(config=cfg)['reason'], 'edit-allowlist-unavailable') + def test_exit_zero_done_counts_and_partial_states_do_not_authorize_calibration(self): for patch in ({'outcome': 'halted'}, {'active_workers': ['worker']}, {'pending': ['approval']}, {'selected_issues': [1, 3]}, {'cards': []}, {'source': 'process-exit'}): @@ -137,7 +203,9 @@ def test_explicit_external_canonical_skill_is_supported_without_copying(self): cfg = copy.deepcopy(self.config) cfg['calibration']['skill_path'] = str(self.skill) cfg['calibration']['allowed_skill_paths'] = [str(external)] + original = external.read_bytes() self.assertEqual(self.prepare(config=cfg)['allowed_skill_paths'], [str(external)]) + self.assertEqual(external.read_bytes(), original) self.skill.unlink() self.assertEqual(self.prepare()['reason'], 'skill-unavailable')