Skip to content

Commit a6300bf

Browse files
petemooreclaude
andcommitted
docs: tick Phase 1 (PR #1) and Phase 2 (PR #2) in WORKPLAN
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 24d7ba7 commit a6300bf

1 file changed

Lines changed: 27 additions & 27 deletions

File tree

docs/WORKPLAN.md

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,17 @@ repo). Tick the phase when its items below are done.
7878
Confirm T9 on #193; run the empty-commit-PR test. See *Validation experiments* below.
7979
**Done 2026-06-15** — both confirmed; T9 confirmed with a richer-than-expected picture (a
8080
`-X theirs` rebase-pollution component, flagged in `docs/questions.md`).
81-
- [ ] **Phase 1 — Safe independent fixes** (low risk, no design dependencies). T4 stale comment;
82-
T6/T6a **reporting-noise fix only** (no-op cycle records nothing) — the transition guard is
83-
deferred to Phase 3 per review C2; dead fields (`ReviewVerdict.Summary`, `BudgetSpent`).
84-
- [ ] **Phase 2 — T9 narrow fix** (squash base = post-rebase branch tip; **store the SHA on a
85-
struct field** so Q13's curate and the reviewer-diff can reuse it — review M4; **also return it
86-
in `RunResult`** so the orchestrator's gave_up path can record the outcome against it, not the
87-
scan-time `pr.HeadSHA` — review N4/MAJOR-1; leave all other `recordOutcome` calls on `pr.HeadSHA`).
88-
Small but serious.
81+
- [x] **Phase 1 — Safe independent fixes** (low risk, no design dependencies). **PR #1 (draft).**
82+
T4 stale comment ✓; T6/T6a **reporting-noise fix only** (no-op cycle records nothing) ✓ — the
83+
transition guard is deferred to Phase 3 per review C2. Dead fields (`ReviewVerdict.Summary`,
84+
`BudgetSpent`) **deliberately deferred** to their owning phases (Summary → Q15/Phase 3.8 replaces
85+
it; BudgetSpent → billing backlog) rather than removed-then-re-added.
86+
- [x] **Phase 2 — T9 narrow fix** (squash base = post-rebase branch tip ✓; **stored on a struct
87+
field** `Pipeline.bumpTipSHA` reused by the squash and the reviewer-diff — review M4 ✓; **also
88+
returned in `RunResult.TipSHA`** and the orchestrator's gave_up path records the outcome (+ sticky
89+
comment marker) against it via `terminalSHA()` — review N4/MAJOR-1 ✓; all other `recordOutcome`
90+
calls left on `pr.HeadSHA`). **PR #2 (draft).** ⚠️ Phase-0 surfaced a `-X theirs` rebase-pollution
91+
component this narrow fix doesn't fully clean — see `docs/questions.md`.
8992
- [ ] **Phase 3 — Core rework** (large, interrelated; sequence as its own PRs in this order):
9093
0. Reconcile `spec.go` to the post-Q10 state machine FIRST (review C2), so the transition guard
9194
is authored against the final graph.
@@ -123,25 +126,22 @@ supporting features); Phases 4–5 are polish and durable documentation.
123126

124127
## Confirmed bugs
125128

126-
- [ ] **T9 — Squash bundles unrelated `main` changes into the "fix" commit.** Squash resets to
127-
the stale scan-time `pr.HeadSHA` after a Phase-0 rebase. **Fix (decided):** reset to the branch
128-
tip captured right after `checkout -b branch pr.HeadRef`, before the agent runs. (Q11 kept
129-
inherit-and-rebase, so this narrow fix is the chosen one.) **Store the SHA on a struct field**
130-
(review M4) — it's the single canonical base for the squash, the Q13 curate, AND the reviewer
131-
diff (which currently uses the ref *name* `origin/<HeadRef>`, same stale-ref hazard). Serious.
132-
Confirm empirically on petemoore/taskcluster #193 (`c1671f49`): bloated commit's parent should
133-
equal the pre-rebase head.
134-
- [ ] **T6 / T6a — Decision graph not enforced + idempotent skips pollute history. (Q8 → c: both,
135-
SPLIT across phases — review C2.)** (1) **Phase 1:** reporting-noise fix — a no-op cycle records
136-
**nothing**: remove the `pending` re-stamp in `Run`'s pre-populate loop and at the top of
137-
`processPR`, **and the skip-path terminal re-stamp** (`orchestrator.go:347-357` — review N1);
138-
the dashboard reads the stored stage from the row. (No `finalized→finalized` self-loop is then
139-
emitted, so the guard needs no self-loop exception.) (2) **Phase 3 (after spec.go reconciled):**
140-
runtime guard — `Report(stage)` validates against the post-Q10 graph, rejects/loud-logs illegal
141-
transitions; collapse non-stage nodes following **both decision AND back edges** (review N2 — or
142-
it rejects legal resume loops). Cost-safety-critical.
143-
- [ ] **T4 — Stale `spec.go` comment** on the `gave_up` node still claims it routes to
144-
`flagged_human` (Bug #24 era). Behaviour is correct; just fix the comment.
129+
- [x] **T9 — Squash bundles unrelated `main` changes into the "fix" commit.** **Done — PR #2
130+
(draft).** Reset base is now the branch tip captured right after `checkout -b branch pr.HeadRef`
131+
(`Pipeline.bumpTipSHA`), reused by the squash AND the reviewer diff/commit listing (was the stale
132+
ref name `origin/<HeadRef>` — MINOR-1). Q11 kept inherit-and-rebase. Confirmed empirically on
133+
petemoore/taskcluster #193 in Phase 0 (the "fix" commit bundled 300 files). The squash-base fix
134+
is validated by a git-integration test (`TestSquashBranchUsesCapturedTipNotStaleBase`).
135+
- [~] **T6 / T6a — Decision graph not enforced + idempotent skips pollute history. (Q8 → c: both,
136+
SPLIT across phases — review C2.)** (1) **Phase 1 — DONE, PR #1:** reporting-noise fix — a no-op
137+
cycle records **nothing**: `prepopulate()` stamps `pending` only for unseen PRs (preserving row
138+
creation), and the `processPR`-top + skip-path re-stamps are removed; the dashboard reads the
139+
stored stage from the row. (No `finalized→finalized` self-loop is then emitted.) (2) **Phase 3
140+
(after spec.go reconciled) — TODO:** runtime guard — `Report(stage)` validates against the
141+
post-Q10 graph, rejects/loud-logs illegal transitions; collapse non-stage nodes following
142+
**both decision AND back edges** (review N2). Cost-safety-critical.
143+
- [x] **T4 — Stale `spec.go` comment** on the `gave_up` node. **Done — PR #1.** Comment now
144+
describes the correct current behaviour (records `gave_up` sticky at the head SHA).
145145
- [ ] **T12 — Re-ingestion risk (Q14 → DECIDED):** exclude the tool's own PRs via a DB record of
146146
what it created (never a branch-name/title heuristic — branch names are attacker-spoofable).
147147
⚠️ **Must be a reap-exempt table (review C1)** — NOT a `pr_progress` column, which `Store.Reap`

0 commit comments

Comments
 (0)