[ndisidore]

Adds a PagerDuty gatekeeper. Agents can read incidents, services, teams, schedules and who is on call, and can act on incidents and schedule overrides through the approval queue. Incident response is where an agent can triage, summarize and nudge the right people, and PagerDuty is where that work happens. Optional event hooks let a gadget react to incidents as they change.

• Five grants, one picker. Account, team, service, incident or schedule, all chosen in one HierarchyPicker configurator. The account is the root; Teams, Services, Schedules and Open incidents are tabs. A resource can be reached from several tabs, but binds the same URL whichever way you pick it
• Scoped OAuth with user tokens. A private app on the deployment's own account (e.g. cloudflare.pagerduty.com), using PKCE and a client secret. Cloudflare Access needs nothing extra, since PagerDuty's sign-in does the SSO
• Identity from PagerDuty's ID token: user, account, subdomain and region. Checks RS256 signature, issuer and audience (CLIENT_ID). Refuses a grant missing scopes, since PagerDuty drops them silently
• US and EU accounts. Region picks the API host
• New observer contract. Every read reports the IDs it depended on, as region/<us|eu>/account/<id>[/<kind>/<id>]: the binding's own, any named in filters, and those in the results. Empty pages and cache hits report them too. A collaborator is checked by opening each object with their own login; a 403 or 404 denies. Answers cached 15 min, denials 15 s
• Errs toward denial. An incident page reports each distinct service on it. Team listings report every row, not just the team, since a non-member may still be able to open a private team
• Narrow bindings stay narrow. A team, service, incident or schedule binding refuses objects outside it
• Auto-approval verdicts.
  • Auto-approvable: acknowledge, snooze, resolve, edit title/priority/urgency, add notes
  • Never: reassign, escalate, request responders, post status updates, create incidents, delete overrides. These page or email people, or delete
  • Overrides: auto-approvable only when you put yourself on call. Implemented as two action types under one manage-overrides rule
• REST POST /incidents, not Events API v2. This is PagerDuty's recommendation for anything that isn't a monitoring tool. It's synchronous, and acts on the user's own grant and permissions
• Pending writes show in reads: incident fields, notes, created incidents (number: 0 until approved), overrides and shifts
• Hooks need manual setup. An admin creates one account-wide Generic Webhook (v3) in PagerDuty and sets PAGERDUTY_WEBHOOK_SECRET; steps are in the README. Without the secret, subscribe() throws, the route returns 404, and install still works
• One hook driver per deployment, fenced by account. The HMAC signature is checked. Deduped on the signed event.id, not the unsigned X-Webhook-Id header. Events more than a day old are dropped. Each delivery re-reads the incident as the hook's owner, and only reaches hooks on the account named in the incident's signed link
• Hooks on account, team, service and incident bindings, not schedules. Your own changes are delivered too, flagged with actor.isYou. A note the webhook trimmed is fetched in full; a trimmed status update is left out, since PagerDuty has no endpoint to read one

Not included: incident workflows, merging incidents, Events API v2, maintenance windows, escalation-policy edits, status pages, custom fields, analytics, or creating the webhook subscription automatically.

Known gaps:

• A pending resolve doesn't make an incident appear under statuses: ["resolved"]
• Hook events older than a day are dropped, though PagerDuty retries for 48 h
• Renaming the account's subdomain detaches existing hooks

Tested:

• The package's unit and workerd suites, against a fake PagerDuty that plays the identity service and the US and EU REST hosts. This includes a webhook test with the same incident ID in two accounts
• The release manifest test, lint, build, and wrangler deploy --dry-run

Not run: a live Workshop, a real PagerDuty account or webhook, or the configurator in a browser.

Plan and code written by Claude Opus 5.5.

---

[github-actions]

Preview: pr1050-feat-gk-pagerduty

https://pr1050-feat-gk-pagerduty-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

[ask-bonk]

Review: 4 findings.

Posted all 4 findings inline in one review. Verified with in-memory reproductions and PagerDuty’s API documentation.

github run

[ask-bonk]

Review: 3 findings.

Posted all 3 inline in one review. Verified against production-code reproductions and PagerDuty’s API documentation.

github run

[clanky-the-clanker]

Superseded by the review of f7cc2688.

This review, of 84c50e1a

💬 Approved with comments — Both earlier warnings are fixed. One README note remains, about pending incidents, and it does not block merging.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

