Skip to content

feat: wire age review actions to Keycast account suspension (#79)#80

Merged
dcadenas merged 5 commits into
mainfrom
feat/keycast-suspension-wiring
May 21, 2026
Merged

feat: wire age review actions to Keycast account suspension (#79)#80
dcadenas merged 5 commits into
mainfrom
feat/keycast-suspension-wiring

Conversation

@mbradley
Copy link
Copy Markdown
Member

Summary

  • Adds keycast-client.ts module with suspendUser, unsuspendUser, banUser functions calling Keycast's PUT /admin/users/:pubkey/status endpoint
  • Wires into age review state transitions: suspend on restrict, unsuspend on clear, ban on deny
  • Wires into cron auto-close: ban expired cases
  • Graceful degradation: missing config or Keycast failure never blocks a state transition
  • Adds KEYCAST_URL to staging and production wrangler configs (KEYCAST_SERVICE_TOKEN added separately via wrangler secret)

Closes #79

Prerequisites

Test plan

  • 15 new tests (10 unit, 5 integration), 160 total passing
  • Verify on staging: restrict an age review case, confirm Keycast status changes to suspended
  • Verify on staging: clear a case, confirm Keycast status returns to active
  • Verify on staging: deny a case, confirm Keycast status changes to banned
  • Verify graceful degradation: deploy without KEYCAST_SERVICE_TOKEN set, confirm age review transitions still work

@mbradley mbradley force-pushed the feat/keycast-suspension-wiring branch from 460ee67 to afa450f Compare May 21, 2026 00:25
mbradley added 3 commits May 20, 2026 20:53
Standalone Keycast admin API client for account suspension.
Three functions: suspendUser, unsuspendUser, banUser. Never throws --
returns KeycastResult { success, status?, error? }. Gracefully
degrades when not configured. Validates pubkey format before URL
interpolation. Handles SecretStoreSecret bindings.

11 unit tests covering success, 4xx, 5xx, network error, missing
config, empty token from unprovisioned SecretStoreSecret, and pubkey
validation.
handleUpdateAgeReviewCase: suspend on restricted_*, unsuspend on
cleared, ban on denied_closed (reason: age_review_denied).
checkAgeReviewDeadlines cron: ban on auto-close (reason:
age_review_expired). All calls are fire-and-forget side effects
wrapped in try/catch -- never block primary operations.

AgeReviewEnv extends both BulkModerateEnv and KeycastEnv.
KEYCAST_URL and KEYCAST_SERVICE_TOKEN added to Env interface.
7 integration tests covering all three transitions, graceful
degradation on failure/throw, and cron path assertion.
KEYCAST_URL var and KEYCAST_SERVICE_TOKEN Secrets Store binding added
to all three environments (prod, staging, test). Inert until the
secret is provisioned in the shared store.

ZENDESK_FIELD_AGE_REVIEW_DEADLINE set to 16123853180303 in all three
configs (folded from PR #77, already deployed).
@mbradley mbradley force-pushed the feat/keycast-suspension-wiring branch from 0c1f4c6 to 516cf15 Compare May 21, 2026 00:54
@mbradley mbradley marked this pull request as ready for review May 21, 2026 01:16
@NotThatKindOfDrLiz
Copy link
Copy Markdown
Member

Follow-up cleanup is pushed in aa989a1.

What changed:

  • removed the out-of-scope worker/wrangler.test.toml file so this PR no longer introduces a public workers.dev config pointed at production data with a committed admin key
  • tightened age-review side effects to trigger on the actual transition, not just the destination state
  • only suspend when a case enters a restricted state for the first time
  • only unsuspend when clearing a case that was previously restricted
  • stopped re-firing bulk restriction and Keycast suspension on restricted-to-restricted transitions
  • extracted the restricted-state helper into shared/age-review.ts so the transition logic has a single source of truth
  • typed the Keycast reason values and removed the duplicate Keycast env fields from worker/src/index.ts
  • added regression coverage for the previously unsafe under_moderator_review -> cleared path and for restricted-to-restricted transitions

Validation run locally:

  • npm run test
  • ./node_modules/.bin/tsc --noEmit
  • cd worker && ../node_modules/.bin/vitest run

GitHub checks are green on the updated SHA.

Copy link
Copy Markdown
Member

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Choose a reason for hiding this comment

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

Final pass looks clean. The unsafe test wrangler file is removed, the Keycast/account-status transitions are now gated correctly, and the new regression coverage closes the previously unsafe clear-path behavior. Local validation and GitHub checks are green on aa989a1.

Copy link
Copy Markdown

@dcadenas dcadenas left a comment

Choose a reason for hiding this comment

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

The Keycast account state must be fixed before merge. Clearing an age review case can currently leave the account suspended after the case moves through review or follow-up states.

Comment thread worker/src/age-review.ts Outdated
const enteredRestrictedState = requestedState !== undefined
&& isAccountRestrictedAgeReviewState(requestedState)
&& !isAccountRestrictedAgeReviewState(existing.state);
const clearedRestrictedState = requestedState === 'cleared'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make the clear transition unsuspend accounts that already passed through a restricted age-review state, not only accounts whose immediately previous state is restricted.
A normal path is restricted_pending_* -> submitted_for_review or needs_follow_up -> cleared, and the middle transition intentionally leaves Keycast suspended.
With the current clearedRestrictedState check, the final clear skips unsuspendUser and leaves the account suspended after the case is cleared.
Add a regression test for restricted_pending_* -> submitted_for_review -> cleared, and cover needs_follow_up if that path can also follow a suspension.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in b4b9f3e. Replaced clearedRestrictedState (which required the immediately prior state to be restricted) with an unconditional check on requestedState === 'cleared'. Both unsuspendUser and un-age-restrict-all are idempotent, so they fire on every clear regardless of prior state.

Added regression tests for restricted_pending_*submitted_for_reviewcleared and needs_follow_upcleared.

Clearing a case that passed through intermediate states
(submitted_for_review, needs_follow_up) after restriction skipped
the Keycast unsuspend and bulk un-age-restrict calls because the
check required the immediately prior state to be restricted.

Both calls are idempotent, so fire them on every clear regardless
of prior state.

Adds regression tests for submitted_for_review → cleared and
needs_follow_up → cleared paths.
@mbradley mbradley requested a review from dcadenas May 21, 2026 13:16
Copy link
Copy Markdown

@dcadenas dcadenas left a comment

Choose a reason for hiding this comment

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

✅ Keycast age actions land cleanly.

@dcadenas dcadenas merged commit ea4d1a4 into main May 21, 2026
3 checks passed
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.

task: wire age review actions to Keycast account suspension

3 participants