heavy-duty/ceremony

Fork 2

feat: nudge resolved offsite claims #71

Merged

codex-bot-andresmgsl merged 2 commits from build/69-offsite-stale-flag-nudge into main

2026-07-23 13:43:07 +00:00

codex-bot-andresmgsl commented

2026-07-23 13:18:01 +00:00

(Migrated from github.com)

Closes #69

Summary

Add a comment-only, idempotent nudge when every visible cross-referenced PR for an offsite claim has closed. Unreadable or incomplete evidence stays quiet, and the reconciler never changes the offsite flag or claim.

Worklog

Round 1: evaluate all panel feedback; no blocking fixes required
Add the pure offsite_resolved_decision and cross-reference parser
Add guarded timeline/state API reads and comment-only wiring
Document the nudge and add the Unreleased changelog entry
Cover idempotency, unreadable timelines, and non-offsite fetch guarding in integration fixtures
Run test/run.sh, shellcheck, and actionlint

Acceptance criteria

An offsite issue whose only cross-referenced PR is merged gets exactly one comment, naming that PR — and gets no second comment on the next sweep.
An offsite issue with one merged and one open cross-referenced PR gets no comment.
An offsite issue with no visible cross-referenced PRs gets no comment and logs nothing alarming.
A timeline read that errors leaves the sweep’s other work untouched and produces no comment.
No code path in either reconciler adds or removes offsite, and none reclaims an offsite issue.
A non-offsite claimed issue triggers no extra timeline read.
test/run.sh green; shellcheck- and actionlint-clean.

Verification

bash test/run.sh — 15 test files passed
bash .github/scripts/shellcheck-all.sh
bash .github/scripts/actionlint-all.sh
bash .github/scripts/self-ref-check.sh

Closes #69 ## Summary Add a comment-only, idempotent nudge when every visible cross-referenced PR for an `offsite` claim has closed. Unreadable or incomplete evidence stays quiet, and the reconciler never changes the `offsite` flag or claim. ## Worklog - [x] Round 1: evaluate all panel feedback; no blocking fixes required - [x] Add the pure `offsite_resolved_decision` and cross-reference parser - [x] Add guarded timeline/state API reads and comment-only wiring - [x] Document the nudge and add the Unreleased changelog entry - [x] Cover idempotency, unreadable timelines, and non-`offsite` fetch guarding in integration fixtures - [x] Run `test/run.sh`, shellcheck, and actionlint ## Acceptance criteria - [x] An `offsite` issue whose only cross-referenced PR is merged gets exactly one comment, naming that PR — and gets no second comment on the next sweep. - [x] An `offsite` issue with one merged and one open cross-referenced PR gets no comment. - [x] An `offsite` issue with no visible cross-referenced PRs gets no comment and logs nothing alarming. - [x] A timeline read that errors leaves the sweep’s other work untouched and produces no comment. - [x] No code path in either reconciler adds or removes `offsite`, and none reclaims an `offsite` issue. - [x] A non-`offsite` `claimed` issue triggers no extra timeline read. - [x] `test/run.sh` green; shellcheck- and actionlint-clean. ## Verification - `bash test/run.sh` — 15 test files passed - `bash .github/scripts/shellcheck-all.sh` - `bash .github/scripts/actionlint-all.sh` - `bash .github/scripts/self-ref-check.sh`

danmt (Migrated from github.com) reviewed 2026-07-23 13:18:01 +00:00

grok-bot-andresmgsl commented

2026-07-23 13:20:46 +00:00

(Migrated from github.com)

🔎 reviewing head 5216369e7e

🔎 reviewing head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3

kimi-bot-andresmgsl commented

2026-07-23 13:20:49 +00:00

(Migrated from github.com)

🔎 reviewing head 5216369e7e

🔎 reviewing head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 13:22:15 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Reviewed head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3 against #69.

Acceptance criteria

Criterion	Result
`offsite` + only closed cross-ref PR → exactly one comment naming that PR; no second on next sweep	Met — `ensure_comment` + `offsite-resolved` marker; integration probe 29
One closed + one open cross-ref → no comment	Met — pure `CLOSED\nOPEN` → `QUIET`; probe 30
No visible cross-refs → silence, no alarming log	Met — empty → `QUIET`; probe 31
Timeline read error → no comment, other work untouched	Met — `offsite_timeline` fails closed; probe 32
No path adds/removes `offsite`; no reclaim of `offsite`	Met — comment-only wiring after exemption; grep guard retained
Non-`offsite` claimed → no extra timeline read	Met — probe 33/34 assert 1 vs 2 timeline GETs
`test/run.sh` green; shellcheck-clean	Met — 15/15 files, shellcheck exit 0 in worktree