One README statement does not hold for incidents still pending creation.

Recommendation: 💬 Optional Nits

• [INFO] The README says incident reads report the incident's ID, but a pending incident has no ID yet. (packages/gatekeeper-pagerduty/README.md:164)

View full review

• README.md:164-165 says an incident read also reports the incident. An incident still pending creation has no PagerDuty ID yet, so its reads report only its service.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: low. The golden manifest, the fixture bundle and the worker config agree.

---

🔒 Security: ✅ LGTM

No security issues found:

• ID token: the signature, issuer and audience are checked, and identity fields are held to strict patterns.
• Request paths: agent-supplied IDs are held to PagerDuty's ID grammar before they reach a path.
• Webhook: the body is size-capped before verification, the HMAC is compared in constant time, deliveries are deduplicated on the signed event ID and fenced to the signed account, and each delivery re-reads the incident as the hook's owner.
• Observer checks: each object is probed with the observer's own token, and a 403 or 404 denies.
• Test key: the RSA key in __tests__/workerd/id-token-key.json is a test fixture only.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• once() classification (src/pagerduty-actions.ts): the try/catch now sits inside the callback passed to host.write. Identity and credential errors thrown before the request are retried as they came. The test at actions.test.ts:207-224 covers this.
• Override test dates (__tests__/workerd/actions.test.ts:43-48): START and END now come from the next midnight UTC, so the tests stay valid whenever the suite runs.

---

⚡ Performance: ✅ LGTM

No issues found. Requests have timeouts, response bodies are capped, and pagination stops at PagerDuty's offset cap.

---

_{Reviewed commit: 84c50e1a}

[devin-ai-integration]

Devin Review found 1 new potential issue.

[ask-bonk]

Review: 2 findings.

Posted both P2 findings inline in one review. Verified through in-memory reproductions using the production public-method flows.

github run

[clanky-the-clanker]

Superseded by the review of c60716ea.

This review, of f7cc2688

💬 Approved with comments — No code defects found. One WARNING: the README overstates how fully pending changes are simulated in reads.

