fix(service): queue-check reports advisory instead of failure for a missing or stale prepared queue - #65
Merged
Merged
Conversation
…g or stale prepared queue checkQueue conflated 'queue never prepared' and 'queue corrupted' into one red hash-mismatch failure. A prepared queue artifact embeds its asOfDate, so the hash can only match on the exact preparation day; any artifact older than today (or none at all in a fresh checkout) made service:queue-check exit 1 ~always, leaving the human-review gate red without actionable meaning. Now a missing artifact reports 'queue not prepared - run service:queue' and an artifact prepared on an earlier calendar day reports 'queue not prepared since <date> - run service:queue' as a non-failing advisory (status advisory, exit 0). Same-day corruption (invalid JSON, or a current-date artifact whose content no longer matches the rebuilt queue) and item-level failures stay red (status failed, exit 1). The queue artifact format and prepare path are unchanged; existing prepared queues still verify with --as-of their own date.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughChangesThe queue checker now separates advisory conditions from failures. Missing or outdated queues return Queue status handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/test-service-engine.mjs (1)
1244-1264: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact CLI contract for failure and stale-date advisories.
Line 1251 accepts any nonzero exit code, although failed queues must exit with code
1. Change it toeq(alteredQueueCli.status, 1).Lines 1262-1264 test only
checkQueueWithClock. Add the matching CLI check with a July 30, 2026 clock, and assert exit code0plus thequeue not prepared since 2026-07-29advisory.Suggested test update
- neq(alteredQueueCli.status, 0) + eq(alteredQueueCli.status, 1) ... assert(staleQueueCheck.advisories.some(message => message.includes("queue not prepared since 2026-07-29"))) + const staleQueueCli = run(QUEUE, ["--mode", "check", "--as-of", "2026-07-30"], E("2026-07-30T12:00:00.000+05:30")) + eq(staleQueueCli.status, 0, staleQueueCli.stderr) + const staleQueueCliResult = JSON.parse(staleQueueCli.stdout) + assert(staleQueueCliResult.advisories.some(message => message.includes("queue not prepared since 2026-07-29")))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-service-engine.mjs` around lines 1244 - 1264, Update the CLI assertions around alteredQueueCli and staleQueueCheck: require alteredQueueCli.status to equal 1 instead of merely being nonzero, then add a CLI checkQueue invocation using the July 30, 2026 clock and assert status 0 with the “queue not prepared since 2026-07-29” advisory. Preserve the existing checkQueueWithClock assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/test-service-engine.mjs`:
- Around line 1244-1264: Update the CLI assertions around alteredQueueCli and
staleQueueCheck: require alteredQueueCli.status to equal 1 instead of merely
being nonzero, then add a CLI checkQueue invocation using the July 30, 2026
clock and assert status 0 with the “queue not prepared since 2026-07-29”
advisory. Preserve the existing checkQueueWithClock assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 982d2730-8ccb-4c65-ac82-415601551e33
📒 Files selected for processing (4)
scripts/check-review-queue.mjsscripts/lib/review-queue.mjsscripts/run-review-queue.mjsscripts/test-service-engine.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
npm run service:queue-checkcould never pass after the preparation day:checkQueue()rebuilt the expected queue in-memory withasOfDate = todayand compared sha256 against the storedruns/service-engine/queue.json, which embeds its ownasOfDate/generatedAt. The hashes only match on the exact calendar day the queue was prepared, so any artifact older than 24h (or none at all in a fresh checkout) made the human-review gate red — a red signal that carried no actionable meaning and conflated "queue never prepared" with "queue corrupted".Change
scripts/lib/review-queue.mjs—checkQueue()now distinguishes the three states:advisory, exit 0): stored artifact missing →queue not prepared - run service:queue; stored artifact prepared on an earlier calendar date →queue not prepared since <date> - run service:queue.failed, exit 1): current-date artifact whose content no longer matches the rebuilt queue (queue output is stale or altered), invalid JSON, or any item-level failure (blocked items, invalid decisions, missing/stale agent work packets).scripts/check-review-queue.mjsandscripts/run-review-queue.mjs— exit non-zero only forstatus === "failed".--as-oftheir own date.scripts/test-service-engine.mjs— queue-check assertions now cover passed / same-day tamper (failed, exit 1) / missing artifact (advisory, exit 0) / stale artifact naming its date (advisory).Verification
Live runs in this worktree:
status: advisory,queue not prepared - run service:queue, exit 0status: advisory,queue not prepared since 2026-08-07 - run service:queue, exit 0status: passed, exit 0status: failed,queue output is stale or altered, exit 1npm run cipasses (38 engine checks + all gates),git diff --checkclean, sgscan: no new findings.Summary by CodeRabbit