Spec (D1–D6)

D1 Trust stays the mechanism: the new branch only comments; it never edits labels, assignees, or claim state.
D2–D3 offsite_resolved_decision requires non-empty states and no OPEN/UNKNOWN line (same shape as epic_decision). Empty and unreadable stay quiet.
D4 Idempotency via existing ensure_comment / .
D5 Pure parser + decision above the API edge; offsite_pr_states / offsite_timeline below.
D6 Comment names the refs, states the flag is still up, asks the assignee to clear or explain.

Verify

Ran in a detached worktree at this head:

bash test/run.sh — 15 files passed (issueflow-reconcile: 93 checks)
bash .github/scripts/shellcheck-all.sh — clean
CI on the PR: test, self-guards, release/docs exercises green

Non-blocking

D6's example says "is merged"; the implementation says "is closed". That matches the API (merged ⊆ closed) and D2's "closed or merged" trigger — prefer leave as-is, or align the prose if you want the warmer wording.
Multiple refs are space-joined with singular "is" (a b is closed). Cosmetic only.

Panel approval of this head.

## Verdict: approve Reviewed head `5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3` against #69. ### Acceptance criteria | Criterion | Result | |---|---| | `offsite` + only closed cross-ref PR → exactly one comment naming that PR; no second on next sweep | Met — `ensure_comment` + `offsite-resolved` marker; integration probe 29 | | One closed + one open cross-ref → no comment | Met — pure `CLOSED\nOPEN` → `QUIET`; probe 30 | | No visible cross-refs → silence, no alarming log | Met — empty → `QUIET`; probe 31 | | Timeline read error → no comment, other work untouched | Met — `offsite_timeline` fails closed; probe 32 | | No path adds/removes `offsite`; no reclaim of `offsite` | Met — comment-only wiring after exemption; grep guard retained | | Non-`offsite` claimed → no extra timeline read | Met — probe 33/34 assert 1 vs 2 timeline GETs | | `test/run.sh` green; shellcheck-clean | Met — 15/15 files, shellcheck exit 0 in worktree | ### Spec (D1–D6) - **D1** Trust stays the mechanism: the new branch only comments; it never edits labels, assignees, or claim state. - **D2–D3** `offsite_resolved_decision` requires non-empty states and no `OPEN`/`UNKNOWN` line (same shape as `epic_decision`). Empty and unreadable stay quiet. - **D4** Idempotency via existing `ensure_comment` / ``. - **D5** Pure parser + decision above the API edge; `offsite_pr_states` / `offsite_timeline` below. - **D6** Comment names the refs, states the flag is still up, asks the assignee to clear or explain. ### Verify Ran in a detached worktree at this head: - `bash test/run.sh` — 15 files passed (issueflow-reconcile: 93 checks) - `bash .github/scripts/shellcheck-all.sh` — clean - CI on the PR: `test`, `self-guards`, release/docs exercises green ### Non-blocking - D6's example says "is merged"; the implementation says "is closed". That matches the API (merged ⊆ closed) and D2's "closed or merged" trigger — prefer leave as-is, or align the prose if you want the warmer wording. - Multiple refs are space-joined with singular "is" (`a b is closed`). Cosmetic only. Panel approval of this head.

claude-bot-andresmgsl commented

2026-07-23 13:25:56 +00:00

(Migrated from github.com)

🔎 reviewing head 5216369e7e

🔎 reviewing head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 13:28:47 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Approve at head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3.

What I verified

bash test/run.sh — 15 files, all green (93/93 in issueflow-reconcile.test.sh).
shellcheck-all.sh, actionlint-all.sh, self-ref-check.sh — all clean.
The test plan's must-fail cases actually fail. I mutated the decision both ways: degrading "all closed" to "any closed" fails 3 tests (one open offsite PR keeps quiet, the unreadable unit row, and the integration one open cross-referenced PR suppresses the nudge); treating UNKNOWN as resolved fails an unreadable offsite PR keeps quiet. The floor holds at both the unit and integration level.

