Record the observed merge base in Smart CI receipts - #2832
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Round 1 independent review at549366887a8e11230e1b0c57cea0a13fa235d6e3:
Fresh consumer proof and fix validation follow. No merge before exact-head required CI, scoped fix review and push aging. Authority is the repository tier declaration (merge free within those gates); no additional owner-click gate is inferred. |
|
Confirmed blocker proof: the new valid historical H1/B1 + final H2/B2 regression failed against the reviewed grouping (22/23 pass, unusableObservationCount actual1 expected0), then passed after removing only the cross-attempt mergeBaseSha equality (23/23). Per-attempt actual-parent/tree and final landed-base checks remain. The consumer review found no additional direct HIGH/CRITICAL blockers. Its later synthetic grouped execution overlapped the author's fix and therefore is not used as red evidence; the author's preserved red-first log is authoritative. Logs: .tmp/issue-2508/review-fix-red-recall.log and review-fix-green-recall.log, to be preserved outside the checkout before teardown. Fix commit and scoped fresh review follow. |
|
Round 2 scoped fresh Terra fix review: SHIP at0620b9ec3297d2c1b80ccbdca2c488380f5631ce. Recall23/23, CLI roundtrip1/1 and diff check passed independently; per-attempt actual Git parent/tree and final landed binding remain enforced. One confirmed blocker fixed; no additional HIGH/CRITICAL finding. Completed checkout cleanup: tracked clean, ignored inventory only .tmp. Copied all logs, exit receipts and review diffs to .claude/local/milestone-20260909/preserved-2832/verification-and-review and verified every copied file hash, then removed the owned worktree with plain git worktree remove. Remote branch and commits preserved; primary unchanged. Hosted observation: Smart CI run34363453732 was cancelled while a second same-head run34363457040 started two seconds later and succeeded (both Plan and Required Gate). The workflow handles synchronize and edited events in one cancel-in-progress group; this is a superseded duplicate, not an unexplained failing test. Required ci-required run34363454720 remains pending and prevents merge. Smart CI Self-Test34363454269 passed at the current head; no claim that base-executed shadow workflow already exercised the new production plumbing. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549366887a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nch-copy Refresh launch copy for shipped batch Apply eligibility
|
Delivered as416b5e9cfdc530b7ad9f3c53c9737516c102b02b. Required ci-required run34366091836 passed at22b2b2fa6d6f51b3d9b26dd34f04af975f1a88e2; main tree exactly equals that qualified head. Producer/consumer and scoped fix reviews passed, aging satisfied, no unresolved threads. The cancelled shadow run34366090967 was superseded by successful same-head34366093956, not an unexplained test failure. One confirmed historical-base grouping blocker was fixed; two MEDIUM diagnostic/local-preview follow-ups remain tracked at #2508 comment5603540885. No full parent-issue closure claim. Worktree evidence is preserved under .claude/local/milestone-20260909/preserved-2832. |
…e merges The row named the eight-PR continuation train because I followed the train. Sweeping every merge since 2026-09-08 for ci/policy.v1.json control paths instead finds three more that no delegation covers: #2832 416b5e9 2026-09-09T15:34:32Z 15 files, including .github/workflows/smart-ci-shadow.yml, both ci/schemas receipts and scripts/ci/smart-ci/lib/plan.mjs Refs #2508 #2834 afadf72 2026-09-09T16:53:40Z scripts/ci/dev-up.test.mjs Refs #2588 #2858 9f9b796 2026-09-10T14:35:25Z scripts/ci/dev-up.test.mjs and scripts/dev-up.ps1 #2858's own body says "Maintainer review remains required because regression coverage touches scripts/ci". #2832 and #2834 carry no such line. None is covered by the 2026-09-09 four-PR delegation, whose members were integrated as #2815 and are explicitly not in dispute. #2832 is the substantial one: shadow workflow, both receipt schemas, and the planner library. A decision packet that undercounts is a mis-framed packet, which is the error a review already caught in this row once. The count is now eleven PRs across three disclosure rows, with five more acknowledged in the 2026-09-06 q-2 ruling: eighteen merges outside the per-PR review, thirteen still awaiting a reply. That rate is the argument for settling (b) either way. Refs #2337, #2336.
… J.3 Fresh-context review of the three previously unreviewed commits on this PR. All three findings are on text I added. HIGH: the Changelog entry said "Eleven control-plane PRs" while listing twelve and while the row it summarises says twelve in three places - the heading, question (a), and the arithmetic paragraph. The sequence is visible: one commit took 8 to Eleven, the next added #2866 to the list but not to the word. A maintainer reading the Changelog first would take eleven as the count and then be unable to reconcile 11 + 1 + 1 against the row's fourteen. Corrected to twelve. MEDIUM: "three of the 36 paths" understated the old coverage. The three globs .github/**, ci/** and scripts/ci/** reach 10 of the 36 declared entries, not 3, because .github/** alone covers eight of them. Measured against ci/policy.v1.json. The substance holds - 26 entries were unreached - but the number was wrong and I had repeated it. MEDIUM: "#2866 had no way to know it had become control-plane" claimed more than was verified. scripts/deploy/** is declared in the checked-in ci/policy.v1.json, so the path was discoverable; what is verified is that the auto-loading rules region never fired. Both the row and the Changelog now state the mechanism instead of the agent's state of knowledge. LOW, fixed while here: the heading read 2026-09-08 to 2026-09-10 when the earliest merge it lists is #2832 on 2026-09-09. 2026-09-08 is the sweep window and the date of J.1 and J.2, which sit directly above it. Declined: the reviewer noted "nineteen ... outside the per-PR review" is narrower than the phrase suggests, since it excludes the sixteen merged under the 2026-09-06 and 2026-09-09 delegations whose post-hoc review is still owed. The exclusion is deliberate and conservative - delegated merges were not violations - and including them would strengthen rather than weaken the row's argument.
1 participant
Smart CI accepts a merge ref whose first parent matches the authenticated live base tip after dispatch, but the receipt previously recorded only the dispatch base. The landed verifier therefore could not validate that accepted observation without treating it as a parent mismatch.
Record mergeBaseSha and mergeBaseTipSha from the same accepted merge observation, pass them through the planner and gate receipt, and validate the observed parent against real Git parents in recall. Keep the event head, tree checks, retry bound and expected control-base equality unchanged. Version-1 readers retain the legacy fallback only when both new fields are absent; partial or malformed bindings fail closed.
Refs #2508. The separate stale/retarget topology work in #2562 is unchanged.
Validation at 22b2b2f, based on 0e74214:
Round 1 independent Terra review: producer/gate SHIP; consumer found one confirmed HIGH cross-attempt merge-base grouping defect. Fixed in0620b9ec: historical attempts may observe different bases while each actual Git parent/tree and final landed base remain checked. Red-first regression failed22/23 before the fix and passed23/23 afterward; CLI roundtrip1/1 and full Smart CI157/157 passed. Scoped fresh Terra fix review SHIP: recall23/23 and CLI roundtrip1/1 passed independently. No additional blocker was found. Connector grouping feedback duplicates the fixed defect; two non-blocking MEDIUM follow-ups (malformed diagnostic receipt schema and no-event local --pr preview) are tracked at #2508 (comment). All three threads were triaged and resolved without another fix round. Required hosted checks and push aging remain pending. Actual actionlint is not locally installed; bootstrap tests do not prove workflow lint. No backend/frontend runtime behavior was changed or re-tested. Canonical STATUS/MASTERPLAN and OUTSTANDING_TASKS.md are unchanged.
Base refresh after #2831: clean merge; all 14 reviewed source files exactly match0620b9ec. Smart CI157/157, doc links674/0, docs governance and diff checks passed again. The base changed only launch copy, so prior producer/consumer/fix reviews remain applicable; required hosted CI and aging apply anew to22b2b2fa.