Repository navigation
fix(release): bound Sentry gate to a recent-activity window + alert on blocked daily releases - #305
Conversation
…n blocked daily releases Daily auto-release reliability (pm-nb08): the sentry-telemetry-gate queried `is:unresolved level:[fatal,error]` with no time bound, so any single stale unresolved fatal/error issue blocked *every* scheduled release until manually resolved in Sentry — silently, with no alerting (3 releases skipped 06-14..16). - sentry-telemetry-gate.mjs: new `--sentry-window-days` flag (default 14, 0 = unbounded legacy) appends Sentry's `lastSeen:-Nd` relative-date filter so a stale benign issue that stopped firing no longer blocks releases; an actively firing error still gates. `window_days` surfaced in JSON + text output. - run-gates.mjs + release.yml: thread/invoke the gate with `--sentry-window-days 14`. - auto-release.yml: open/update an "Auto Release blocked" GitHub issue on scheduled failure (mirrors the pm-ehbb nightly-failure alert) so blocked daily releases are never silently skipped. - docs/RELEASING.md: document the window + blocked-release alerting. Cross-platform test hardening (pm-i84i, partial): fix the two hardcoded POSIX-separator assertions that fail the windows-latest nightly — init-command asserts `path.resolve(...)` and schema-command asserts `path.join(...)` instead of forward-slash literals — and add a win32-simulated guard (runtime-schema-path-win32-guard.spec.ts) so a POSIX per-PR run catches a schema-path separator regression. The `file://`-URL TS dynamic-import cluster remains open (needs a Windows runner to verify). Verified live against Sentry (lastSeen:-14d query accepted, high=0), full smoke test green, coverage 100/100/100/100 (4687 tests), static gate + changelog check pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRyLiTvXUw6EQRTvXSufn3
Reviewer's GuideAdds a configurable recent-activity window to the Sentry release gate and wires it through release/auto-release workflows with alerting for blocked scheduled runs, plus fixes and guards around path-separator-sensitive tests to keep Windows CI green. Sequence diagram for auto-release blocked-run GitHub alertsequenceDiagram
actor Scheduler
participant GitHub_Actions as GitHub_Actions_auto-release
participant Release_Job as release_job
participant Alert_Step as Alert_on_blocked_scheduled_auto-release
participant GitHub_API as GitHub_API
Scheduler->>GitHub_Actions: schedule event
GitHub_Actions->>Release_Job: run jobs
Release_Job-->>GitHub_Actions: failure()
GitHub_Actions->>Alert_Step: run bash alert step
Alert_Step->>GitHub_API: gh issue list --search Auto Release blocked
GitHub_API-->>Alert_Step: existing issue number or empty
alt existing issue
Alert_Step->>GitHub_API: gh issue comment <number>
else no existing issue
Alert_Step->>GitHub_API: gh issue create --title Auto Release blocked
end
Flow diagram for Sentry gate recent-activity window usageflowchart LR
A[run-gates.mjs] -->|reads --sentry-window-days| B[sentry-telemetry-gate.mjs]
B -->|parseNumber sentry-window-days| C[sentryWindowDays]
C --> D[buildSentryGateQuery]
D -->|returns query with lastSeen:-Nd when windowDays>0| E[fetchSentryIssues]
E -->|query Sentry API| F[Sentry]
E --> G[sentrySummary]
G --> H[release gate decision]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai full review |
|
@codex review |
|
/gemini review |
|
To use Codex here, create a Codex account and connect to github. |
✅ Action performedFull review finished. |
|
Warning Review limit reached
More reviews will be available in 22 minutes and 18 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughTwo independent fixes are shipped together. First, the Sentry telemetry release gate gains a ChangesSentry Gate Windowing and Blocked-Release Alerting
Windows Path Separator Fixes and Regression Guard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
release-automation-contracttest that checks for"`${baseQuery} lastSeen:-${windowDays}d`"insentry-telemetry-gate.mjsis quite brittle to any refactor of spacing or interpolation; consider asserting on a more stable snippet (e.g., justlastSeen:-${windowDays}dor using a small regex) so the contract still holds if the implementation is reformatted. - The
run-gates.spec.tsexpectation that joins the entire spawn command and looks for the fullsentry-telemetry-gate.mjs --json --telemetry-mode best-effort --sentry-window-days 14string is sensitive to argument ordering changes; you could make this more robust by asserting presence of individual args/values in the array instead of a single concatenated substring.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `release-automation-contract` test that checks for ``"`${baseQuery} lastSeen:-${windowDays}d`"`` in `sentry-telemetry-gate.mjs` is quite brittle to any refactor of spacing or interpolation; consider asserting on a more stable snippet (e.g., just `lastSeen:-${windowDays}d` or using a small regex) so the contract still holds if the implementation is reformatted.
- The `run-gates.spec.ts` expectation that joins the entire spawn command and looks for the full `sentry-telemetry-gate.mjs --json --telemetry-mode best-effort --sentry-window-days 14` string is sensitive to argument ordering changes; you could make this more robust by asserting presence of individual args/values in the array instead of a single concatenated substring.
## Individual Comments
### Comment 1
<location path="CHANGELOG.md" line_range="22" />
<code_context>
### Fixed
+- Daily auto-release silently blocked 06-14..16 by a single stale unresolved Sentry error \(sentry-telemetry-gate is:unresolved has no time window\) ([pm-nb08](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-nb08.toon))
- appendLineAtomic is not truly atomic: concurrent appends to history JSONL can interleave ([pm-xy9n](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-xy9n.toon))
- Project-scope extension install writes ~69MB node\_modules per workspace \(peer pm-cli + transitive Sentry/OTel\) ([pm-oxq2](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-oxq2.toon))
</code_context>
<issue_to_address>
**issue (typo):** Add a missing verb and optional comma for grammatical clarity.
The sentence is missing a verb. Consider: “Daily auto-release was silently blocked … by a single stale, unresolved Sentry error …” for grammatical completeness and clarity.
```suggestion
- Daily auto-release was silently blocked 06-14..16 by a single stale, unresolved Sentry error \(sentry-telemetry-gate is:unresolved has no time window\) ([pm-nb08](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-nb08.toon))
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request implements a durable fix for daily auto-releases being blocked by stale unresolved Sentry errors by introducing a configurable recent-activity window (--sentry-window-days, defaulting to 14 days) to the Sentry telemetry gate. It also addresses Windows-specific path separator failures in the test suite by replacing hardcoded POSIX separators with platform-native path utilities and adding a simulated win32 regression guard. The review feedback suggests validating that the --sentry-window-days parameter is an integer to prevent Sentry API errors, and adding corresponding unit tests while ensuring dash-prefixed argument values are handled unambiguously.
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.
| const outputJson = flagBool(flags, "json", false); | ||
| const sentryProject = flagString(flags, "sentry-project", "unbrained/pm-cli"); | ||
| const sentryLimit = parseNumber(flagString(flags, "sentry-limit", null), "sentry-limit", 200); | ||
| const sentryWindowDays = parseNumber(flagString(flags, "sentry-window-days", null), "sentry-window-days", 14); |
There was a problem hiding this comment.
The --sentry-window-days parameter is parsed as a generic number, but Sentry's relative date query syntax (e.g., lastSeen:-Nd) expects an integer number of days. If a decimal value (such as 14.5) is provided, Sentry may return a 400 Bad Request or ignore the filter, leading to unexpected gate failures or bypasses.
Consider validating that sentryWindowDays is an integer.
| const sentryWindowDays = parseNumber(flagString(flags, "sentry-window-days", null), "sentry-window-days", 14); | |
| const sentryWindowDays = parseNumber(flagString(flags, "sentry-window-days", null), "sentry-window-days", 14); | |
| if (!Number.isInteger(sentryWindowDays)) { | |
| fail("Invalid --sentry-window-days value \"" + sentryWindowDays + "\". Expected an integer."); | |
| } |
| it("fails on a negative --sentry-window-days value", async () => { | ||
| const { errors } = await runSentryGate({ argv: ["--sentry-window-days", "-3"] }); | ||
| expect(errors.join("\n")).toContain('Invalid --sentry-window-days value "-3"'); | ||
| }); |
There was a problem hiding this comment.
To complement the integer validation for --sentry-window-days, we should add a unit test to verify that decimal values are rejected. Note that when passing dash-prefixed values (like negative numbers) in command-line arguments, we should use the unambiguous key=value format (e.g., --sentry-window-days=-3) instead of passing them as separate positional tokens, to prevent them from being mistaken for flags.
it("fails on a negative --sentry-window-days value", async () => {
const { errors } = await runSentryGate({ argv: ["--sentry-window-days=-3"] });
expect(errors.join("\n")).toContain('Invalid --sentry-window-days value "-3"');
});
it("fails on a non-integer --sentry-window-days value", async () => {
const { errors } = await runSentryGate({ argv: ["--sentry-window-days=14.5"] });
expect(errors.join("\n")).toContain('Invalid --sentry-window-days value "14.5"');
});References
- When pre-parsing or normalizing command-line arguments, avoid treating dash-prefixed tokens (starting with "-") as positional values to prevent them from being mistaken for actual flags. Direct users to unambiguous formats (e.g., "key=value" or JSON) for dash-leading values.
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable recent-activity window (--sentry-window-days, defaulting to 14 days) to the Sentry telemetry gate query to prevent stale, benign unresolved issues from blocking scheduled releases. It also adds alerting for blocked scheduled releases by opening or updating a GitHub issue on failure, and resolves cross-platform test failures on Windows by replacing hardcoded POSIX path separators with OS-native path helpers and adding a win32-simulated regression guard. The review feedback suggests truncating the windowDays parameter to an integer to prevent invalid Sentry query syntax if a non-integer value 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.
| if (windowDays <= 0) { | ||
| return baseQuery; | ||
| } |
There was a problem hiding this comment.
If a non-integer value is passed to --sentry-window-days (e.g., 14.5), the generated Sentry query will contain lastSeen:-14.5d, which is invalid in Sentry's relative date query syntax and can cause the API request to fail. To prevent this, we should ensure windowDays is truncated to an integer using Math.floor() before constructing the query.
| if (windowDays <= 0) { | |
| return baseQuery; | |
| } | |
| if (windowDays <= 0) { | |
| return baseQuery; | |
| } | |
| windowDays = Math.floor(windowDays); |
|
Thanks for the reviews. Addressed in the follow-up commit: Gemini (medium) — integer validation for Sourcery — CHANGELOG.md grammar: declined. Re-running the bots below. |
Sentry's `lastSeen:-Nd` relative-date syntax only accepts whole days, so a
decimal window (e.g. 14.5) would produce a malformed query that Sentry rejects
(400) or silently ignores. parseNumber gains an opt-in `{ integer: true }` and
the window flag uses it; added a unit test asserting `--sentry-window-days 14.5`
is rejected.
Coverage 100/100/100/100.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRyLiTvXUw6EQRTvXSufn3
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable recent-activity window (--sentry-window-days, defaulting to 14 days) to the Sentry release gate to prevent stale, unresolved errors from blocking scheduled releases, and adds GitHub issue alerting for blocked auto-releases. It also resolves cross-platform test failures on Windows by normalizing hardcoded POSIX path separators and introducing a win32-simulated regression guard. The code review feedback correctly identifies that the updated parseNumber utility in sentry-telemetry-gate.mjs silently converts empty or whitespace-only strings to 0 (unbounded) instead of throwing an error, and suggests explicitly validating against empty strings.
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.
| function parseNumber(value, key, fallback, { integer = false } = {}) { | ||
| if (value === null) { | ||
| return fallback; | ||
| } | ||
| const parsed = Number(value); | ||
| if (!Number.isFinite(parsed) || parsed < 0) { | ||
| if (!Number.isFinite(parsed) || parsed < 0 || (integer && !Number.isInteger(parsed))) { | ||
| fail(`Invalid --${key} value "${value}".`); | ||
| } | ||
| return parsed; |
There was a problem hiding this comment.
The parseNumber function currently converts empty or whitespace-only strings to 0 because Number("") and Number(" ") evaluate to 0. This can lead to unexpected behavior where passing an empty string to --sentry-window-days (or other numeric flags) silently disables the window (setting it to 0 / unbounded) instead of failing with an invalid value error.
Consider explicitly rejecting empty or whitespace-only strings before passing them to Number().
function parseNumber(value, key, fallback, { integer = false } = {}) {
if (value === null) {
return fallback;
}
if (typeof value === "string" && value.trim() === "") {
fail("Invalid --" + key + " value \"" + value + "\".");
}
const parsed = Number(value);
if (!Number.isFinite(parsed) || parsed < 0 || (integer && !Number.isInteger(parsed))) {
fail("Invalid --" + key + " value \"" + value + "\".");
}
return parsed;
}Reject blank numeric Sentry window values instead of coercing them to zero, and make the Sentry window contract tests less brittle about command argument ordering and source formatting. Regenerate the pm-changelog-owned changelog from the corrected pm-nb08 title and record PR #305 verification, temp-dir smoke, GitHub alert, dependency, Sentry, telemetry, and coverage evidence on pm-i84i.
|
@coderabbitai full review |
|
@codex review |
|
/gemini review |
|
To use Codex here, create a Codex account and connect to github. |
|
Thanks for the update, @unbraind. I've reviewed the final state of the pull request, including the follow-up commit addressing the integer validation for |
There was a problem hiding this comment.
Code Review
This pull request resolves issues with the daily auto-release being silently blocked by stale Sentry errors and addresses Windows-specific test failures due to hardcoded POSIX path separators. It introduces a configurable --sentry-window-days flag (defaulting to 14 days) to bound Sentry queries to a recent-activity window, adds alerting to open GitHub issues on scheduled release failures, and normalizes path assertions in unit tests using platform-native helpers. The review feedback correctly points out a potential TypeError in sentry-telemetry-gate.mjs where checking value === null fails to guard against undefined values before calling value.trim(), and suggests using loose equality value == null as a fix.
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.
| if (value === null) { | ||
| return fallback; | ||
| } |
There was a problem hiding this comment.
Using strict equality value === null does not guard against undefined values. If value is undefined, calling value.trim() on line 363 will throw a TypeError: Cannot read properties of undefined (reading 'trim'). Using loose equality value == null is the idiomatic way in JavaScript to safely check for both null and undefined.
| if (value === null) { | |
| return fallback; | |
| } | |
| if (value == null) { | |
| return fallback; | |
| } |
✅ Action performedFull review finished. |
Treat null and undefined numeric flag values as missing before trimming in the Sentry telemetry gate parser, addressing the Gemini current-head defensive review. Verified with the Sentry gate focused spec, static quality, changelog check, and full 100/100/100/100 coverage.
|
/gemini review |
|
@codex review |
|
@coderabbitai full review |
|
To use Codex here, create a Codex account and connect to github. |
✅ Action performedFull review finished. |
|
Thanks for re-triggering the review, @unbraind. As I noted previously, the changes look solid and address the identified issues effectively. Since all feedback has been incorporated and the test suite is passing, my assessment remains that this pull request is ready for merge. |
There was a problem hiding this comment.
Code Review
This pull request addresses cross-platform test failures on Windows (pm-i84i) and scheduled auto-releases being silently blocked by stale Sentry errors (pm-nb08). To resolve the Windows test failures, hardcoded POSIX path separators in unit tests were replaced with platform-native path utilities, and a simulated win32 path regression guard was introduced. To prevent stale Sentry errors from blocking releases, a new --sentry-window-days flag (defaulting to 14 days) was added to bound the unresolved error query using Sentry's lastSeen syntax, and a GitHub issue alerting mechanism was introduced for scheduled auto-release failures. There are no review comments provided, so no further feedback is available.
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.
Record the merged PR #305 evidence and release the active pm-i84i claim while leaving the broader Windows file:// TypeScript dynamic import item open for the remaining acceptance work.
Summary
Hardens daily auto-release reliability so a single stale Sentry error can no longer silently block every release, and so blocked scheduled releases are surfaced instead of skipped. Also folds in the verifiable cross-platform test fixes for the Windows nightly.
Closes pm-nb08; partial progress on pm-i84i (separator portion).
pm-nb08 — Sentry gate recent-activity window + blocked-release alerting
The
sentry-telemetry-gatequeriedis:unresolved level:[fatal,error]with no time bound, so any lingering unresolved fatal/error issue blocked every scheduled release until someone resolved it in Sentry — silently, with no alert (3 releases skipped 06-14..16, per pm-nb08).scripts/release/sentry-telemetry-gate.mjs: new--sentry-window-daysflag (default14,0= unbounded legacy) appends Sentry'slastSeen:-Ndrelative-date filter. A stale benign issue that stopped firing no longer gates; an actively firing error still does.window_daysis surfaced in the JSON + human-readable output.scripts/release/run-gates.mjs+.github/workflows/release.yml: thread / invoke the gate with--sentry-window-days 14..github/workflows/auto-release.yml: opens/updates an "Auto Release blocked" GitHub issue on scheduled-run failure (mirrors the existing pm-ehbb nightly-failure alert) so a blocked daily release is never silently skipped.docs/RELEASING.md: documents the window + alerting.lastSeen(notage/firstSeen) is deliberate: it gates on recent activity, so an old issue first seen long ago but still erroring continues to block, while one that aged out does not.pm-i84i — cross-platform test hardening (partial)
Fixes the two hardcoded POSIX-separator assertions that fail the
windows-latestnightly:init-command.spec.tsnow assertspath.resolve("/repo", "/repo/.agents/pm")(the actualresolvePmRootoutput) instead of a forward-slash literal.schema-command.spec.tsnow assertspath.join(".agents", "pm", "schema", "types.json")(thepath.join-built hint path) instead of a forward-slash literal.runtime-schema-path-win32-guard.spec.tsmocksnode:path→path.win32so a POSIX per-PR run catches a schema-path separator regression rather than only the nightly.The
file://-URL TS dynamic-import cluster (the bulk of the nightly failures) stays open in pm-i84i — it needs a Windows runner to verify, and a per-PRwindows-latestmatrix decision.Verification
lastSeen:-14dquery accepted,checked=true,critical=0 high=0.pnpm changelog:pm:checkpasses (CHANGELOG regenerated with the pm-nb08 Fixed entry).Contract coverage
release-automation-contract.spec.ts: new test locksbuildSentryGateQuery+ the--sentry-window-days 14invocation in release.yml/run-gates.ci-workflow-contract.spec.ts: locks the release.yml gate invocation and the new auto-release blocked-release alert step.🤖 Generated with Claude Code
Summary by Sourcery
Bound the Sentry-based release reliability gate to a configurable recent-activity window and added alerting for blocked scheduled auto-releases, while hardening tests for cross-platform path handling and release automation contracts.
Bug Fixes:
Enhancements:
Documentation:
Tests: