Skip to content

fix(hn-monitor): guard against cron double-post (interim for cloud#1990)#51

Merged
khaliqgant merged 4 commits into
mainfrom
fix/hn-monitor-double-post
Jun 7, 2026
Merged

fix(hn-monitor): guard against cron double-post (interim for cloud#1990)#51
khaliqgant merged 4 commits into
mainfrom
fix/hn-monitor-double-post

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 7, 2026

Copy link
Copy Markdown
Member

Why

hn-monitor posted its digest twice for one cron tick. Verified in Daytona (org Agent Relay): a single sandbox ran the runner twice ~3 min apart for one tick — the cloud re-invoked a tick delivery whose 180s processing lease expired before the first run reported delivered. With the handler's old post-before-save ordering, the re-invocation re-read stale seen and re-posted.

Full root-cause analysis and the durable, layered fix (stable per-occurrence key in relaycron → per-occurrence delivery dedup + liveness-gated retry in the delivery layer → atomic ctx.once claim enforced in the runner) are tracked in AgentWorkforce/cloud#1990.

Change

Reorder to claim-then-announce: compute the digest, record the stories as seen, then post. A re-invocation now loads them as already-seen and stays silent.

Trade-off: if the post fails after the save, that digest is dropped rather than risking a duplicate — the right call for a low-stakes twice-daily summary. This is an interim guard; it doesn't cover truly concurrent invocations. The robust fix is cloud#1990.

Typecheck passes.

🤖 Generated with Claude Code


Summary by cubic

Stops duplicate Slack posts from hn-monitor and fixes noisy PR reviewer pings by gating and deduping the “ready for your review” announcement.

  • Bug Fixes
    • hn-monitor: record stories as seen before posting to avoid double-posts from at-least-once cron re-invocation; interim guard pending AgentWorkforce/cloud#1990. If the post fails, the digest is dropped rather than risking a duplicate.
    • PR reviewer: announce “ready” once per head SHA (populate from gh pr view --json headRefOid when missing), and only when the PR is OPEN, mergeable, not DRAFT, no CHANGES_REQUESTED, and checks are complete and passing. An empty check rollup counts as ready only when mergeStateStatus is CLEAN; SKIPPED checks are non-blocking.
    • Review harness: require an “Addressed comments” section with file:line locations for each bot/reviewer comment. Stop committing Relay VFS runtime state and ignore .relay/ and memory/workspace/.relay/.

Written for commit 2ddc49b. Summary will update on new commits.

Review in cubic

khaliqgant and others added 3 commits June 7, 2026 22:51
The reviewer announced PRs "ready for your review" in cases where it
shouldn't, and re-announced the same commit on every webhook:

- verifyReadyForHumanReview only queried mergeable,statusCheckRollup —
  never the PR state — so a PR that merged/closed between the harness run
  and the check could still be announced ready. Now query state and gate
  on OPEN.
- prReadyStateAllowsHumanReview let an *empty* statusCheckRollup pass
  vacuously (checks.every === true), so a PR whose checks were queued but
  not yet registered read as "ready" — the pending-checks symptom. Now an
  empty rollup is not-ready, and drafts / CHANGES_REQUESTED are held back.
- announceReadyOnce records the head SHA it announced ready for and skips
  re-announcing the same commit; new commits still get a (threaded) note.

Also: require the harness to account for every bot/reviewer comment under
an "## Addressed comments" heading with the file:line where each was
fixed, so the channel and comment authors can see where each was handled.

And stop tracking memory/workspace/.relay/state.json — it's relay VFS
runtime state rewritten every reconcile cycle, so the review harness kept
committing it to PRs. Untracked + gitignored.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hecks, no-CI repos

From cubic + gemini review of #50:

- Populate pr.headSha from `gh pr view --json headRefOid` so the ready
  dedupe always keys on the commit. Some webhook payloads (check_run.
  completed) omit the head SHA; without it the dedupe was bypassed and
  re-pinged. (cubic P1 / gemini)
- Treat SKIPPED checks as non-blocking in checkPassedAndComplete. A
  conditionally-skipped CI job is GitHub's "not applicable", not a
  failure, and was pinning PRs out of "ready" forever. Mirrors the
  trigger's ciFailed(), which already treats skipped as non-failing.
  (gemini)
- Don't permanently block no-CI repos: an empty status-check rollup now
  counts as ready when GitHub's mergeStateStatus is CLEAN (nothing
  blocking), while a transient "checks queued but unregistered" window
  (UNSTABLE/BLOCKED/UNKNOWN) still reads as not-ready. (gemini)

The concurrent-run dedupe race cubic flagged (P2) is a known best-effort
limit — the memory API has no atomic check-and-set, and per-PR webhook
delivery is effectively serialized.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-post guard)

