ci: accept later PR timestamps for re-attestation

[Vadevious]

Summary

Fix the re-attestation gate that keeps #5548 pending after the author clears its checklist. A later GitHub PR update can advance the live updated_at timestamp between the author edit event and the gate's live read. Accept that ordering while retaining the author, head SHA, exact body, changed previous body, and post-checkpoint requirements.

Verification

• On 2b73577: node --test .github/scripts/pr-quality-state.test.cjs — 58 pass, including the eight-hour delay and three rejected timestamp cases.
• On f318461: bun test tests/ci-workflows/pr-readiness-reattest.test.ts — 22 pass, including the delayed live read.
• On f318461: bun run typecheck — pass.
• On the preceding code commit: bun run privacy:scan — pass; bun test tests/ci-workflows — 1,195 pass, 150 skip, 9 Windows environment failures where subprocess tests could not find bun. The two follow-up commits change only the explanation and timestamp regression tests, so the broad suite was not repeated. The full repository suite was not run because the 72-file workflow suite alone took about 130 seconds. Hosted CI still needs to validate this exact head.

Security review

This changes a privileged pull_request_target gate. The diff leaves triggers, permissions, trusted checkout, and write actions unchanged. The lag has no upper bound intentionally: even a delayed or replayed author event proves a post-checkpoint edit only while the same author, head SHA, and exact body remain live; an older event cannot cross a later checkpoint. A maintainer security review is required before merge under MAINTAINERS.md.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (none needed for this internal gate correction).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults by a maintainer.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• Required local validation passed; commands, results, and any full-suite exception are documented.

• I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Updated pull request readiness checks to recognize qualifying author edits when the event timestamp is earlier than the pull request’s latest update, provided it falls after the readiness checkpoint.
  • This helps prevent valid edits from being missed when there is a delay between an edit event and the latest pull request activity.
  • Readiness checks now remain pending when timestamps are missing, invalid, or indicate an event occurred after the pull request’s latest update.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c915f6e5-be06-4be8-927e-10f3f2ecbb55

📥 Commits

Reviewing files that changed from the base of the PR and between f318461 and 2b73577.

📒 Files selected for processing (1)

• .github/scripts/pr-quality-state.test.cjs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Author-body reattestation accepts an author event timestamp after the checkpoint and no later than the live PR timestamp, provided both timestamps are finite. Tests cover delayed events, future events, and missing or invalid live timestamps.

Changes

Reattestation timestamp handling

Layer / File(s)	Summary
Timestamp validation and coverage .github/scripts/pr-readiness-reattest.cjs, .github/scripts/pr-quality-state.test.cjs, tests/ci-workflows/pr-readiness-reattest.test.ts	qualifyingAuthorBodyEdit no longer requires the event timestamp to equal the live PR timestamp. It checks that the event timestamp falls between the checkpoint and live timestamps. Tests cover a delayed event reaching await-check, the checkpointAt reset, and rejection cases that remain in await-clear.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 2b735

No actionable merge blocker is identified. Complete the planned CI validation and maintainer security review before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing later live PR timestamps during re-attestation. This matches the implementation in .github/scripts/pr-readiness-reattest.cjs and t…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
• ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

[lidge-jun]

리뷰 · 우선순위 62 / 80

이 PR은 작성자가 체크리스트를 비워도 준비 확인이 다음 단계로 안 넘어가는 일을 고칩니다. 지금 #5548이 그 대기입니다. 게이트 기록은 await-clear이고, 기준 시각은 2026-09-23T14:34:48Z입니다.

예전에는 작성자가 본문을 고친 시각과, 봇이 조금 뒤에 다시 읽은 PR 시각이 글자까지 같아야 통과했습니다. 그 사이에 다른 갱신이 PR 시각만 뒤로 밀면, 본문이 그대로여도 거절입니다. 체크리스트를 비워도 대기에 남습니다.

이제는 고친 시각이 기준 시각보다 뒤이고, 다시 읽은 시각과 같거나 더 이르면 통과합니다. 작성자가 그 PR의 사람 계정인지, 헤드 커밋이 같은지, 본문 글자가 같은지, 이전 본문과 다른지는 그대로 봅니다. 고친 시각이 다시 읽은 시각보다 미래면 거절합니다. 워크플로는 .github/scripts/pr-quality-state.cjs가 이 파일을 불러 씁니다. 베이스는 dev입니다. 아직 드래프트이고 준비 체크는 0/4입니다. 이 헤드의 enforce-target 잡은 스텝 없이 취소됐습니다. 같은 시각 비교를 고치는 다른 열린 PR은 없습니다.