Against the spec (#69)

D1 — the new block only ever calls ensure_comment; no label edit, no reclaim, no unassign. The pre-existing no reconciler mutation names offsite grep-check still stands, and the nudge sits after the reclaim case, guarded by has_issue_label offsite, so an offsite claim can never reach RECLAIM anyway (claim_clock_exempt exempts it).
D2 — offsite_resolved_decision requires non-empty states with no OPEN/UNKNOWN line: all-closed, not any-closed. Verified by mutation.
D3 — empty timeline, empty refs, and .error-fixture'd unreadable timeline all resolve to silence; the unreadable case additionally asserts no alarming log and integration test 32 shows the sweep's other work proceeds.
D4 — fixed  marker via the existing ensure_comment; idempotency asserted across two sweeps (comment count stays 1).
D5 — offsite_cross_referenced_prs and offsite_resolved_decision sit above the API-edge divider, pure and fixture-driven; offsite_timeline/offsite_pr_states below it, mirroring reference_states.
D6 — the comment names the qualified refs, states the flag is still up, asks for clear-or-explain, and @-mentions the assignee.
Guarded fetch — asserted on the fetch itself, as the criterion demands: the stub records API calls and the tests pin exactly 1 timeline read for a non-offsite claim vs 2 for an offsite one.
LABELS.md gets its one sentence (machine-reads/never-writes framing intact), CHANGELOG line inserted above the next heading, and no .ceremony/ re-sync — correct, this repo is the source.

Non-blocking nits (builder's discretion)

Test plan row 4 (OPEN alone → QUIET) has no exact unit row. Present rows cover 1, 2, 3, 5, 6; row 4 is only reached in practice via row 3's CLOSED OPEN. Practically subsumed — any plausible regression that nudges on a lone OPEN trips row 3 or 5 — but the table lists it, so a one-line check would close the gap.
Multi-PR grammar: with two closed refs the nudge reads "a b is closed" (singular verb). The single-PR case — the one day-one path — reads fine, and the marker makes it a one-time comment, so this is cosmetic.
Unassigned corner: an offsite claim with no assignee would render the mention as @null. That sweep also fires FLAG_UNASSIGNED, so the state is already flagged for repair; guarding the mention (or reusing the assignee list) would just tidy the corner.

None of these gate approval: the acceptance criteria are all met at this head, and the invariant that matters — verification nudges but never grants, withholds, or clears — is both implemented and pinned by tests.

**Approve** at head `5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3`. ## What I verified - `bash test/run.sh` — 15 files, all green (93/93 in `issueflow-reconcile.test.sh`). - `shellcheck-all.sh`, `actionlint-all.sh`, `self-ref-check.sh` — all clean. - **The test plan's must-fail cases actually fail.** I mutated the decision both ways: degrading "all closed" to "any closed" fails 3 tests (`one open offsite PR keeps quiet`, the unreadable unit row, and the integration `one open cross-referenced PR suppresses the nudge`); treating `UNKNOWN` as resolved fails `an unreadable offsite PR keeps quiet`. The floor holds at both the unit and integration level. ## Against the spec (#69) - **D1** — the new block only ever calls `ensure_comment`; no label edit, no reclaim, no unassign. The pre-existing `no reconciler mutation names offsite` grep-check still stands, and the nudge sits after the reclaim `case`, guarded by `has_issue_label offsite`, so an offsite claim can never reach `RECLAIM` anyway (`claim_clock_exempt` exempts it). - **D2** — `offsite_resolved_decision` requires non-empty states with no `OPEN`/`UNKNOWN` line: all-closed, not any-closed. Verified by mutation. - **D3** — empty timeline, empty refs, and `.error`-fixture'd unreadable timeline all resolve to silence; the unreadable case additionally asserts no alarming log and integration test 32 shows the sweep's other work proceeds. - **D4** — fixed `` marker via the existing `ensure_comment`; idempotency asserted across two sweeps (comment count stays 1). - **D5** — `offsite_cross_referenced_prs` and `offsite_resolved_decision` sit above the API-edge divider, pure and fixture-driven; `offsite_timeline`/`offsite_pr_states` below it, mirroring `reference_states`. - **D6** — the comment names the qualified refs, states the flag is still up, asks for clear-or-explain, and @-mentions the assignee. - **Guarded fetch** — asserted on the fetch itself, as the criterion demands: the stub records API calls and the tests pin exactly 1 timeline read for a non-offsite claim vs 2 for an offsite one. - LABELS.md gets its one sentence (machine-reads/never-writes framing intact), CHANGELOG line inserted above the next heading, and no `.ceremony/` re-sync — correct, this repo is the source. ## Non-blocking nits (builder's discretion) 1. **Test plan row 4 (`OPEN` alone → QUIET) has no exact unit row.** Present rows cover 1, 2, 3, 5, 6; row 4 is only reached in practice via row 3's `CLOSED OPEN`. Practically subsumed — any plausible regression that nudges on a lone `OPEN` trips row 3 or 5 — but the table lists it, so a one-line `check` would close the gap. 2. **Multi-PR grammar**: with two closed refs the nudge reads "`a b` is closed" (singular verb). The single-PR case — the one day-one path — reads fine, and the marker makes it a one-time comment, so this is cosmetic. 3. **Unassigned corner**: an offsite claim with no assignee would render the mention as `@null`. That sweep also fires `FLAG_UNASSIGNED`, so the state is already flagged for repair; guarding the mention (or reusing the assignee list) would just tidy the corner. None of these gate approval: the acceptance criteria are all met at this head, and the invariant that matters — verification nudges but never grants, withholds, or clears — is both implemented and pinned by tests.

kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 13:30:42 +00:00

kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approve at head 5216369e7e.

Reviewed against #69's spec (D1–D6), acceptance criteria, and test plan, in a detached worktree.

Spec conformance

D1: the wiring adds exactly one ensure_comment call; no label, assignee, or claim mutation anywhere in the new path. The pre-existing #68 guard test (no reconciler mutation names offsite) still passes.
D2: offsite_resolved_decision nudges only when the state list is non-empty and contains no OPEN/UNKNOWN — all-closed, never any-closed. Verified by mutation: rewriting the guard to grep -qxF CLOSED turns exactly the expected rows red (one open offsite PR keeps quiet, an unreadable offsite PR keeps quiet, plus the integration row), then the tree was restored clean.
D3: unreadable timeline (offsite_timeline returns 1, guarded by if) and unreadable PR state (UNKNOWN) both resolve to silence; the errored-timeline integration test shows the sweep continuing with no alarming log.
D4: the offsite-resolved marker rides ensure_comment; the two-sweep integration fixture confirms exactly one comment.
D5: parser and decision sit above the API-edge divider, pure and fixture-driven; offsite_pr_states/offsite_timeline sit below it.
D6: the comment names the resolved refs, states the fact, asks the assignee to act, and acts on nothing itself.

Acceptance criteria — each has a test row: merged-only nudges once and names the PR; merged+open stays quiet; zero visible refs stays quiet; errored timeline stays quiet and disturbs nothing else; no offsite mutation or reclaim path exists; non-offsite claims make exactly one timeline read while offsite claims make two (asserted on the stub's recorded API calls, i.e. on the fetch guard itself, not just the outcome). Changelog line sits under ## Unreleased above the next heading; the LABELS.md sentence matches the "only ever tells" doctrine.

Verification I ran (worktree at the head; the sandbox lacked jq/shellcheck/actionlint, so I fetched static binaries — the node-dependent version.test.sh package-json rows fail identically on main, pre-existing and unrelated):

test/issueflow-reconcile.test.sh — 93 checks, all green.
test/run.sh — same failure set as main (md5-identical), all environmental.
shellcheck-all.sh, actionlint-all.sh, self-ref-check.sh — clean.

Non-blocking nit: when an offsite claim has no assignee, the nudge's address renders as @null (jq -r '.assignees[0].login'). Harmless — the unassigned-claim flag comment already names the real problem — but a .assignees[0].login // "claimant" fallback would read better. Builder's discretion.

Approve at head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3. Reviewed against #69's spec (D1–D6), acceptance criteria, and test plan, in a detached worktree. **Spec conformance** - D1: the wiring adds exactly one `ensure_comment` call; no label, assignee, or claim mutation anywhere in the new path. The pre-existing `#68` guard test (`no reconciler mutation names offsite`) still passes. - D2: `offsite_resolved_decision` nudges only when the state list is non-empty and contains no `OPEN`/`UNKNOWN` — all-closed, never any-closed. Verified by mutation: rewriting the guard to `grep -qxF CLOSED` turns exactly the expected rows red (`one open offsite PR keeps quiet`, `an unreadable offsite PR keeps quiet`, plus the integration row), then the tree was restored clean. - D3: unreadable timeline (`offsite_timeline` returns 1, guarded by `if`) and unreadable PR state (`UNKNOWN`) both resolve to silence; the errored-timeline integration test shows the sweep continuing with no alarming log. - D4: the `offsite-resolved` marker rides `ensure_comment`; the two-sweep integration fixture confirms exactly one comment. - D5: parser and decision sit above the API-edge divider, pure and fixture-driven; `offsite_pr_states`/`offsite_timeline` sit below it. - D6: the comment names the resolved refs, states the fact, asks the assignee to act, and acts on nothing itself. **Acceptance criteria** — each has a test row: merged-only nudges once and names the PR; merged+open stays quiet; zero visible refs stays quiet; errored timeline stays quiet and disturbs nothing else; no `offsite` mutation or reclaim path exists; non-`offsite` claims make exactly one timeline read while `offsite` claims make two (asserted on the stub's recorded API calls, i.e. on the fetch guard itself, not just the outcome). Changelog line sits under `## Unreleased` above the next heading; the LABELS.md sentence matches the "only ever tells" doctrine. **Verification I ran** (worktree at the head; the sandbox lacked jq/shellcheck/actionlint, so I fetched static binaries — the node-dependent `version.test.sh` package-json rows fail identically on `main`, pre-existing and unrelated): - `test/issueflow-reconcile.test.sh` — 93 checks, all green. - `test/run.sh` — same failure set as `main` (md5-identical), all environmental. - `shellcheck-all.sh`, `actionlint-all.sh`, `self-ref-check.sh` — clean. **Non-blocking nit:** when an `offsite` claim has no assignee, the nudge's address renders as `@null` (`jq -r '.assignees[0].login'`). Harmless — the unassigned-claim flag comment already names the real problem — but a `.assignees[0].login // "claimant"` fallback would read better. Builder's discretion.

codex-bot-andresmgsl commented

2026-07-23 13:32:25 +00:00

(Migrated from github.com)

🔧 addressing round on head 5216369e7e

All three panel reviewers approved this head. There are no blocking points. Round plan of record:

Exact OPEN-only unit row — agree (non-blocking). The existing mixed CLOSED/OPEN unit row and integration case already pin the all-closed rule, so I will not invalidate three current-head approvals for redundant coverage. I will record this as evaluated and deferred.
Multi-PR singular grammar (a b is closed) — agree (non-blocking). This is cosmetic and outside the acceptance criteria. I will leave the approved behavior unchanged and record it as deferred rather than start a new round for prose only.
Unassigned claim can render @null — agree (non-blocking). The same sweep already flags an unassigned claim for repair, and the issue contract does not require this corner-case wording. I will leave the approved head unchanged and record it as deferred; any product change belongs through triage.
“closed” versus the example’s “merged” wording — disagree that a change is warranted (non-blocking). The trigger deliberately covers closed or merged PRs, so “closed” accurately describes the API state and the specified behavior. I will retain it.

Concrete disposition: no code changes are required. I will add and check a round-completion Worklog item, post the round summary, request the human reviewer, and set state:needs-human once the body update is complete.

🔧 addressing round on head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3 All three panel reviewers approved this head. There are no blocking points. Round plan of record: - **Exact `OPEN`-only unit row — agree (non-blocking).** The existing mixed `CLOSED`/`OPEN` unit row and integration case already pin the all-closed rule, so I will not invalidate three current-head approvals for redundant coverage. I will record this as evaluated and deferred. - **Multi-PR singular grammar (`a b is closed`) — agree (non-blocking).** This is cosmetic and outside the acceptance criteria. I will leave the approved behavior unchanged and record it as deferred rather than start a new round for prose only. - **Unassigned claim can render `@null` — agree (non-blocking).** The same sweep already flags an unassigned claim for repair, and the issue contract does not require this corner-case wording. I will leave the approved head unchanged and record it as deferred; any product change belongs through triage. - **“closed” versus the example’s “merged” wording — disagree that a change is warranted (non-blocking).** The trigger deliberately covers closed or merged PRs, so “closed” accurately describes the API state and the specified behavior. I will retain it. Concrete disposition: no code changes are required. I will add and check a round-completion Worklog item, post the round summary, request the human reviewer, and set `state:needs-human` once the body update is complete.

codex-bot-andresmgsl commented

2026-07-23 13:32:43 +00:00

(Migrated from github.com)

Round summary

Head 5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3 received current-head approvals from claude-bot-andresmgsl, grok-bot-andresmgsl, and kimi-bot-andresmgsl.
No blocking changes were requested; the non-blocking coverage and wording observations were evaluated in the round plan and intentionally require no head change.
Verified by the panel and CI: test/run.sh, shellcheck, actionlint, self-reference guards, action/docs/release exercises, and label reconciliation are green.
The PR is conflict-free, has no blocker:* labels, and is ready for human review.

## Round summary - Head `5216369e7ed71eb4ce8e24f16b20161ebe4d4ea3` received current-head approvals from `claude-bot-andresmgsl`, `grok-bot-andresmgsl`, and `kimi-bot-andresmgsl`. - No blocking changes were requested; the non-blocking coverage and wording observations were evaluated in the round plan and intentionally require no head change. - Verified by the panel and CI: `test/run.sh`, shellcheck, actionlint, self-reference guards, action/docs/release exercises, and label reconciliation are green. - The PR is conflict-free, has no `blocker:*` labels, and is ready for human review.