From fb1de94fe3c0ebd596b9fcae7bf8412ddd6363e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 15:49:30 +0800 Subject: [PATCH 1/2] fix(walkthrough): honesty + discoverability fixes across CLI surfaces Auto interactive-walkthrough of the Atlas CLI surfaces (5 reviewed, no P0/P1). Four evidence-backed fixes under TDD + Second-Check: - validate-prd Check 6: fail-closed on an ABSENT requirements section instead of vacuously passing + claiming 'all requirements are specific' (P2 honesty). - watcher permit reason: surface a cheating-winner discrepancy instead of masking it behind 'no to-be-slashed' (P2 legibility; the permit boolean is unchanged). - set-status --status: enumerate the valid status vocabulary in -h (P2). - engine-preflight/next-task/economy-report: add missing flag help (P3 consistency). RAG-ready runlogs under .i-walkthroughs/entries/. Offline suite 886 passed / 5 skipped. Co-Authored-By: Claude Opus 4.8 --- .i-walkthroughs/README.md | 6 ++ .../2026-06-17-cli-undocumented-flags.md | 57 ++++++++++ ...6-17-cli-validate-prd-check6-false-pass.md | 58 ++++++++++ ...26-06-17-cli-watcher-permit-reason-mask.md | 57 ++++++++++ .i-walkthroughs/index.jsonl | 3 + .i-walkthroughs/schema.md | 100 ++++++++++++++++++ prd_taskmaster/cli.py | 20 ++-- prd_taskmaster/tournament/watcher.py | 7 +- prd_taskmaster/validation.py | 31 ++++-- tests/core/test_cli.py | 20 ++++ tests/core/test_tournament_watcher.py | 15 +++ tests/core/test_validation.py | 23 ++-- 12 files changed, 373 insertions(+), 24 deletions(-) create mode 100644 .i-walkthroughs/README.md create mode 100644 .i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md create mode 100644 .i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md create mode 100644 .i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md create mode 100644 .i-walkthroughs/index.jsonl create mode 100644 .i-walkthroughs/schema.md diff --git a/.i-walkthroughs/README.md b/.i-walkthroughs/README.md new file mode 100644 index 0000000..38bd175 --- /dev/null +++ b/.i-walkthroughs/README.md @@ -0,0 +1,6 @@ +# Interactive-walkthrough runlogs + +RAG-ingestion-ready learning units from `/interactive-walkthrough` runs on this repo. +One file per (surface, issue) under `entries/`; `index.jsonl` is the append-only machine index; +`schema.md` is the single source of truth. Headless repo: "route" = a CLI/MCP surface +(e.g. `cli:validate-prd`), evidence = live CLI/test runs. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md b/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md new file mode 100644 index 0000000..260776f --- /dev/null +++ b/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md @@ -0,0 +1,57 @@ +--- +id: 2026-06-17-cli-undocumented-flags +schema_version: 1 +repo: anombyte93/prd-taskmaster +branch: walkthrough/atlas-cli-surfaces +commit: pending +date: 2026-06-17 +route: "cli:set-status" +screen_purpose: "Expose the engine's CLI flags clearly enough that a cold user can discover what each does — and what values it accepts — from -h alone." +mode: auto +viewport: cli +device_class: desktop +severity: P2 +category: interaction +root_cause: "Several argparse flags were added with no help= string; most damaging, set-status --status is a REQUIRED flag whose valid vocabulary (pending/in-progress/done/review/deferred/cancelled/blocked) appeared nowhere in help, SKILL.md, or the MCP tool." +assumption_trap: "the flag name is self-documenting (a required enum flag is not — the accepted values must be shown)." +reusable_rule: "Every CLI flag gets help text; a required enum flag must enumerate its accepted values in that help." +evidence_method: [code] +files_changed: ["prd_taskmaster/cli.py", "tests/core/test_cli.py"] +first_check: {result: fail, metric: "build_parser(): set-status --status, engine-preflight --no-configure, next-task --tag, economy-report --input all have help==None", evidence_ref: "pytest test_cli_flags_have_discoverable_help (pre-fix RED) + live set-status -h showing bare '--status STATUS'"} +second_check: {result: pass, metric: "all four flags have non-empty help; set-status --status help contains 'in-progress'", evidence_ref: "pytest (GREEN) + live: set-status -h now prints 'Task status; one of: pending, in-progress, done, review, deferred, cancelled, blocked'", regression_scan: pass} +regression_checks: ["set-status invalid-status error path unchanged (help-only, no argparse choices added → still returns FR-28 dict, not an argparse usage error)", "all existing CLI subprocess tests", "full offline suite"] +verdict: resolved +status: verified +approval: pending +evidence_dir: evidence/2026-06-17-cli-undocumented-flags +tags: [cli, discoverability, help-text, consistency, set-status] +related: [] +title: "Undocumented CLI flags — esp. set-status --status whose valid vocabulary was undiscoverable" +--- + +## Lesson (TL;DR) +Four CLI flags carried no help text; the worst was `set-status --status`, a REQUIRED flag whose accepted values were findable nowhere a user looks (`-h`, SKILL.md, or the MCP tool). A cold user had to read source to learn the status vocabulary. Every flag gets help; a required enum flag enumerates its values. + +## Screen & Purpose +Surfaces `cli:set-status`, `cli:engine-preflight`, `cli:next-task`, `cli:economy-report`. Soul purpose: a self-describing CLI a cold user can drive from `-h`. + +## Issue (first check) +`build_parser()` introspection: `set-status --status`, `engine-preflight --no-configure`, `next-task --tag`, `economy-report --input` all had `help is None`. Live `set-status -h` printed `--status STATUS` with no value list — the vocabulary (pending/in-progress/done/review/deferred/cancelled/blocked) was invisible. + +## Root Cause +Flags were added via `add_argument(...)` without `help=`. Inconsistent with sibling flags (e.g. `parse-prd --input`) that do carry help. + +## Fix +`cli.py`: add help strings to the four flags. For `set-status --status`, the help enumerates the valid status vocabulary. Help-only (NOT argparse `choices=`) so the engine's existing FR-28-safe status validation + error dict is preserved rather than replaced by an argparse usage error. + +## Second Check (re-verification, MANDATORY) +Post-fix: introspection shows all four flags with non-empty help; `set-status --status` help contains "in-progress"; live `set-status -h` prints the full status list. Regression scan: the invalid-status path still returns the engine's FR-28 error dict (no `choices=` added, so no behavior change to error handling); all existing CLI subprocess tests pass; full offline suite green. `regression_scan=pass`, `verdict=resolved`. + +## Reusable Rule +Every CLI flag gets help text; a required enum flag must enumerate its accepted values in that help. + +## Decision Trail +Owner: walkthrough auto-mode; real owner approves via PR. Rejected: adding argparse `choices=` for `--status` (would move validation to the parse layer and emit an argparse usage error instead of the engine's structured FR-28 dict — a contract change). Deferred: also documenting the status vocabulary in SKILL.md + the MCP tool description (follow-up; this entry fixes the CLI surface). + +## Revisions +- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md b/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md new file mode 100644 index 0000000..548cebd --- /dev/null +++ b/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md @@ -0,0 +1,58 @@ +--- +id: 2026-06-17-cli-validate-prd-check6-false-pass +schema_version: 1 +repo: anombyte93/prd-taskmaster +branch: walkthrough/atlas-cli-surfaces +commit: pending +date: 2026-06-17 +route: "cli:validate-prd" +screen_purpose: "Give a user a fast, honest, point-scored verdict on whether a PRD is complete enough to parse into a trustworthy task graph." +mode: auto +viewport: cli +device_class: desktop +severity: P2 +category: trust +root_cause: "Check 6 ran VAGUE_PATTERN.findall over an empty string when no requirements section existed; an empty match list read as 'no vague terms' → passed:true + 'All requirements are specific'." +assumption_trap: "no-vague-terms == requirements-are-testable (true only when requirements actually exist)." +reusable_rule: "A presence/quality check over an ABSENT section must fail-closed, never pass-and-assert-quality on emptiness." +evidence_method: [code] +files_changed: ["prd_taskmaster/validation.py", "tests/core/test_validation.py"] +first_check: {result: fail, metric: "empty PRD → check6.passed=true, detail='All requirements are specific', checks_passed=3, score=13", evidence_ref: "live: python3 script.py validate-prd --input /tmp/wt_empty.md (pre-fix)"} +second_check: {result: pass, metric: "empty PRD → check6.passed=false, detail='No requirements section found', checks_passed=2, score=8, grade=NEEDS_WORK", evidence_ref: "live post-fix run + pytest -k 'validate_empty_prd or check6_fails_closed'", regression_scan: pass} +regression_checks: ["sample_prd (real requirements) still passes check6", "thorough PRD still 57/57 EXCELLENT exit 0", "grade boundary tests", "full offline suite tests/core+tests/plugin"] +verdict: resolved +status: verified +approval: pending +evidence_dir: evidence/2026-06-17-cli-validate-prd-check6-false-pass +tags: [validate-prd, honesty, fail-closed, vacuous-pass] +related: [] +title: "validate-prd Check 6 falsely passed + claimed specificity on a PRD with zero requirements" +--- + +## Lesson (TL;DR) +`validate-prd` Check 6 ("Functional requirements are testable") passed AND printed "All requirements are specific" on a PRD that had no requirements section at all — a positively false claim about content that does not exist. Root cause: a vague-term scan over an empty string returns no matches, which the code read as success. A presence/quality check over an absent section must fail-closed. + +## Screen & Purpose +Surface `cli:validate-prd` (13-check PRD quality gate, `validation.py:run_validate_prd`). Soul purpose: an honest, point-scored verdict on PRD completeness. Headless/CLI; evidence = live runs + unit tests. + +## Issue (first check) +Live, pre-fix, on a PRD containing only a title + Overview (zero requirements): +`check 6 → passed:true, detail:"All requirements are specific"`; `checks_passed:3, score:13`. The detail makes an affirmative specificity claim about requirements that are absent. (The final grade NEEDS_WORK was already correct; the defect is the per-check honesty + an inflated checks_passed.) + +## Root Cause +`reqs_section` was empty; `VAGUE_PATTERN.findall("")==[]` → `passed = len([])==0 = True` and the else-branch detail "All requirements are specific" fired. The check never distinguished "present and specific" from "absent". + +## Fix +`validation.py`: when `reqs_section.strip()` is empty, emit `passed:false, detail:"No requirements section found"` (fail-closed); otherwise the original vague-term logic. Smallest coherent change; checks 5 & 10 (user-stories / NFRs are genuinely OPTIONAL sections) left intentionally untouched. + +## Second Check (re-verification, MANDATORY) +Same empty PRD, post-fix: `check 6 → passed:false, detail:"No requirements section found"`, `checks_passed:2, score:8, grade:NEEDS_WORK`. Same-metric assertion now passes. Regression scan: `sample_prd` (real requirements) still passes check 6; a thorough PRD still scores 57/57 EXCELLENT (exit 0); grade-boundary tests green; full offline suite green. `regression_scan=pass`, `verdict=resolved`. + +## Reusable Rule +A presence/quality check over an ABSENT section must fail-closed — never pass-and-assert-quality on emptiness. + +## Decision Trail +Owner: walkthrough auto-mode (Hayden stand-in); real owner approves via the PR. Rejected: only rewording the detail while keeping `passed:true` (would leave the vacuous pass + inflated count). Deferred: checks 5 & 10 pass-on-absent (P3; those sections are legitimately optional) — recorded for a future decision. + +## Revisions +- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md b/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md new file mode 100644 index 0000000..251b393 --- /dev/null +++ b/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md @@ -0,0 +1,57 @@ +--- +id: 2026-06-17-cli-watcher-permit-reason-mask +schema_version: 1 +repo: anombyte93/prd-taskmaster +branch: walkthrough/atlas-cli-surfaces +commit: pending +date: 2026-06-17 +route: "cli:watcher-run" +screen_purpose: "Let an operator see the fail-closed real-slash permit for a job and understand WHY it is or isn't permitted." +mode: auto +viewport: cli +device_class: desktop +severity: P2 +category: trust +root_cause: "The permit reason ladder tested `not to_slash` before `discrepancies`, so a job with no to-be-slashed submissions but a cheating-winner DISCREPANCY reported the benign 'no to-be-slashed' message instead of the discrepancy." +assumption_trap: "empty to-be-slashed set == nothing interesting happened (ignores discrepancies/abstains on the winners)." +reusable_rule: "Order a fail-closed reason ladder most-serious-finding first; never let a benign branch shadow a security-relevant one." +evidence_method: [code] +files_changed: ["prd_taskmaster/tournament/watcher.py", "tests/core/test_tournament_watcher.py"] +first_check: {result: fail, metric: "permit_enforce_slash(record with empty to_slash + cheating-winner DISCREPANCY).reason == 'blocked: no to-be-slashed submissions to confirm' (discrepancy masked)", evidence_ref: "pytest test_permit_reason_surfaces_winner_discrepancy_over_empty_slash (pre-fix RED)"} +second_check: {result: pass, metric: "same record → reason contains 'discrepancy' and NOT 'no to-be-slashed'; permitted still False; ex-win in discrepancies", evidence_ref: "pytest (post-fix GREEN)", regression_scan: pass} +regression_checks: ["existing permit reason tests (discrepancy-among-slashes, abstained-winner, empty-to_slash, mixed-batch) unchanged", "permitted boolean decision unchanged across all permit tests", "full offline suite"] +verdict: resolved +status: verified +approval: pending +evidence_dir: evidence/2026-06-17-cli-watcher-permit-reason-mask +tags: [watcher, slash-gate, fail-closed, legibility, self-review] +related: [atlas-watcher-built] +title: "watcher permit reason masked a caught cheating-winner discrepancy with a benign 'no to-be-slashed' message" +--- + +## Lesson (TL;DR) +The watcher's real-slash permit correctly BLOCKED a job where a winner was an independent discrepancy, but its human-readable `reason` said "no to-be-slashed submissions to confirm" — hiding the very thing the operator most needs to see. The boolean was right; the explanation lied by omission. Order the reason ladder most-serious-first. + +## Screen & Purpose +Surface `cli:watcher-run` → `watcher.permit_enforce_slash` (the fail-closed gate that decides whether real `--enforce-slash` forfeiture may proceed). Soul purpose: an operator-legible, honest permit verdict. + +## Issue (first check) +A record with zero to-be-slashed submissions but a cheating winner (recorded PASS, watcher DISCREPANCY) returned `permitted:false` (correct) with `reason:"blocked: no to-be-slashed submissions to confirm"` (wrong — the discrepancy was in `out["discrepancies"]` but never surfaced in the headline reason). `to_slash` is empty because the winner recorded PASS, so the `elif not to_slash` branch fired before `elif discrepancies`. + +## Root Cause +Reason-ladder ordering: `permitted → not to_slash → discrepancies → abstained → track_record`. The benign empty-set branch shadowed the security-relevant discrepancy branch. + +## Fix +`watcher.py`: reorder the ladder to `permitted → discrepancies → abstained → not to_slash → track_record`. The `permitted` boolean is unchanged (already `not discrepancies and not abstained and ...`); only the explanatory string ordering changed. + +## Second Check (re-verification, MANDATORY) +Post-fix, same record: `reason` contains "discrepancy", does NOT contain "no to-be-slashed", `permitted` still False, `ex-win` in `discrepancies`. Regression scan: all prior permit tests (discrepancy-among-slashes, abstained-winner, empty-to_slash, mixed-batch, concordance boundary) unchanged; every `permitted` boolean identical to before. Full offline suite green. `regression_scan=pass`, `verdict=resolved`. + +## Reusable Rule +Order a fail-closed reason ladder most-serious-finding first; a benign branch must never shadow a security-relevant one. + +## Decision Trail +Owner: walkthrough auto-mode; real owner approves via PR. This fix touches code authored earlier this same session — a good reminder that the adversarial surface review caught a legibility gap the build-time tests didn't assert. No alternatives rejected; the reorder is the minimal correct change. + +## Revisions +- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/index.jsonl b/.i-walkthroughs/index.jsonl new file mode 100644 index 0000000..5f21453 --- /dev/null +++ b/.i-walkthroughs/index.jsonl @@ -0,0 +1,3 @@ +{"id":"2026-06-17-cli-validate-prd-check6-false-pass","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:validate-prd","date":"2026-06-17","severity":"P2","category":"trust","verdict":"resolved","status":"verified","title":"validate-prd Check 6 falsely passed + claimed specificity on a PRD with zero requirements","file":"entries/2026-06-17-cli-validate-prd-check6-false-pass.md"} +{"id":"2026-06-17-cli-watcher-permit-reason-mask","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:watcher-run","date":"2026-06-17","severity":"P2","category":"trust","verdict":"resolved","status":"verified","title":"watcher permit reason masked a caught cheating-winner discrepancy","file":"entries/2026-06-17-cli-watcher-permit-reason-mask.md"} +{"id":"2026-06-17-cli-undocumented-flags","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:set-status","date":"2026-06-17","severity":"P2","category":"interaction","verdict":"resolved","status":"verified","title":"Undocumented CLI flags incl. set-status --status vocabulary","file":"entries/2026-06-17-cli-undocumented-flags.md"} diff --git a/.i-walkthroughs/schema.md b/.i-walkthroughs/schema.md new file mode 100644 index 0000000..ced26a9 --- /dev/null +++ b/.i-walkthroughs/schema.md @@ -0,0 +1,100 @@ +# `.i-walkthroughs/` schema (v1) + +> **Skill template.** This is the canonical copy the `interactive-walkthrough` skill carries. +> On first use in a repo that has no `.i-walkthroughs/`, copy this file to +> `/.i-walkthroughs/schema.md` (then create `README.md`, an empty `index.jsonl`, +> `entries/`, and `evidence/`) and commit, so the Runlog gate's "per `.i-walkthroughs/schema.md`" +> always resolves against a repo-local source of truth. + +Canonical field dictionary for walkthrough runlog entries. A future repo-wide RAG ingester +reads THIS file. Stable contract: glob `**/.i-walkthroughs/entries/*.md`, parse YAML +frontmatter as the metadata record, treat each `##` body heading as a chunk boundary, upsert +keyed on `id` (== filename stem), gate migrations on `schema_version`. + +## Folder layout + +``` +.i-walkthroughs/ + README.md # what this is + RAG-forward note + schema.md # THIS file — single source of truth + index.jsonl # append-only machine index, one JSON object per entry + entries/ + .md # one self-contained learning unit per file + evidence/ + / # per-entry assets (id == entry id) + before-.png # pre-fix VIEWPORT screenshot + after-.png # post-fix (second-check) VIEWPORT screenshot + dom-measure.json # {"before":{...},"after":{...}} authoritative numbers +``` + +**File strategy:** one file per `(screen, issue)` learning unit (NOT an append-to-one-log), +because a RAG hit must return a complete, single-issue document with correct metadata so +filters like `verdict:resolved AND severity:P1` select exactly the right unit. `index.jsonl` +is the fast non-vector path to enumerate/pre-filter the corpus without parsing markdown. + +**`` == filename stem == frontmatter `id` == `index.jsonl` row key.** Never reused, never changed. + +## Slug / id rules + +`` = `--` +- `route-slug`: route with `/` → `-`, leading `-` stripped; root `/` → `root`. +- `issue-slug`: 2–4 word kebab summary of the defect. +- lowercase ASCII, no spaces. Same-day collision → append `-2`, `-3`. + +## Frontmatter fields + +| key | type | purpose | +|---|---|---| +| `id` | string (== filename stem) | Primary key for RAG upsert/dedup + `index.jsonl` join + git anchor. Immutable. | +| `schema_version` | int (start 1) | Migration gate for field renames. | +| `repo` | string `owner/name` | Repo-wide RAG filter dimension (this repo's `owner/name`, e.g. `anombyte93/atlas-ai-website`). | +| `branch` | string | Branch the fix landed on. | +| `commit` | git SHA \| `pending` | Hard link to the resolving diff; `pending` until committed. | +| `date` | ISO date `YYYY-MM-DD` | Day the second check passed; filename prefix + index sort key. | +| `route` | string app route | Primary filter: "everything learned about `/calculator`". Leading slash; root `/`. | +| `screen_purpose` | string (one sentence) | The screen soul purpose; anchors the embedding to intended behavior. | +| `mode` | enum `interactive` \| `auto` \| `mobile` | Which walkthrough mode produced this. | +| `viewport` | string `WxH` or `WxH@dpr` | Reproduction context (e.g. `390x844`). | +| `device_class` | enum `mobile` \| `tablet` \| `desktop` | Coarse class for filtering + regression arm. | +| `severity` | enum `P0` \| `P1` \| `P2` \| `P3` | Reuses `reference/auto-report-schema.md` labels verbatim. | +| `category` | enum `layout` \| `copy` \| `a11y` \| `perf` \| `security` \| `interaction` \| `data` \| `trust` \| `role-boundary` | Defect taxonomy for clustering. | +| `root_cause` | string (one line) | The why (NOT the symptom). High-value learn-from-mistakes field. | +| `assumption_trap` | string \| null | The false belief made/nearly made (e.g. "desktop pass == mobile pass"). Drives `reusable_rule`. | +| `reusable_rule` | string (one line, imperative) | Portable rule a future agent applies to avoid repeating this. Highest-signal retrieval target. | +| `evidence_method` | list[enum `dom-measure` \| `viewport-screenshot` \| `console` \| `network` \| `axe` \| `lighthouse` \| `code`] | How proof was gathered, ordered by authority (`dom-measure` first). | +| `files_changed` | list[string] repo-relative | Source files the fix touched. | +| `first_check` | object `{result: fail\|pass, metric, evidence_ref}` | Pre-fix proof the issue was real. Required for P0/P1. | +| `second_check` | object `{result: pass\|fail, metric, evidence_ref, regression_scan: pass\|fail}` | MANDATORY post-fix re-verification. `result=pass` ONLY if same metric now passes AND `regression_scan=pass`. `metric` must be the SAME numeric assertion as `first_check.metric`. | +| `regression_checks` | list[string] | The specific adjacent things re-verified (other viewport class, overflow, focus, console/CSP). Backs `second_check.regression_scan`. | +| `verdict` | enum `resolved` \| `partial` \| `regressed` \| `reverted` | Headline outcome. `resolved`=gone + no regression; `partial`=residual remains; `regressed`=fix broke something; `reverted`=rolled back. | +| `status` | enum `open` \| `verified` \| `shipped` | Lifecycle, orthogonal to verdict. `open`=no passing second check yet; `verified`=second check passed; `shipped`=committed/merged. | +| `approval` | string \| `pending` | Who approved the mutating action (owner handle + when + channel). In `--mobile` this is the Discord reply. | +| `evidence_dir` | string repo-relative | `evidence//` — keeps binaries out of embedded text but linkable. | +| `tags` | list[string] kebab | Free-form RAG facets beyond the fixed enums. | +| `related` | list[string] entry ids | Graph edges (recurrence → prior resolved entry, supersedes, caused-by). | +| `title` | string (short headline) | Display + embedding-friendly summary; also `index.jsonl` `title`. | + +## Body sections (in order; each `##` is a chunk boundary) + +1. **Lesson (TL;DR)** — standalone 2–4 sentences; restates `root_cause` + `reusable_rule` in prose. Lead chunk. +2. **Screen & Purpose** — route, role/context, viewport, soul purpose. +3. **Issue (first check)** — observable QUANTITATIVE symptom; DOM numbers first, then screenshot ref. Never an opinion. +4. **Root Cause** — underlying cause + the `assumption_trap` narrative. +5. **Fix** — smallest coherent change, `files_changed`, load-bearing snippet, commit. +6. **Second Check (re-verification, MANDATORY)** — fresh same-screen/same-viewport evidence: (a) same-defect proof via the same metric, (b) explicit regression scan, then the verdict. +7. **Reusable Rule** — the portable rule, imperative. +8. **Decision Trail** — owner decision + channel, rejected alternatives, deferred items, open questions. +9. **Revisions** — append-only log of in-turn updates (e.g. a regressed second check → re-fix). Keeps the full mistake arc in one unit. + +## Re-touch rule + +- In-turn correction (a fix regresses on its own second check this turn) → **update the same entry** (Revisions section; verdict flips). +- A separate later walkthrough re-touching the same screen → **new dated entry** with `related` linking back. The corpus accumulates `mistake → fix → second-check` triples over time. + +## `index.jsonl` row shape + +One JSON object per line, append-only (never rewrite prior lines): + +```json +{"id":"...","repo":"owner/name","branch":"...","route":"/...","date":"YYYY-MM-DD","severity":"P0|P1|P2|P3","category":"...","verdict":"resolved|partial|regressed|reverted","status":"open|verified|shipped","title":"...","file":"entries/.md"} +``` diff --git a/prd_taskmaster/cli.py b/prd_taskmaster/cli.py index 41a832a..0011a7a 100644 --- a/prd_taskmaster/cli.py +++ b/prd_taskmaster/cli.py @@ -176,7 +176,10 @@ def build_parser() -> argparse.ArgumentParser: # engine-preflight (batched Phase 1: preflight + taskmaster + providers + capabilities) p = sub.add_parser("engine-preflight", help="One-call Phase 1: all probes + summary") - p.add_argument("--no-configure", action="store_true") + p.add_argument( + "--no-configure", action="store_true", + help="Skip auto-configuring providers; read-only probe (default configures providers on an existing .taskmaster project)", + ) # detect-taskmaster sub.add_parser("detect-taskmaster", help="Find MCP or CLI taskmaster") @@ -329,7 +332,7 @@ def build_parser() -> argparse.ArgumentParser: # economy-report p = sub.add_parser("economy-report", help="Summarize .atlas-ai/telemetry.jsonl per (op_class, model)") - p.add_argument("--input", default=None) + p.add_argument("--input", default=None, help="Telemetry JSONL path (default: .atlas-ai/telemetry.jsonl)") # context-pack p = sub.add_parser("context-pack", help="Extract AST-based Python signature context") @@ -357,17 +360,20 @@ def build_parser() -> argparse.ArgumentParser: # next-task p = sub.add_parser("next-task", help="Select the next TaskMaster-compatible task") - p.add_argument("--tag") + p.add_argument("--tag", help="Tasks.json tag context to read (default: the active/master tag)") # claim-task p = sub.add_parser("claim-task", help="Atomically select and claim the next task") - p.add_argument("--tag") + p.add_argument("--tag", help="Tasks.json tag context to read (default: the active/master tag)") # set-status p = sub.add_parser("set-status", help="Set a task or subtask status") - p.add_argument("--id", required=True) - p.add_argument("--status", required=True) - p.add_argument("--tag") + p.add_argument("--id", required=True, help="Task or subtask id (e.g. 7 or 1.2)") + p.add_argument( + "--status", required=True, + help="Task status; one of: pending, in-progress, done, review, deferred, cancelled, blocked", + ) + p.add_argument("--tag", help="Tasks.json tag context to write (default: the active/master tag)") p.add_argument( "--evidence-ref", default=None, diff --git a/prd_taskmaster/tournament/watcher.py b/prd_taskmaster/tournament/watcher.py index f0fc4cd..c060d60 100644 --- a/prd_taskmaster/tournament/watcher.py +++ b/prd_taskmaster/tournament/watcher.py @@ -429,12 +429,15 @@ def permit_enforce_slash( f"permitted: {len(confirmed)} confirmed slash(es), " f"concordance={concordance:.3f} over {observations} prior decisions" ) - elif not to_slash: - reason = "blocked: no to-be-slashed submissions to confirm" + # Surface the most-serious finding FIRST: a discrepancy (incl. a cheating + # winner) or an abstain must never be masked by the benign "nothing to + # slash" message when to_slash happens to be empty. elif discrepancies: reason = f"blocked: {len(discrepancies)} discrepancy(ies) in the job" elif abstained: reason = f"blocked: {len(abstained)} submission(s) in the job could not be independently verified (abstain)" + elif not to_slash: + reason = "blocked: no to-be-slashed submissions to confirm" elif not track_record_ok: reason = ( f"blocked: thin track record — {observations}/{MIN_OBSERVATIONS} prior " diff --git a/prd_taskmaster/validation.py b/prd_taskmaster/validation.py index 3368ca4..b97dbe3 100644 --- a/prd_taskmaster/validation.py +++ b/prd_taskmaster/validation.py @@ -123,15 +123,28 @@ def run_validate_prd(input_path: str) -> dict: reqs_section = get_section_content(text, "Functional Requirements") if not reqs_section: reqs_section = get_section_content(text, "Requirements") - vague_in_reqs = VAGUE_PATTERN.findall(reqs_section) - checks.append({ - "id": 6, - "category": "required", - "name": "Functional requirements are testable", - "passed": len(vague_in_reqs) == 0, - "detail": f"Vague terms found: {vague_in_reqs}" if vague_in_reqs else "All requirements are specific", - "points": 5, - }) + if not reqs_section.strip(): + # Fail-closed: an ABSENT requirements section must not vacuously pass and + # claim "all requirements are specific" — functional requirements aren't + # optional, so their absence is itself a testability failure. + checks.append({ + "id": 6, + "category": "required", + "name": "Functional requirements are testable", + "passed": False, + "detail": "No requirements section found", + "points": 5, + }) + else: + vague_in_reqs = VAGUE_PATTERN.findall(reqs_section) + checks.append({ + "id": 6, + "category": "required", + "name": "Functional requirements are testable", + "passed": len(vague_in_reqs) == 0, + "detail": f"Vague terms found: {vague_in_reqs}" if vague_in_reqs else "All requirements are specific", + "points": 5, + }) # Check 7: Each requirement has priority (Must/Should/Could or P0/P1/P2) has_priority = bool(re.search( diff --git a/tests/core/test_cli.py b/tests/core/test_cli.py index 8bdbf74..f681a6f 100644 --- a/tests/core/test_cli.py +++ b/tests/core/test_cli.py @@ -390,3 +390,23 @@ def test_native_expand_and_rate_no_key_return_agent_action_json(tmp_path, monkey ) assert rate["ok"] is False assert rate["agent_action_required"]["op"] == "rate" + + +def test_cli_flags_have_discoverable_help(): + """Walkthrough fix: previously-undocumented flags must carry help text; + set-status --status must reveal its valid vocabulary.""" + from prd_taskmaster.cli import build_parser + p = build_parser() + subs = p._subparsers._group_actions[0].choices + + def flag_help(cmd, flag): + for a in subs[cmd]._actions: + if flag in a.option_strings: + return a.help + return None + + assert flag_help("set-status", "--status"), "set-status --status needs help" + assert "in-progress" in (flag_help("set-status", "--status") or ""), "status vocab must be discoverable" + assert flag_help("engine-preflight", "--no-configure"), "--no-configure needs help" + assert flag_help("next-task", "--tag"), "next-task --tag needs help" + assert flag_help("economy-report", "--input"), "economy-report --input needs help" diff --git a/tests/core/test_tournament_watcher.py b/tests/core/test_tournament_watcher.py index 74d79c7..87ecd16 100644 --- a/tests/core/test_tournament_watcher.py +++ b/tests/core/test_tournament_watcher.py @@ -323,6 +323,21 @@ def test_permit_blocks_on_abstained_winner(tmp_path): assert "ex-win" in out["abstained"] +def test_permit_reason_surfaces_winner_discrepancy_over_empty_slash(tmp_path): + # A cheating winner (recorded PASS, watcher DISCREPANCY) with NO to-be-slashed + # subs must surface the discrepancy in the reason, not a benign 'no to-be-slashed'. + ledger = tmp_path / "watcher.jsonl" + _seed_ledger(ledger, decisions=watcher.MIN_OBSERVATIONS, confirmed=watcher.MIN_OBSERVATIONS) + record = {"job_id": "job-1", "submissions": [ + _v("ex-win", agreement="DISCREPANCY", slash_grounds=True, recorded_passes_both=True), + ]} + out = watcher.permit_enforce_slash(record, ledger_path=ledger) + assert out["permitted"] is False + assert "ex-win" in out["discrepancies"] + assert "discrepanc" in out["reason"].lower() + assert "no to-be-slashed" not in out["reason"].lower() + + def test_permit_blocks_on_empty_to_slash(tmp_path): # No to-be-slashed submissions → never a blanket green-light. ledger = tmp_path / "watcher.jsonl" diff --git a/tests/core/test_validation.py b/tests/core/test_validation.py index a3a12ee..372860e 100644 --- a/tests/core/test_validation.py +++ b/tests/core/test_validation.py @@ -405,12 +405,23 @@ def test_validate_empty_prd(self, tmp_project): empty_prd.write_text("") out = run_validate_prd(str(empty_prd)) assert out["grade"] == "NEEDS_WORK" - # 3 checks pass vacuously on empty PRD: - # ch 5: no stories found → pass - # ch 6: no vague reqs → pass - # ch 10: no NFR section → pass - assert out["checks_passed"] == 3 - assert out["score"] == 13 # 5 + 5 + 3 + # 2 checks pass on absent-but-OPTIONAL sections; ch 6 (functional + # requirements) now FAILS-closed on an absent requirements section rather + # than vacuously claiming "all requirements are specific". + # ch 5: no stories found → pass (user stories optional) + # ch 10: no NFR section → pass (NFRs optional) + assert out["checks_passed"] == 2 + assert out["score"] == 8 # 5 (ch5) + 3 (ch10) + + def test_check6_fails_closed_on_absent_requirements(self, tmp_project): + """Check 6 must not vacuously pass (and assert specificity) when there is + no requirements section at all — functional requirements aren't optional.""" + prd = tmp_project / ".taskmaster" / "docs" / "prd.md" + prd.write_text("# PRD: Thing\n\n## Overview\nA thing.\n") # no requirements section + out = run_validate_prd(str(prd)) + check6 = next(c for c in out["checks"] if c["id"] == 6) + assert check6["passed"] is False + assert "no requirements" in check6["detail"].lower() def test_validate_grade_boundaries(self, tmp_project): """Verify grade boundary calculations match documented thresholds.""" From 38d34e37a2ad1b5a03e5c2c758578ed9e87b0ac8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 16:42:29 +0800 Subject: [PATCH 2/2] chore(walkthrough): make .i-walkthroughs/ local-only (owner decision) Add .i-walkthroughs/ to .gitignore and remove the tracked runlog store from the index (local files kept). The RAG runlogs stay on disk but are no longer committed. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 3 + .i-walkthroughs/README.md | 6 -- .../2026-06-17-cli-undocumented-flags.md | 57 ---------- ...6-17-cli-validate-prd-check6-false-pass.md | 58 ---------- ...26-06-17-cli-watcher-permit-reason-mask.md | 57 ---------- .i-walkthroughs/index.jsonl | 3 - .i-walkthroughs/schema.md | 100 ------------------ 7 files changed, 3 insertions(+), 281 deletions(-) delete mode 100644 .i-walkthroughs/README.md delete mode 100644 .i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md delete mode 100644 .i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md delete mode 100644 .i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md delete mode 100644 .i-walkthroughs/index.jsonl delete mode 100644 .i-walkthroughs/schema.md diff --git a/.gitignore b/.gitignore index cde10a2..9dccfa1 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,9 @@ SHARING_TEMPLATES.md .taskmaster/ .atlas-ai/ +# walkthrough RAG runlog store — local-only by owner decision (2026-06-17) +.i-walkthroughs/ + # Logs logs *.log diff --git a/.i-walkthroughs/README.md b/.i-walkthroughs/README.md deleted file mode 100644 index 38bd175..0000000 --- a/.i-walkthroughs/README.md +++ /dev/null @@ -1,6 +0,0 @@ -# Interactive-walkthrough runlogs - -RAG-ingestion-ready learning units from `/interactive-walkthrough` runs on this repo. -One file per (surface, issue) under `entries/`; `index.jsonl` is the append-only machine index; -`schema.md` is the single source of truth. Headless repo: "route" = a CLI/MCP surface -(e.g. `cli:validate-prd`), evidence = live CLI/test runs. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md b/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md deleted file mode 100644 index 260776f..0000000 --- a/.i-walkthroughs/entries/2026-06-17-cli-undocumented-flags.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -id: 2026-06-17-cli-undocumented-flags -schema_version: 1 -repo: anombyte93/prd-taskmaster -branch: walkthrough/atlas-cli-surfaces -commit: pending -date: 2026-06-17 -route: "cli:set-status" -screen_purpose: "Expose the engine's CLI flags clearly enough that a cold user can discover what each does — and what values it accepts — from -h alone." -mode: auto -viewport: cli -device_class: desktop -severity: P2 -category: interaction -root_cause: "Several argparse flags were added with no help= string; most damaging, set-status --status is a REQUIRED flag whose valid vocabulary (pending/in-progress/done/review/deferred/cancelled/blocked) appeared nowhere in help, SKILL.md, or the MCP tool." -assumption_trap: "the flag name is self-documenting (a required enum flag is not — the accepted values must be shown)." -reusable_rule: "Every CLI flag gets help text; a required enum flag must enumerate its accepted values in that help." -evidence_method: [code] -files_changed: ["prd_taskmaster/cli.py", "tests/core/test_cli.py"] -first_check: {result: fail, metric: "build_parser(): set-status --status, engine-preflight --no-configure, next-task --tag, economy-report --input all have help==None", evidence_ref: "pytest test_cli_flags_have_discoverable_help (pre-fix RED) + live set-status -h showing bare '--status STATUS'"} -second_check: {result: pass, metric: "all four flags have non-empty help; set-status --status help contains 'in-progress'", evidence_ref: "pytest (GREEN) + live: set-status -h now prints 'Task status; one of: pending, in-progress, done, review, deferred, cancelled, blocked'", regression_scan: pass} -regression_checks: ["set-status invalid-status error path unchanged (help-only, no argparse choices added → still returns FR-28 dict, not an argparse usage error)", "all existing CLI subprocess tests", "full offline suite"] -verdict: resolved -status: verified -approval: pending -evidence_dir: evidence/2026-06-17-cli-undocumented-flags -tags: [cli, discoverability, help-text, consistency, set-status] -related: [] -title: "Undocumented CLI flags — esp. set-status --status whose valid vocabulary was undiscoverable" ---- - -## Lesson (TL;DR) -Four CLI flags carried no help text; the worst was `set-status --status`, a REQUIRED flag whose accepted values were findable nowhere a user looks (`-h`, SKILL.md, or the MCP tool). A cold user had to read source to learn the status vocabulary. Every flag gets help; a required enum flag enumerates its values. - -## Screen & Purpose -Surfaces `cli:set-status`, `cli:engine-preflight`, `cli:next-task`, `cli:economy-report`. Soul purpose: a self-describing CLI a cold user can drive from `-h`. - -## Issue (first check) -`build_parser()` introspection: `set-status --status`, `engine-preflight --no-configure`, `next-task --tag`, `economy-report --input` all had `help is None`. Live `set-status -h` printed `--status STATUS` with no value list — the vocabulary (pending/in-progress/done/review/deferred/cancelled/blocked) was invisible. - -## Root Cause -Flags were added via `add_argument(...)` without `help=`. Inconsistent with sibling flags (e.g. `parse-prd --input`) that do carry help. - -## Fix -`cli.py`: add help strings to the four flags. For `set-status --status`, the help enumerates the valid status vocabulary. Help-only (NOT argparse `choices=`) so the engine's existing FR-28-safe status validation + error dict is preserved rather than replaced by an argparse usage error. - -## Second Check (re-verification, MANDATORY) -Post-fix: introspection shows all four flags with non-empty help; `set-status --status` help contains "in-progress"; live `set-status -h` prints the full status list. Regression scan: the invalid-status path still returns the engine's FR-28 error dict (no `choices=` added, so no behavior change to error handling); all existing CLI subprocess tests pass; full offline suite green. `regression_scan=pass`, `verdict=resolved`. - -## Reusable Rule -Every CLI flag gets help text; a required enum flag must enumerate its accepted values in that help. - -## Decision Trail -Owner: walkthrough auto-mode; real owner approves via PR. Rejected: adding argparse `choices=` for `--status` (would move validation to the parse layer and emit an argparse usage error instead of the engine's structured FR-28 dict — a contract change). Deferred: also documenting the status vocabulary in SKILL.md + the MCP tool description (follow-up; this entry fixes the CLI surface). - -## Revisions -- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md b/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md deleted file mode 100644 index 548cebd..0000000 --- a/.i-walkthroughs/entries/2026-06-17-cli-validate-prd-check6-false-pass.md +++ /dev/null @@ -1,58 +0,0 @@ ---- -id: 2026-06-17-cli-validate-prd-check6-false-pass -schema_version: 1 -repo: anombyte93/prd-taskmaster -branch: walkthrough/atlas-cli-surfaces -commit: pending -date: 2026-06-17 -route: "cli:validate-prd" -screen_purpose: "Give a user a fast, honest, point-scored verdict on whether a PRD is complete enough to parse into a trustworthy task graph." -mode: auto -viewport: cli -device_class: desktop -severity: P2 -category: trust -root_cause: "Check 6 ran VAGUE_PATTERN.findall over an empty string when no requirements section existed; an empty match list read as 'no vague terms' → passed:true + 'All requirements are specific'." -assumption_trap: "no-vague-terms == requirements-are-testable (true only when requirements actually exist)." -reusable_rule: "A presence/quality check over an ABSENT section must fail-closed, never pass-and-assert-quality on emptiness." -evidence_method: [code] -files_changed: ["prd_taskmaster/validation.py", "tests/core/test_validation.py"] -first_check: {result: fail, metric: "empty PRD → check6.passed=true, detail='All requirements are specific', checks_passed=3, score=13", evidence_ref: "live: python3 script.py validate-prd --input /tmp/wt_empty.md (pre-fix)"} -second_check: {result: pass, metric: "empty PRD → check6.passed=false, detail='No requirements section found', checks_passed=2, score=8, grade=NEEDS_WORK", evidence_ref: "live post-fix run + pytest -k 'validate_empty_prd or check6_fails_closed'", regression_scan: pass} -regression_checks: ["sample_prd (real requirements) still passes check6", "thorough PRD still 57/57 EXCELLENT exit 0", "grade boundary tests", "full offline suite tests/core+tests/plugin"] -verdict: resolved -status: verified -approval: pending -evidence_dir: evidence/2026-06-17-cli-validate-prd-check6-false-pass -tags: [validate-prd, honesty, fail-closed, vacuous-pass] -related: [] -title: "validate-prd Check 6 falsely passed + claimed specificity on a PRD with zero requirements" ---- - -## Lesson (TL;DR) -`validate-prd` Check 6 ("Functional requirements are testable") passed AND printed "All requirements are specific" on a PRD that had no requirements section at all — a positively false claim about content that does not exist. Root cause: a vague-term scan over an empty string returns no matches, which the code read as success. A presence/quality check over an absent section must fail-closed. - -## Screen & Purpose -Surface `cli:validate-prd` (13-check PRD quality gate, `validation.py:run_validate_prd`). Soul purpose: an honest, point-scored verdict on PRD completeness. Headless/CLI; evidence = live runs + unit tests. - -## Issue (first check) -Live, pre-fix, on a PRD containing only a title + Overview (zero requirements): -`check 6 → passed:true, detail:"All requirements are specific"`; `checks_passed:3, score:13`. The detail makes an affirmative specificity claim about requirements that are absent. (The final grade NEEDS_WORK was already correct; the defect is the per-check honesty + an inflated checks_passed.) - -## Root Cause -`reqs_section` was empty; `VAGUE_PATTERN.findall("")==[]` → `passed = len([])==0 = True` and the else-branch detail "All requirements are specific" fired. The check never distinguished "present and specific" from "absent". - -## Fix -`validation.py`: when `reqs_section.strip()` is empty, emit `passed:false, detail:"No requirements section found"` (fail-closed); otherwise the original vague-term logic. Smallest coherent change; checks 5 & 10 (user-stories / NFRs are genuinely OPTIONAL sections) left intentionally untouched. - -## Second Check (re-verification, MANDATORY) -Same empty PRD, post-fix: `check 6 → passed:false, detail:"No requirements section found"`, `checks_passed:2, score:8, grade:NEEDS_WORK`. Same-metric assertion now passes. Regression scan: `sample_prd` (real requirements) still passes check 6; a thorough PRD still scores 57/57 EXCELLENT (exit 0); grade-boundary tests green; full offline suite green. `regression_scan=pass`, `verdict=resolved`. - -## Reusable Rule -A presence/quality check over an ABSENT section must fail-closed — never pass-and-assert-quality on emptiness. - -## Decision Trail -Owner: walkthrough auto-mode (Hayden stand-in); real owner approves via the PR. Rejected: only rewording the detail while keeping `passed:true` (would leave the vacuous pass + inflated count). Deferred: checks 5 & 10 pass-on-absent (P3; those sections are legitimately optional) — recorded for a future decision. - -## Revisions -- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md b/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md deleted file mode 100644 index 251b393..0000000 --- a/.i-walkthroughs/entries/2026-06-17-cli-watcher-permit-reason-mask.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -id: 2026-06-17-cli-watcher-permit-reason-mask -schema_version: 1 -repo: anombyte93/prd-taskmaster -branch: walkthrough/atlas-cli-surfaces -commit: pending -date: 2026-06-17 -route: "cli:watcher-run" -screen_purpose: "Let an operator see the fail-closed real-slash permit for a job and understand WHY it is or isn't permitted." -mode: auto -viewport: cli -device_class: desktop -severity: P2 -category: trust -root_cause: "The permit reason ladder tested `not to_slash` before `discrepancies`, so a job with no to-be-slashed submissions but a cheating-winner DISCREPANCY reported the benign 'no to-be-slashed' message instead of the discrepancy." -assumption_trap: "empty to-be-slashed set == nothing interesting happened (ignores discrepancies/abstains on the winners)." -reusable_rule: "Order a fail-closed reason ladder most-serious-finding first; never let a benign branch shadow a security-relevant one." -evidence_method: [code] -files_changed: ["prd_taskmaster/tournament/watcher.py", "tests/core/test_tournament_watcher.py"] -first_check: {result: fail, metric: "permit_enforce_slash(record with empty to_slash + cheating-winner DISCREPANCY).reason == 'blocked: no to-be-slashed submissions to confirm' (discrepancy masked)", evidence_ref: "pytest test_permit_reason_surfaces_winner_discrepancy_over_empty_slash (pre-fix RED)"} -second_check: {result: pass, metric: "same record → reason contains 'discrepancy' and NOT 'no to-be-slashed'; permitted still False; ex-win in discrepancies", evidence_ref: "pytest (post-fix GREEN)", regression_scan: pass} -regression_checks: ["existing permit reason tests (discrepancy-among-slashes, abstained-winner, empty-to_slash, mixed-batch) unchanged", "permitted boolean decision unchanged across all permit tests", "full offline suite"] -verdict: resolved -status: verified -approval: pending -evidence_dir: evidence/2026-06-17-cli-watcher-permit-reason-mask -tags: [watcher, slash-gate, fail-closed, legibility, self-review] -related: [atlas-watcher-built] -title: "watcher permit reason masked a caught cheating-winner discrepancy with a benign 'no to-be-slashed' message" ---- - -## Lesson (TL;DR) -The watcher's real-slash permit correctly BLOCKED a job where a winner was an independent discrepancy, but its human-readable `reason` said "no to-be-slashed submissions to confirm" — hiding the very thing the operator most needs to see. The boolean was right; the explanation lied by omission. Order the reason ladder most-serious-first. - -## Screen & Purpose -Surface `cli:watcher-run` → `watcher.permit_enforce_slash` (the fail-closed gate that decides whether real `--enforce-slash` forfeiture may proceed). Soul purpose: an operator-legible, honest permit verdict. - -## Issue (first check) -A record with zero to-be-slashed submissions but a cheating winner (recorded PASS, watcher DISCREPANCY) returned `permitted:false` (correct) with `reason:"blocked: no to-be-slashed submissions to confirm"` (wrong — the discrepancy was in `out["discrepancies"]` but never surfaced in the headline reason). `to_slash` is empty because the winner recorded PASS, so the `elif not to_slash` branch fired before `elif discrepancies`. - -## Root Cause -Reason-ladder ordering: `permitted → not to_slash → discrepancies → abstained → track_record`. The benign empty-set branch shadowed the security-relevant discrepancy branch. - -## Fix -`watcher.py`: reorder the ladder to `permitted → discrepancies → abstained → not to_slash → track_record`. The `permitted` boolean is unchanged (already `not discrepancies and not abstained and ...`); only the explanatory string ordering changed. - -## Second Check (re-verification, MANDATORY) -Post-fix, same record: `reason` contains "discrepancy", does NOT contain "no to-be-slashed", `permitted` still False, `ex-win` in `discrepancies`. Regression scan: all prior permit tests (discrepancy-among-slashes, abstained-winner, empty-to_slash, mixed-batch, concordance boundary) unchanged; every `permitted` boolean identical to before. Full offline suite green. `regression_scan=pass`, `verdict=resolved`. - -## Reusable Rule -Order a fail-closed reason ladder most-serious-finding first; a benign branch must never shadow a security-relevant one. - -## Decision Trail -Owner: walkthrough auto-mode; real owner approves via PR. This fix touches code authored earlier this same session — a good reminder that the adversarial surface review caught a legibility gap the build-time tests didn't assert. No alternatives rejected; the reorder is the minimal correct change. - -## Revisions -- 2026-06-17: initial fix; second check passed on first attempt. diff --git a/.i-walkthroughs/index.jsonl b/.i-walkthroughs/index.jsonl deleted file mode 100644 index 5f21453..0000000 --- a/.i-walkthroughs/index.jsonl +++ /dev/null @@ -1,3 +0,0 @@ -{"id":"2026-06-17-cli-validate-prd-check6-false-pass","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:validate-prd","date":"2026-06-17","severity":"P2","category":"trust","verdict":"resolved","status":"verified","title":"validate-prd Check 6 falsely passed + claimed specificity on a PRD with zero requirements","file":"entries/2026-06-17-cli-validate-prd-check6-false-pass.md"} -{"id":"2026-06-17-cli-watcher-permit-reason-mask","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:watcher-run","date":"2026-06-17","severity":"P2","category":"trust","verdict":"resolved","status":"verified","title":"watcher permit reason masked a caught cheating-winner discrepancy","file":"entries/2026-06-17-cli-watcher-permit-reason-mask.md"} -{"id":"2026-06-17-cli-undocumented-flags","repo":"anombyte93/prd-taskmaster","branch":"walkthrough/atlas-cli-surfaces","route":"cli:set-status","date":"2026-06-17","severity":"P2","category":"interaction","verdict":"resolved","status":"verified","title":"Undocumented CLI flags incl. set-status --status vocabulary","file":"entries/2026-06-17-cli-undocumented-flags.md"} diff --git a/.i-walkthroughs/schema.md b/.i-walkthroughs/schema.md deleted file mode 100644 index ced26a9..0000000 --- a/.i-walkthroughs/schema.md +++ /dev/null @@ -1,100 +0,0 @@ -# `.i-walkthroughs/` schema (v1) - -> **Skill template.** This is the canonical copy the `interactive-walkthrough` skill carries. -> On first use in a repo that has no `.i-walkthroughs/`, copy this file to -> `/.i-walkthroughs/schema.md` (then create `README.md`, an empty `index.jsonl`, -> `entries/`, and `evidence/`) and commit, so the Runlog gate's "per `.i-walkthroughs/schema.md`" -> always resolves against a repo-local source of truth. - -Canonical field dictionary for walkthrough runlog entries. A future repo-wide RAG ingester -reads THIS file. Stable contract: glob `**/.i-walkthroughs/entries/*.md`, parse YAML -frontmatter as the metadata record, treat each `##` body heading as a chunk boundary, upsert -keyed on `id` (== filename stem), gate migrations on `schema_version`. - -## Folder layout - -``` -.i-walkthroughs/ - README.md # what this is + RAG-forward note - schema.md # THIS file — single source of truth - index.jsonl # append-only machine index, one JSON object per entry - entries/ - .md # one self-contained learning unit per file - evidence/ - / # per-entry assets (id == entry id) - before-.png # pre-fix VIEWPORT screenshot - after-.png # post-fix (second-check) VIEWPORT screenshot - dom-measure.json # {"before":{...},"after":{...}} authoritative numbers -``` - -**File strategy:** one file per `(screen, issue)` learning unit (NOT an append-to-one-log), -because a RAG hit must return a complete, single-issue document with correct metadata so -filters like `verdict:resolved AND severity:P1` select exactly the right unit. `index.jsonl` -is the fast non-vector path to enumerate/pre-filter the corpus without parsing markdown. - -**`` == filename stem == frontmatter `id` == `index.jsonl` row key.** Never reused, never changed. - -## Slug / id rules - -`` = `--` -- `route-slug`: route with `/` → `-`, leading `-` stripped; root `/` → `root`. -- `issue-slug`: 2–4 word kebab summary of the defect. -- lowercase ASCII, no spaces. Same-day collision → append `-2`, `-3`. - -## Frontmatter fields - -| key | type | purpose | -|---|---|---| -| `id` | string (== filename stem) | Primary key for RAG upsert/dedup + `index.jsonl` join + git anchor. Immutable. | -| `schema_version` | int (start 1) | Migration gate for field renames. | -| `repo` | string `owner/name` | Repo-wide RAG filter dimension (this repo's `owner/name`, e.g. `anombyte93/atlas-ai-website`). | -| `branch` | string | Branch the fix landed on. | -| `commit` | git SHA \| `pending` | Hard link to the resolving diff; `pending` until committed. | -| `date` | ISO date `YYYY-MM-DD` | Day the second check passed; filename prefix + index sort key. | -| `route` | string app route | Primary filter: "everything learned about `/calculator`". Leading slash; root `/`. | -| `screen_purpose` | string (one sentence) | The screen soul purpose; anchors the embedding to intended behavior. | -| `mode` | enum `interactive` \| `auto` \| `mobile` | Which walkthrough mode produced this. | -| `viewport` | string `WxH` or `WxH@dpr` | Reproduction context (e.g. `390x844`). | -| `device_class` | enum `mobile` \| `tablet` \| `desktop` | Coarse class for filtering + regression arm. | -| `severity` | enum `P0` \| `P1` \| `P2` \| `P3` | Reuses `reference/auto-report-schema.md` labels verbatim. | -| `category` | enum `layout` \| `copy` \| `a11y` \| `perf` \| `security` \| `interaction` \| `data` \| `trust` \| `role-boundary` | Defect taxonomy for clustering. | -| `root_cause` | string (one line) | The why (NOT the symptom). High-value learn-from-mistakes field. | -| `assumption_trap` | string \| null | The false belief made/nearly made (e.g. "desktop pass == mobile pass"). Drives `reusable_rule`. | -| `reusable_rule` | string (one line, imperative) | Portable rule a future agent applies to avoid repeating this. Highest-signal retrieval target. | -| `evidence_method` | list[enum `dom-measure` \| `viewport-screenshot` \| `console` \| `network` \| `axe` \| `lighthouse` \| `code`] | How proof was gathered, ordered by authority (`dom-measure` first). | -| `files_changed` | list[string] repo-relative | Source files the fix touched. | -| `first_check` | object `{result: fail\|pass, metric, evidence_ref}` | Pre-fix proof the issue was real. Required for P0/P1. | -| `second_check` | object `{result: pass\|fail, metric, evidence_ref, regression_scan: pass\|fail}` | MANDATORY post-fix re-verification. `result=pass` ONLY if same metric now passes AND `regression_scan=pass`. `metric` must be the SAME numeric assertion as `first_check.metric`. | -| `regression_checks` | list[string] | The specific adjacent things re-verified (other viewport class, overflow, focus, console/CSP). Backs `second_check.regression_scan`. | -| `verdict` | enum `resolved` \| `partial` \| `regressed` \| `reverted` | Headline outcome. `resolved`=gone + no regression; `partial`=residual remains; `regressed`=fix broke something; `reverted`=rolled back. | -| `status` | enum `open` \| `verified` \| `shipped` | Lifecycle, orthogonal to verdict. `open`=no passing second check yet; `verified`=second check passed; `shipped`=committed/merged. | -| `approval` | string \| `pending` | Who approved the mutating action (owner handle + when + channel). In `--mobile` this is the Discord reply. | -| `evidence_dir` | string repo-relative | `evidence//` — keeps binaries out of embedded text but linkable. | -| `tags` | list[string] kebab | Free-form RAG facets beyond the fixed enums. | -| `related` | list[string] entry ids | Graph edges (recurrence → prior resolved entry, supersedes, caused-by). | -| `title` | string (short headline) | Display + embedding-friendly summary; also `index.jsonl` `title`. | - -## Body sections (in order; each `##` is a chunk boundary) - -1. **Lesson (TL;DR)** — standalone 2–4 sentences; restates `root_cause` + `reusable_rule` in prose. Lead chunk. -2. **Screen & Purpose** — route, role/context, viewport, soul purpose. -3. **Issue (first check)** — observable QUANTITATIVE symptom; DOM numbers first, then screenshot ref. Never an opinion. -4. **Root Cause** — underlying cause + the `assumption_trap` narrative. -5. **Fix** — smallest coherent change, `files_changed`, load-bearing snippet, commit. -6. **Second Check (re-verification, MANDATORY)** — fresh same-screen/same-viewport evidence: (a) same-defect proof via the same metric, (b) explicit regression scan, then the verdict. -7. **Reusable Rule** — the portable rule, imperative. -8. **Decision Trail** — owner decision + channel, rejected alternatives, deferred items, open questions. -9. **Revisions** — append-only log of in-turn updates (e.g. a regressed second check → re-fix). Keeps the full mistake arc in one unit. - -## Re-touch rule - -- In-turn correction (a fix regresses on its own second check this turn) → **update the same entry** (Revisions section; verdict flips). -- A separate later walkthrough re-touching the same screen → **new dated entry** with `related` linking back. The corpus accumulates `mistake → fix → second-check` triples over time. - -## `index.jsonl` row shape - -One JSON object per line, append-only (never rewrite prior lines): - -```json -{"id":"...","repo":"owner/name","branch":"...","route":"/...","date":"YYYY-MM-DD","severity":"P0|P1|P2|P3","category":"...","verdict":"resolved|partial|regressed|reverted","status":"open|verified|shipped","title":"...","file":"entries/.md"} -```