.github/scripts/pr-readiness-reattest.cjs:129 - 다시 읽은 시각은 이벤트보다 뒤이기만 하면 됩니다. 1초 뒤와 며칠 뒤의 옛 edited 재실행이 같은 조건입니다. 본문이 같으면 통과합니다. “이 수정이 PR의 마지막 갱신이다”를 보던 비교는 이 줄에서 빠졌습니다.

.github/scripts/pr-quality-state.test.cjs:822 - 이벤트가 라이브보다 미래인 거절은 여기 한 줄입니다. 시각은 이벤트 2026-09-22T01:00:02.000Z, 라이브 2026-09-22T01:00:01.000Z입니다. 통과 테스트는 라이브가 1초 늦은 경우만 있습니다. 몇 시간 뒤에도 통과하는지는 테스트에 없습니다.

메인테이너의 판단이 필요한 지점

이 파일은 pull_request_target에서 돌아갑니다. 본문의 보안 체크는 비어 있습니다. MAINTAINERS.md가 말하는 메인테이너 보안 리뷰가 머지 전에 필요합니다. 시각 차이도 정해야 합니다. 차이를 제한하지 않으면 막힌 워크플로를 나중에 다시 돌려도 풀립니다. 몇 분으로 자르면, 본문만 같은 옛 배달이 단계를 올리는 일은 막힙니다.

너의 추천

고친 시각이 다시 읽은 시각보다 이르거나 같을 때만 통과하게 두세요. 미래 시각은 지금처럼 거절이 맞습니다. 차이를 제한하지 않을 거면 그 뜻을 비교 위 주석에 적고, 라이브가 몇 시간 뒤여도 본문이 같으면 통과하는 테스트를 넣으세요. 몇 분으로 자를 거면 eventMs <= liveMs 옆에 상한을 두세요. 준비 체크와 보안 서명은 그 다음에 채우면 됩니다. 닫을 중복 PR은 없습니다.

이 댓글은 grok-bot이 작성했습니다

[abhisheksharma2411]

Hi @Vadevious — this relaxes a gate, so I went at it adversarially rather than reading the diff and nodding. It holds. Everything below is evidence for that plus one suggestion.

The change, restated

-    event.updatedAt === live.updatedAt &&
-    Number.isFinite(checkpointMs) && Number.isFinite(eventMs) &&
-    eventMs > checkpointMs
+    Number.isFinite(checkpointMs) && Number.isFinite(eventMs) && Number.isFinite(liveMs) &&
+    eventMs > checkpointMs && eventMs <= liveMs

Worth saying explicitly, because "removed an equality check from a security gate" reads alarming out of context: the equality was proving "nothing changed between the event and the live read", and the surrounding predicate already proves the stronger thing — the live body and head still match the event's. So the equality was standing in for a freshness property that event.body === live.body and event.headSha === live.headSha establish directly, at live-read time. Dropping it doesn't widen what qualifies; it stops a benign GitHub timestamp bump from disqualifying it.

Eleven cases against f318461

ok   exact-equal timestamps still clear                    (control)
ok   the delayed live read this PR enables                 (control, 8h lag)
ok   a DIFFERENT author's edit must not clear
ok   a bot's edit must not clear
ok   an edit on a different head SHA must not clear
ok   an event body differing from live must not clear
ok   a no-op edit (previousBody == body) must not clear
ok   a PRE-checkpoint event must not clear
ok   a FUTURE-dated event (after live) must not clear
ok   a live read with no timestamp must not clear
ok   a live read with a junk timestamp must not clear

11/11 as expected

The last three are the ones the new clause is responsible for. Number.isFinite(liveMs) makes a missing or unparseable live.updatedAt fail closed, which is the right direction for a gate — and it's a behaviour that only exists because of this PR, since liveMs wasn't parsed before.

Your own suites pass here too: pr-quality-state.test.cjs 57 pass, and I confirmed eventMs <= liveMs is doing real work rather than being vacuous — a future-dated event is rejected by it alone.

One suggestion

The new clause introduces three behaviours nothing currently pins:

• a future-dated event is rejected
• a missing live.updatedAt fails closed
• an unparseable live.updatedAt fails closed

Your tests cover the delayed direction thoroughly (the eight-hour case in both suites), but not these. They're the ones a future simplification would break silently — Number.isFinite(liveMs) looks like defensive noise to someone who doesn't know it's the fail-closed path, and eventMs <= liveMs looks redundant next to eventMs > checkpointMs. Three assertions would make them non-optional.

Happy to push them as cases on your branch if useful — say the word and they're yours, no credit needed.

Nice fix. The comment explaining why the lag isn't capped is doing real work too; that's the part someone would otherwise "tighten" back.

[lidge-jun]

Carried into bundle #5740, which is now on dev (squash-merged as e535c65) with a Co-authored-by trailer for you, so this PR is closing as landed. Thank you for the fix. If something from this branch did not make it in, the bundle description lists what was changed during the carry.