Findings: 1 warning (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #1050.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] README says every pending change is shown as if applied, but escalation-policy reassignment, responder requests, status updates and removal of an existing override are not reflected in reads.
- **File:** packages/gatekeeper-pagerduty/README.md:6

---

📝 Documentation: ⚠️ 1 warning

The README says every pending change appears in reads as if applied, but some don't.

Recommendation: 📝 Minor Comments

• [WARNING] README says every pending change is shown as if applied, but escalation-policy reassignment, responder requests, status updates and removal of an existing override are not reflected in reads. (packages/gatekeeper-pagerduty/README.md:6)

View full review

• [WARNING] packages/gatekeeper-pagerduty/README.md:6-7 says every pending change is shown as if it had already happened. Several are not:

  • overlayIncident (src/pagerduty-simulation.ts:42-83) ignores a pending reassignment to an escalation policy, pending responder requests and pending status updates.
  • overlayShifts and overlayOnCalls (src/pagerduty-simulation.ts:211-249) don't reflect removing an override PagerDuty already has.
  • A pending resolve doesn't make an incident appear under statuses: ["resolved"], a gap the pull request description lists.

  Qualify the sentence, and list these gaps under Limitations.

The earlier note about pending incidents is fixed: README.md:164-166 now says a pending incident's reads report only its service.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: low. The golden manifest, the fixture bundle and the worker config agree.

---

🔒 Security: ✅ LGTM

No security issues found:

• Pending rows: a row that overlay drops is never treated as authorized.
• Simulation: pagerduty-simulation.ts is pure over the user's own pending entries and sends no requests.
• Agent-supplied IDs: they are still held to PagerDuty's ID grammar before they reach a request path.
• Observer checks: every read reports the account, the binding's scope, the filter IDs and the result IDs.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

No issues found:

• Cursor overlay (gatekeeper-kit/src/cursors.ts): #serve applies revalidate to injected rows and overlay to provider rows, then authorizes the row it serves. When a page is refused, the retry stops at the queue entries that page covered (held.covered), not at row object identity, so it is correct when overlay returns a fresh object. The new tests in cursors.test.ts cover this case.
• Pending incidents and on-call (pagerduty-session-shared.ts, pagerduty-simulation.ts): revalidate, overlay and identity fit the kit's CursorInjection contract. The services of injected rows are authorized before the cursor serves any page. The replaceWithin and overlayOnCalls logic handles these cases:
  • an override spanning two entries
  • an override created and then deleted while still pending

---

⚡ Performance: ✅ LGTM

No issues found.

---

_{Reviewed commit: f7cc2688}

[ask-bonk]

Review: 2 findings.

Posted both P2 findings inline in one review. Verified with in-memory reproductions using the production public-method flows and PagerDuty’s API documentation.

github run

[clanky-the-clanker]

Superseded by the review of 83890371.

This review, of c60716ea

💬 Approved with comments — No code defects found. One WARNING remains: the README says every pending change appears in reads as if already applied, and several kinds of change do not.

Findings: 1 warning

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #1050.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] The README says every pending change is shown as if already applied, but reads do not reflect these pending changes: a reassignment to an escalation policy, responder requests, status updates, removal of an existing override, and a resolve under the resolved-status filter.
- **File:** packages/gatekeeper-pagerduty/README.md:6

---

📝 Documentation: ⚠️ 1 warning

The README overstates how completely pending changes are simulated in reads

Recommendation: 📝 Minor Comments

• [WARNING] The README says every pending change is shown as if already applied, but reads do not reflect these pending changes: a reassignment to an escalation policy, responder requests, status updates, removal of an existing override, and a resolve under the resolved-status filter. (packages/gatekeeper-pagerduty/README.md:6)

View full review

• [WARNING] packages/gatekeeper-pagerduty/README.md:6-8 says every pending change is shown as if it had already happened. That is not true for these cases:

  • overlayIncident (src/pagerduty-simulation.ts:43-86) only handles a reassignment to users. It ignores a pending reassignment to an escalation policy, pending responder requests and pending status updates.
  • A pending removal of an override PagerDuty already has does not show in shifts or on-call entries.
  • A pending resolve does not make an incident appear under statuses: ["resolved"].

  Qualify the sentence, and list these gaps under Limitations.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: low. The golden manifest entry, the fixture bundle's size and hash, deploy-inputs.json and the worker config's migrations all agree.

---

🔒 Security: ✅ LGTM

I found no security issues:

• Webhook: the receiver checks the HMAC signature in constant time before it parses the body. It delivers an event only to hooks on the account named in the signed incident link, and it re-reads the incident as the hook's owner.
• IDs: IDs supplied by agents must match PagerDuty's ID format before they reach a request path.
• Simulation: the simulation code is pure and sends no requests.
• Overlay: a row that overlay drops is never treated as authorized.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

I found no issues:

• Simulation: the incident and on-call overlays in pagerduty-simulation.ts handle these cases correctly:
  • an override that spans two entries
  • an override created and then deleted while still pending
  • a pending incident's own defaults
• Errors at apply time: the actions in pagerduty-actions.ts sort errors as follows:
  • 401 and 429 answers are retried.
  • Other 4xx answers become apply errors.
  • Everything else becomes an unknown outcome.
• Scope: each apply checks the binding's scope again with a fresh read.
• Cursor overlay: the overlay in gatekeeper-kit/src/cursors.ts stays correct when overlay returns a new object.

---

⚡ Performance: ✅ LGTM

I found no performance issues. Every request has a timeout and a cap on the response size, pagination is bounded, and the policy and service lookups are cached.

---

_{Reviewed commit: c60716ea}

[ask-bonk]

Review: 1 finding.

Posted one P2 finding inline, verified with an in-memory reproduction using the production public-method flow.

github run

[clanky-the-clanker]

Superseded by the review of 011f9f1f.

This review, of 83890371

💬 Approved with comments — No code defects found. One WARNING remains: the README (and the simulation module's header comment) says every pending change shows in reads as if applied, and several kinds of pending change do not.

Findings: 1 warning

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #1050.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] The README says every pending change is shown as if applied, but reads do not reflect these pending changes: a reassignment to an escalation policy, responder requests, status updates, and the removal of an override PagerDuty already has.
- **File:** packages/gatekeeper-pagerduty/README.md:6

---

📝 Documentation: ⚠️ 1 warning

One WARNING remains: the README says every pending change shows in reads, and several do not

Recommendation: 📝 Minor Comments

• [WARNING] The README says every pending change is shown as if applied, but reads do not reflect these pending changes: a reassignment to an escalation policy, responder requests, status updates, and the removal of an override PagerDuty already has. (packages/gatekeeper-pagerduty/README.md:6)

View full review

• [WARNING] packages/gatekeeper-pagerduty/README.md:6-8 says every pending change is shown to the gadget as if it had already happened. The header comment of src/pagerduty-simulation.ts:1-8 makes the same broad claim. Several pending changes do not show in reads:

  • overlayIncident (src/pagerduty-simulation.ts:43-86) only applies a reassignment to users. It ignores a pending reassignment to an escalation policy, pending responder requests and pending status updates.
  • overlayShifts and overlayOnCalls leave in place an override PagerDuty already has that is pending removal. Their own doc comments say so.

  Qualify the sentence, and list these gaps under Limitations. The resolved-status filter gap from the last review is fixed.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: low. The golden manifest entry, the fixture bundle's size and hash, deploy-inputs.json and the worker config's migrations all agree.

---

🔒 Security: ✅ LGTM

I found no security issues.

• Hook scope: a hook's scope comes from the capability's own context, so a gadget cannot widen it. Every delivery re-checks that scope against a fresh read made as the hook's owner. Deliveries are also fenced to the account named in the signed incident link, both in the driver's pre-filter and again in the facet.
• Webhook: the body size is capped, and the HMAC is checked in constant time before the body is parsed. Deduplication uses the signed event ID, and stale events are dropped.
• IDs: agent-supplied IDs must match PagerDuty's ID format before they reach a request path.
• Cursor overlay: a row that overlay drops is never treated as authorized.
• Identity: the ID token is checked for signature (RS256), issuer and audience. Observer checks deny any subresource outside the connection's own region and account.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

I found no issues.

• Kit cursor retry (gatekeeper-kit/src/cursors.ts): rows are matched by queue entry, not by object identity, so the logic stays correct when overlay returns new objects. On a retry after a refused page, a provider row the refusal did not cover ends the page. A row the refusal covered, or one overlay dropped, is consumed.
• Scope at apply time: team, service and incident bindings re-check their scope with fresh reads (requireInScope, currentIncident). Listings re-check it on every page.
• Simulation: the overlays are pure. They handle an override created and then deleted while still pending, an override spanning two on-call entries, a pending incident's defaults, and a user filter that a pending override puts on call.
• Release wiring: the fixture bundle hash, the golden manifest, cloudflare.config.ts and wrangler.jsonc agree, including the migrations.

---

⚡ Performance: ✅ LGTM

I found no performance issues.

• Re-running the cursor's serve-time overlay on a retry is bounded by the page size and makes no extra requests.
• Every request has a timeout and a cap on the response size, and listings stop at PagerDuty's offset cap.
• The v3-then-v2 schedule probe costs one extra request for a layer-based schedule, and its result is cached.

---

_{Reviewed commit: 83890371}

[ask-bonk]

Review: 1 finding.

Posted one new P2 finding inline, verified with an in-memory reproduction using the production public-method flow.

github run

[clanky-the-clanker]

Superseded by the review of 09f0ef21.

This review, of 011f9f1f

💬 Approved with comments — No code defects. The README now describes the pending-change gaps accurately; only an optional fix to a code comment remains.

Findings: 1 suggestion (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 suggestion

README fixed; the simulation's header comment still says every pending action shows in reads

Recommendation: 💬 Optional Nits

• [SUGGESTION] The simulation's header comment says reads show every pending action as applied; several kinds of pending change are not shown. (packages/gatekeeper-pagerduty/src/pagerduty-simulation.ts:1)

View full review

The README now qualifies the claim and lists the pending changes that reads do not show. The header comment of src/pagerduty-simulation.ts still opens with "every pending action", which overstates what the simulation does.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The golden manifest entry, the fixture bundle's size and hash, deploy-inputs.json and the worker config's migrations all agree.

---

🔒 Security: ✅ LGTM

I found no security issues.

• Webhook: the HMAC is checked in constant time before the body is parsed, events are deduped on the signed event ID, stale events are dropped, and deliveries are fenced to the account named in the signed incident link.
• Hook delivery: each delivery re-checks the hook's scope against a fresh read made as the hook's owner.
• Input: agent-supplied IDs are validated against PagerDuty's ID format before they reach a request path.
• Identity: the ID token's RS256 signature, issuer and audience are verified.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

I found no issues.

• Scope at apply time: each action now carries the team or service of the capability that staged it. requireInScope and currentIncident re-read the object, so anything that has since left the capability's reach is refused.
• Kit cursor: rows are matched by queue entry rather than object identity, so a retry stays correct when overlay returns new objects.

---

⚡ Performance: ✅ LGTM

I found no performance issues.

---

_{Reviewed commit: 011f9f1f}

[ask-bonk]

Review: 1 findings.

Posted one new P2 finding inline, verified with an in-memory reproduction using production public methods and PagerDuty’s documentation.

github run

[clanky-the-clanker]

Superseded by the review of 099f0011.

This review, of 09f0ef21

✅ Approved — No findings. The one remaining note from the last review, about the simulation's header comment, has been fixed.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low. The golden manifest, the fixture bundle, deploy-inputs.json and the migrations agree.

---

🔒 Security: ✅ LGTM

No security issues found:

• The webhook's HMAC is checked in constant time, events are deduped on the signed event ID, and deliveries go only to hooks on the event's account.
• The ID token is verified.
• IDs that reach a request path are checked against PagerDuty's ID format first.
• The only key material in the change is a test-only key used to sign ID tokens in the tests.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

No issues found. The kit cursor now matches retried rows by queue entry. When an action is applied, its scope is re-checked with fresh reads.

---

⚡ Performance: ✅ LGTM

No issues found.

---

📝 Documentation: ✅ LGTM

No issues found. The earlier note about the simulation's header comment is fixed: the comment now lists which pending changes reads don't show.

---

_{Reviewed commit: 09f0ef21}

[clanky-the-clanker]

Superseded by the review of edeaf8e8.

This review, of 08621ffa

✅ Approved — The only file changed since the last approved review moves PagerDuty's hooks onto the kit's HookRegistry. Webhook verification, the account fence and the per-delivery authorization are all unchanged, and no issues were found in any area.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low

---

🔒 Security: ✅ LGTM

Webhook handling in pagerduty-hooks.ts:

• The body is size-capped before the HMAC is checked, and the comparison runs in constant time.
• Signature rotation is supported.
• Stale events and events with an unreadable timestamp are dropped.
• The account fence is taken from the signed incident link and checked in both watches and deliver.
• Each delivery re-reads the incident as the hook's owner before calling authorize.

Agent-supplied IDs are validated before they reach request paths. Actions that page, email or delete are never auto-approvable.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

pagerduty-hooks.ts uses the kit's HookRegistry correctly:

• register() runs inside transactionSync, and unregister() reschedules the alarm.
• ingest() dedupes on the signed event.id.
• Delivered stubs are disposed after each delivery.

The kit cursor's overlay and refresh hooks behave as their tests expect, including a refused page's retry and a refresh that throws.

---

⚡ Performance: ✅ LGTM

No issues found.

---

📝 Documentation: ✅ LGTM

No issues found.

---

_{Reviewed commit: 08621ffa}

[ask-bonk]

Review: 3 findings.

All three remain reproducible at 08621ffa and already have open threads:

• [P1] Retained service cursors bypass the originating team’s scope — pagerduty-sessions.ts:227. After the service moves to another team, getDetails() refuses, but the cursor fetches and returns another responder. Revalidate scope before every page, following the narrow-binding invariant in .agents/skills/write-gatekeeper/SKILL.md.

• [P2] Buffered on-call pages retain rejected overrides — pagerduty-session-shared.ts:553. After rejecting a pending override, the retained cursor still returns its replacement user while a fresh cursor returns the original user. Preserve provider entries and re-derive their remaining pieces at serve time, as packages/gatekeeper-kit/USAGE.md requires.

• [P2] Captured user filters exclude newly staged coverage — pagerduty-session-shared.ts:544. Opening an Alice-filtered cursor before staging her override returns null; a fresh cursor correctly finds her. Reevaluate filter pushdown before fetching candidates.

Verified with in-memory reproductions using production session methods, REST client and cursor implementation. No additional findings in the hook-registry refactor; no duplicate inline comments posted.

github run

[clanky-the-clanker]

Superseded by the review of 74a486c0.

This review, of edeaf8e8

✅ Approved — The cursor overlay/refresh hooks still fail closed on a refused page's retry and when refresh throws. The PagerDuty changes since the last review (on-call entries scoped as each page is served, team incident listings, the override existence check) are consistent and tested. No issues found.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low

---

🔒 Security: ✅ LGTM

• Refused pages: a retry still fails closed. authorizePage sees only the provider rows as served, after overlay has run, and a refresh that throws fails that next().
• Agent-supplied IDs: they are held to [A-Z0-9]{1,64} before they reach request paths.
• OAuth: the flow uses PKCE and single-use nonces. A reconnect is pinned to the original user and account, and its new credentials are staged until the Workshop confirms them.
• Observer access: checkSubresourceAccess fails closed.
• Webhooks: verification, the account fence and the per-delivery re-read haven't changed since the last review.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• cursors.ts:
  • A refused page's retry matches its rows by queue entry (held.covered), so overlay returning fresh objects doesn't break the match.
  • An entry that now yields rows the refusal didn't cover starts the next page.
  • The pieces of a split row are checked as one entry.
  • refresh runs before any held-page state changes, so if it throws, the buffer is left untouched.
  • The new tests cover each of these paths.
• PagerDuty sessions: reads re-check the binding's team or service scope on every call, and leave provisional ~N IDs out of their observations. Each cursor releases its duplicated queue stub when the walk is dropped.
• Actions: writes that can't be safely repeated (snooze, notes, responder requests, status updates, creating incidents and overrides) go through once() with claimBeforeApply.

---

⚡ Performance: ✅ LGTM

No issues found.

---

📝 Documentation: ✅ LGTM

No issues found in the changed documentation.

---

_{Reviewed commit: edeaf8e8}

[ask-bonk]

Review: 1 finding.

[P1] Injected incident pages bypass serve-time authorization — packages/gatekeeper-pagerduty/src/pagerduty-session-shared.ts:489

Provider-derived incidents are authorized when the cursor is created, but injected-only pages skip authorization when returned. Using production session methods, REST client and cursor, I allowed creation, then configured authorization to refuse subsequent reads: both next() calls still disclosed incidents, with no additional authorization call.

The stored observer requirements protect later admission, but cannot replace the kernel’s live per-observation checks for already-admitted collaborators who lose access. Authorize every returned page—including injected rows—before consuming it, preserving refusal/retry behavior. This follows the per-read authorization contract.

Already covered by the existing thread; no duplicate inline comment posted.

The retained-service scope check and buffered on-call simulation fixes passed the in-memory reproductions.

github run

[clanky-the-clanker]

✅ Approved — Rows injected with inject.authorize now reach authorizePage, including pages made only of them, and that path still fails closed on a refused page and when refresh throws. No issues found.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low

---

🔒 Security: ✅ LGTM

• Every injected page is authorized: a page made only of injected incidents now goes through authorizePage (pagerduty-session-shared.ts, cursors.ts #admit and #fill).
• Refused pages: if authorizePage throws, the page stays held, and its retry rebuilds from the entries that page consumed. A provider row the refusal did not cover leads the next page.
• Changed incidents: listIncidents still reads them as the user. current() swallows only denials, so a denied incident is left out and any other error is rethrown.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Kit cursor (cursors.ts):
  • With inject.authorize set, #admit adds injected rows to page.provided. A page made only of injected rows is therefore not injectedOnly, and it reaches authorizePage.
  • Injections without the flag behave as before.
  • The new tests cover both cases.
• listIncidents:
  • It sets inject: { authorize: true }.
  • The result callback reports the service of every incident it serves, injected ones included.
  • refresh re-reads the team's services and the pending actions before each page, so a service that has left the team drops out of both overlay and revalidate.
• Earlier discussion: this resolves the open thread about authorizing injected incident rows on each page.

---

⚡ Performance: ✅ LGTM

• refresh: it runs once per next(). The team-services lookup goes through the 5-minute cache, so walking a listing doesn't add a PagerDuty request per page.
• overlay and revalidate: they run in memory, bounded by the page size and the number of pending actions.

---

📝 Documentation: ✅ LGTM

The changes to USAGE.md and the plan describe the new inject.authorize option and the per-page refresh hook, and they match the code.

---

_{Reviewed commit: 74a486c0}

[ask-bonk]

LGTM!

github run