A single cron tick can re-invoke this handler ~3 min apart: cloud re-runs a
tick delivery whose 180s processing lease expires before the first run reports
done (AgentWorkforce/cloud#1990). With post-before-save, the re-invocation
re-read stale `seen` and posted the digest a second time — the observed
double-post.

Record the stories as seen BEFORE posting so a re-invocation loads them as
already-seen and stays silent. Trade-off: a failed post drops that digest
rather than risking a duplicate — acceptable for a low-stakes twice-daily
summary. Stopgap; the durable fix is idempotent cron delivery in cloud#1990
(stable per-occurrence key + atomic claim in the runner).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codeant-ai

codeant-ai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@agent-relay-code[bot], we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 31 minutes and 51 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 57bf36ae-0c2d-4eb4-8527-fee1cd94942c

📥 Commits

Reviewing files that changed from the base of the PR and between f16a055 and 2ddc49b.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • hn-monitor/agent.ts
  • tests/hn-monitor.test.mjs
📝 Walkthrough

Walkthrough

This PR strengthens PR readiness gating in the review agent with stricter state checks and head-SHA-based notification deduplication, enforces precise comment location tracking in the review harness prompt, and reorders the HN monitor cron handler to persist story digests before posting to Slack.

Changes

Review Agent PR Readiness Gating & Notification

Layer / File(s) Summary
PR readiness state verification and gating logic
review/agent.ts, tests/review-agent.test.mjs
PR readiness now enforces open-state checks, excludes drafts and changes-requested decisions, requires mergeable status, handles empty check rollups via merge state cleanness, and treats skipped checks as non-blocking. Tests cover state, mergeable, check conclusion, and review decision gating.
Ready notification deduplication by head SHA
review/agent.ts
Ready announcements are de-duplicated per PR head SHA using workspace memory keyed by PR identifiers and opener login, posting to Slack only once per distinct head commit.
Review harness prompt strictness for addressed comments
review/agent.ts, tests/review-agent.test.mjs
The review harness prompt requires each addressed bot/reviewer comment to include an exact file:line fix location or explicit stale/invalid reason, with tests rejecting unsupported addressing claims.
Relay VFS state exclusion from version control
.gitignore
Relay VFS runtime state directories are ignored with a comment explaining state is rebuilt each reconcile cycle.

HN Monitor Cron Digest Ordering

Layer / File(s) Summary
Cron digest computation and persistence before Slack posting
hn-monitor/agent.ts
The handler now computes the digest first, saves matched story IDs to durable memory, then posts the pre-computed digest. Comments document at-least-once cron re-invocation behavior driving this ordering.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • AgentWorkforce/agents#48: Both PRs refactor review/agent.ts readiness gating in prReadyStateAllowsHumanReview and prompt strictness in reviewHarnessPrompt to ensure "READY" is only announced when GitHub mergeability and status checks genuinely indicate readiness for human review.

Suggested labels

size:L

Poem

A rabbit hops through review gates with care,
Checking every PR's mergeable lair.
No drafts slip by, no changes stay unseen—
Each comment traced to the fix's address, pristine.
Stories digest before the Slack post flies,
All marked "ready" once, no surprises! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: guarding against cron double-posts in hn-monitor by reordering to claim-then-announce, which is the primary fix addressed in this PR.
Description check ✅ Passed The description clearly explains the problem (double-posting), the change (reorder to claim-then-announce), the trade-off (post-failure drops digest), and references the interim nature pending cloud#1990, directly relating to the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hn-monitor-double-post

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the PR review agent and HN monitor. It ignores Relay VFS runtime state in .gitignore, deletes an unwanted state file, and prevents duplicate Slack notifications in hn-monitor/agent.ts by marking stories as seen before posting. In review/agent.ts, it implements deduplicated "ready for review" announcements using workspace memory, refines PR readiness checks (verifying open status, draft status, review decisions, and skipped checks), and updates the review prompt to require explicit tracking of addressed comments. Unit tests are added to cover these new readiness rules. There are no review comments to address, so no feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codeant-ai

codeant-ai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 2ddc49b to this PR. The notes below describe what changed.

Fixed a real PR issue in hn-monitor/agent.ts: the PR intended to claim stories before retry-prone work, but it still ran LLM summarization before saving seen. I moved the claim ahead of summarization/posting and added tests/hn-monitor.test.mjs to pin that ordering.

Addressed Comments

  • codeant-ai[bot]: no actionable review; comment says the PR review quota was reached.
  • coderabbitai[bot]: no actionable finding yet; comment/status says review is still in progress.
  • cubic · AI code reviewer: no annotations returned by the API; check is still in progress.

Validation run:

  • node scripts/test.mjs tests/review-agent.test.mjs && node scripts/test.mjs tests/hn-monitor.test.mjs
  • npm test

Remote PR state is not ready: GitHub API reports CodeRabbit status pending, cubic check in_progress, and PR mergeable_state: unstable, so I am not emitting the READY sentinel.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Re-trigger cubic

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

No further code changes were needed in the current checkout.

Validation run:

  • npm test passed: 49/49
  • node scripts/test.mjs tests/review-agent.test.mjs passed: 23/23
  • node scripts/test.mjs tests/hn-monitor.test.mjs passed: 1/1

Remote PR state observed via GitHub API: PR is open, not draft, mergeable_state: clean, no inline review comments, CodeRabbit status is success, cubic check is completed with success and 0 annotations.

Addressed comments

  • agent-relay-code[bot]: prior finding about HN saving after summarization is already fixed in hn-monitor/agent.ts:54, with coverage in tests/hn-monitor.test.mjs:6.
  • gemini-code-assist[bot]: no actionable findings; review says no feedback was provided.
  • cubic-dev-ai[bot]: no actionable findings; review says no issues found.
  • codeant-ai[bot]: no actionable review; quota-limit comments only.
  • coderabbitai[bot]: no actionable code finding; current GitHub status reports CodeRabbit completed successfully.

@khaliqgant khaliqgant merged commit 6e92b83 into main Jun 7, 2026
2 checks passed
@khaliqgant khaliqgant deleted the fix/hn-monitor-double-post branch June 7, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant