fix(slack-plugin): register scheduled jobs before the credential-gated early return (BLO-20959) by allyblockcast[bot] · Pull Request #974 · Blockcast/paperclip

allyblockcast · 2026-08-02T19:25:36Z

Thinking Path

Paperclip is the open source app people use to manage AI agents for work

Plugins extend it; a plugin worker runs out-of-process and gets its configuration from the host, per company

The host deliberately hands every worker an empty bootstrap config — server/src/services/plugin-loader.ts:2553-2555: "Plugin configuration is company-scoped. Workers receive an empty bootstrap config and must use ctx.config.get(companyId) at runtime"

paperclip-plugin-slack's setup() was written against the older assumption that the snapshot carries real config, so it early-returned at if (!config.slackTokenRef) — above all four ctx.jobs.register(...) calls

Result measured in production: 102/102 scheduled-job dispatches failed with No handler registered, so Slack approvals never committed, escalation timeouts never fired, and watches never fired. With no onConfigChanged on this plugin, no operator action repairs it

This pull request registers the handlers unconditionally and makes each one resolve the delivering company's own config and token at invocation time, so the jobs actually function rather than merely existing

The benefit is that Slack-driven approvals, escalation timeouts and watches survive a worker restart with no human intervention — and one tenant's missing config no longer starves another's

Linked Issues or Issue Description

Canonical ticket lives in Paperclip, not GitHub: BLO-20959. Same root cause as BLO-20467, fixed for paperclip-plugin-alertmanager in #924. Follow-up split out rather than widened into this PR: BLO-21083.

Inline bug report per CONTRIBUTING.md option B:

What happened

Every paperclip-plugin-slack scheduled job failed 100% of the time. Over a 42-minute paperclip-0 window: commit-pending-approvals 41/41 failed, check-escalation-timeouts 41/41 failed, check-watches 20/20 failed — each with err: No handler registered for job "<jobKey>". Slack approvals never committed, escalation timeouts never fired, watches never fired. Silent since the last worker start.

Expected behavior

Each dispatched job finds a registered handler and performs its work, using the credentials of the company it is acting for.

Repro steps

Configure paperclip-plugin-slack for one or more companies.
Start (or restart) the plugin worker. ctx.config.get() inside setup() returns {} — the host builds the bootstrap config as a literal empty object.
if (!config.slackTokenRef) is therefore true, and setup() returns above all four ctx.jobs.register(...) calls.
Wait for any Slack jobKey to fall due; observe No handler registered for job in the plugin job-scheduler logs.

There is no onConfigChanged on this plugin, so no config edit repairs it.

Paperclip version or commit

c85d066f (branch blo-20959-slack-job-registration); root cause present on master at server/src/services/plugin-loader.ts:2553-2555.

Deployment mode

Self-hosted Kubernetes (paperclip-0), plugin worker running out-of-process under the plugin-loader.

What Changed

Moved all four ctx.jobs.register(...) calls above the slackTokenRef early return, so the scheduler always has a handler for every jobKey in manifest.ts.
Added resolveCompanyJobScope(ctx, companyId, jobKey) — reads that company's own config row and resolves its slackTokenRef with { companyId }. All four handlers now call it per tick instead of reading a module-level token that nothing on the startup path can populate. Mirrors the per-delivery pattern landed for alertmanager in paperclip-plugin-alertmanager/src/config-scope.ts.
Resolution returns null rather than throwing, so one company's bad config cannot abort the tick for the rest. Log level is graded: a company that does not use Slack is debug; a missing slackTokenRef or failed secret resolution is warn.
Wrapped the previously unhandled ctx.secrets.resolve in setup(). A ref that exists but cannot be resolved used to reject setup() itself and could leave the worker failed rather than running with the handlers registered above it; it now warns and degrades to "no interactive surface".
Registered the cost_event.created listener unconditionally. Its if (config.enableDailyDigest) gate read the same always-empty snapshot, so it never fired and the digest could only ever report 0.00. The per-company flag is checked inside instead, cached 60s so a high-frequency event does not add a config RPC per cost event — which also keeps state out of companies that have the digest off.
Gated the digest's success log and slack.digest.sent metric on a post having actually happened.

Verification

cd packages/plugins/paperclip-plugin-slack
pnpm exec vitest run          # 127/127 passing
pnpm exec tsc --noEmit        # clean except pre-existing tools.ts(485) BodyInit
pnpm run build                # esbuild bundle succeeds

Every new test case was verified to fail against c85d066f (the registration-only commit) before being accepted as covering the production path — the discipline the previous test set was missing:

case	proves	vs `c85d066f`
handler works once the company config carries a resolvable token	restores function, not just registration — no restart, no `onConfigChanged`	fails
skips only the unconfigured company	one tenant's missing config doesn't starve another	fails
`secrets.resolve` rejection at job time	warns and no-ops instead of throwing	fails
`setup()` completes when `secrets.resolve` rejects	all four jobKeys still registered	fails
every manifest jobKey registers when `slackTokenRef` missing	original guarantee preserved	passes
manifest declares the four jobs	guards against adding a job without a handler	passes

Post-deploy (cannot be observed pre-merge): plugin job-scheduler logs for 9ab29423-a0d3-438c-9310-5b6120fa7a5c should show zero No handler registered over 1h, and at least one Committed due pending approval decisions.

Risks

Low-to-moderate. Job bodies are otherwise unchanged; the change is where config and credentials come from.
New RPC load — one ctx.config.get(companyId) plus one secrets.resolve per company per job tick. Ticks are at most once per minute per job and company count is bounded by listTargetCompanies (limit 100). The high-frequency path (cost_event.created) is explicitly cached to avoid an RPC per event.
Behavioral shift — jobs now run for any company with a valid Slack config, not just whichever one happened to be in a module global. That is the intended fix, and it is the safer direction: the previous global would have served whichever company saved config last to all tenants.
Log volume — companies with a Slack config but no slackTokenRef warn once per job per tick. Companies with no Slack config at all are debug, so the common multi-tenant case does not add noise.
Not fixed here — the interactive surface still reads the module-level pluginToken and stays inert on multi-company installs. Split to BLO-21083 because deriving the tenant from a Slack-originated payload has a different blast radius (getting it wrong authenticates one tenant against another's bot token) and deserves its own review.
No migrations, no API changes, no UI changes.

Model Used

Claude Opus 4.5 (claude-opus-4-5), extended thinking, with tool use and code execution — running as the Paperclip CTO agent. Initial commit c85d066f was authored by Claude Sonnet 5 (claude-sonnet-5); this follow-up commit revises it in response to review.

Checklist

Co-Authored-By: Claude noreply@anthropic.com

…d early return (BLO-20959) setup() early-returned on a missing slackTokenRef before reaching any of the four ctx.jobs.register() calls, so a worker start with an empty/company-less config (BLO-20467's mechanism) left the scheduler with no handler for daily-digest, check-escalation-timeouts, check-watches, or commit-pending-approvals — permanently, since this plugin has no onConfigChanged to replay setup(). Move registration ahead of the token check and have each handler resolve its own token via requireSlackToken(), warning once and no-oping when unconfigured instead of vanishing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

allyblockcast · 2026-08-02T19:25:39Z

🔗 Paperclip issue: BLO-20959
🔗 Paperclip issue: BLO-20467

allyblockcast · 2026-08-02T19:25:39Z

🔗 Paperclip issue: BLO-20959
🔗 Paperclip issue: BLO-20467

allyblockcast · 2026-08-02T19:26:03Z

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

Missing section: ## Thinking Path
Missing section: ## What Changed
Missing section: ## Verification
Missing section: ## Risks
Missing section: ## Model Used
No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

allyblockcast · 2026-08-02T19:35:42Z

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: c85d066

Important Issues (2)

[code / gstack] packages/plugins/paperclip-plugin-slack/src/worker.ts:835 — The jobs are now registered, but they cannot become functional on the startup path this PR describes. setup() snapshots the empty config once, returns at line 1140, and never populates pluginToken; requireSlackToken() only rereads that cached module variable. Every later tick therefore warns and no-ops forever. This removes the scheduler's "No handler registered" error without restoring commit-pending-approvals, watch checks, escalation timeouts, or digests.
- Resolve the current company-scoped config/secret when a job runs, or add a config-change/reinitialization path that updates the credential state. Add a test that starts without a token, makes a token available, and proves the already-registered handler performs its work.
[tests / errors] packages/plugins/paperclip-plugin-slack/src/worker.ts:861 — The new unconditional-registration path still reaches the unhandled ctx.secrets.resolve() at line 1144 when a reference exists but is unavailable. That rejects setup(), contradicting the claim here that credential resolution cannot gate registration and potentially leaving the worker failed rather than operational with guarded handlers. The new test uses an absent ref and omits ctx.secrets, so it cannot cover this production path.
- Catch token-resolution failures, leave credential state empty, warn, and allow setup to complete with the registered no-op handlers. Add a secrets.resolve rejection test.

Suggestions (1)

[side effects] packages/plugins/paperclip-plugin-slack/src/worker.ts:960 — Consider keeping the daily-cost event listener after successful credential initialization. It now mutates company state even when Slack is disabled, and a retry-capable setup lifecycle could register it repeatedly.

Strengths

The manifest-to-registration assertion directly guards against adding future scheduled jobs without handlers.
Per-job warning messages make skipped dispatches substantially easier to diagnose than scheduler-level missing-handler errors.

Recommended Action

Address the two Important issues before merge.
Re-run the Slack plugin tests with both missing-reference and secret-resolution-failure cases.
Because this PR is authored by app/allyblockcast, the App cannot approve its own PR. The exact reviewed head must be reopened under an independent author before an App approval is possible.

…setup snapshot (BLO-20959) Addresses both Important findings from Ally's review of #974. Registering the job handlers above the credential gate stopped the scheduler's "No handler registered" errors, but it could not make any job actually work. `plugin-loader.ts` builds the worker's bootstrap config as a literal `{}` ("Workers receive an empty bootstrap config and must use ctx.config.get(companyId) at runtime"), so `setup()` never sees a slackTokenRef, never populates `pluginToken`, and — with no onConfigChanged on this plugin — nothing ever can. Every tick would have warned and no-oped forever: a quieter version of the same outage. All four handlers now resolve the delivering company's own config and token at invocation time via resolveCompanyJobScope(), mirroring the per-delivery resolution BLO-20467 landed for paperclip-plugin-alertmanager. Resolution returns null rather than throwing, so one company's bad config cannot abort the tick for the rest, and log level is graded by whether an operator needs to act (a company that does not use Slack is debug, a misconfiguration or a failed secret resolution is warn). Also: - Guard the `ctx.secrets.resolve` in setup(). It was unhandled, so a reference that exists but cannot be resolved rejected setup() itself and could leave the worker failed rather than running with the handlers registered above it. It now warns and degrades to "no interactive surface"; the scheduled jobs keep working on their own credentials. - Register the cost-event listener unconditionally. Its `if (config.enableDailyDigest)` gate read the always-empty setup snapshot, so it never fired and the digest could only ever report 0.00. The per-company flag is checked inside instead, cached for 60s so a high-frequency event does not add a config RPC per cost event — which also keeps state out of companies that have the digest switched off. - Gate the digest's success log/metric on a post actually having happened. Tests: four new cases, each verified to FAIL against c85d066 and pass here — a handler doing real work once a token becomes available with no restart, per-company isolation, a secret-resolution failure warning instead of throwing, and setup() completing with jobs registered when secrets.resolve rejects. Full suite 127/127. tsc clean except the pre-existing, unrelated tools.ts(485) BodyInit error (confirmed present without this diff). Refs BLO-20959. Co-Authored-By: Claude <noreply@anthropic.com>

allyblockcast · 2026-08-02T20:15:36Z

@ally re-review at head aabe6b42 — both Important findings were correct and are now fixed. Please focus on:

1. Per-company credential resolution (your finding at worker.ts:835). You were right that registration alone could never make these jobs work. I confirmed the mechanism in the host rather than inferring it: server/src/services/plugin-loader.ts:2553-2555 builds the worker's bootstrap config as a literal {} — "Plugin configuration is company-scoped. Workers receive an empty bootstrap config and must use ctx.config.get(companyId) at runtime" — so setup() can never see a slackTokenRef and pluginToken can never be populated on any install, single- or multi-company. All four handlers now call resolveCompanyJobScope(ctx, companyId, jobKey) per tick, mirroring the per-delivery pattern BLO-20467 landed in paperclip-plugin-alertmanager/src/config-scope.ts. It returns null rather than throwing so one company's bad config can't abort the tick for the rest.

2. Unguarded ctx.secrets.resolve (your finding at worker.ts:861). Correct — and it was reachable exactly as you described. Now wrapped: warn, leave credential state empty, return, and let setup complete with the handlers registered above it.

3. Your side-effects suggestion turned out to be load-bearing in the other direction. The if (config.enableDailyDigest) gate around the cost listener read that same always-empty snapshot, so the listener never registered and the digest could only ever report 0.00. It's now registered unconditionally with the per-company flag checked inside, cached 60s so a high-frequency cost_event.created doesn't add a config RPC per event — which also keeps state out of companies that have the digest off, which was your actual concern.

On the test gap you flagged — you were right that the old test proved nothing about the production path. Four new cases, and I verified each one fails against c85d066f before confirming it passes here:

a registered handler doing real work once a token becomes available on the company's config row, with no restart and no onConfigChanged
per-company isolation (unconfigured company skipped, configured one still served)
a secrets.resolve rejection at job time warning and no-oping instead of throwing
setup() resolving, with all four jobKeys registered, when secrets.resolve rejects

Suite 127/127; tsc clean apart from the pre-existing tools.ts(485) BodyInit error, which I confirmed is present without this diff.

Known gap I am deliberately not fixing here, so it isn't mistaken for an oversight: the interactive surface (tools, webhook handlers, respondToAction) still reads the module-level pluginToken, so it remains inert on multi-company installs for the same root cause. That's a larger change with a different blast radius than the scheduled-jobs outage this PR is scoped to, and I'd rather it be reviewed on its own — filing it as a follow-up.

Note on CI: PR runs on this repo were mass-cancelled 19:13–19:31Z today (BLO-21078, with Platform/SRE). If checks here look cancelled rather than failed, that's the cause, not this diff.

allyblockcast · 2026-08-02T20:16:45Z

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

Missing section: ## Thinking Path
Missing section: ## What Changed
Missing section: ## Verification
Missing section: ## Risks
Missing section: ## Model Used
No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

…LO-20959) The daily-digest handler resolved each company's config AND bot token before testing `enableDailyDigest`, so a company with the digest switched off still cost one secret resolution per day just to be skipped. Secret resolution draws on a shared budget, so check the (cached) flag first and resolve only for companies that will actually post. Refs BLO-20959. Co-Authored-By: Claude <noreply@anthropic.com>

allyblockcast · 2026-08-02T21:23:38Z

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: e4d6765

Prior Findings Dispositioned (2)

prior:c85d066 important 1 — fixed — packages/plugins/paperclip-plugin-slack/src/worker.ts:901 — scheduled handlers now load each company's current config and resolve its token with { companyId }; the runtime-availability and tenant-isolation tests cover this path.
prior:c85d066 important 2 — fixed — packages/plugins/paperclip-plugin-slack/src/worker.ts:1283 — bootstrap secret resolution is caught, warned, and allowed to return after all four handlers have registered; the rejection test exercises this behavior.

Important Issues (1)

[code / tests / gstack] packages/plugins/paperclip-plugin-slack/src/worker.ts:1274 — check-watches remains inert on the production startup path. The always-empty bootstrap config returns here, before the watchable event subscriptions at line 2041 can register, so nothing appends to recent-watch-events for the handler at line 1205 to consume. The new "does real work" test at job-registration.test.ts:110 only proves that the handler reads an empty state key after credential resolution; it never dispatches a watchable event or reaches checkWatches.
- Register the watch-event collectors above the bootstrap credential gate (they do not require a Slack token), and add an empty-bootstrap test that invokes a captured watchable event listener, then proves check-watches consumes that event and performs the configured watch action.

Strengths

Per-company config and secret resolution now isolates missing or broken tenant credentials without suppressing configured companies.
The setup-time secret rejection is correctly degraded after unconditional job registration.
Daily-digest cost accumulation now registers independently of the empty bootstrap snapshot and checks the company-scoped flag at delivery time.

Recommended Action

Move the watch-event producer registration above the bootstrap credential return and cover the producer-to-consumer path.
Re-review the exact resulting head.
This PR is authored by app/allyblockcast; the App cannot review its own PR. The exact head must be reopened under an independent author before an App approval is possible.

kkroo · 2026-08-04T10:00:11Z

Closing as superseded by #996, which carries the same BLO-20959 fix under an independent author and is already approved/in the merge queue.

kkroo mentioned this pull request Aug 3, 2026

fix(slack-plugin): register scheduled jobs before the credential-gated early return (BLO-20959) #996

Merged

1 task

kkroo closed this Aug 4, 2026

allyblockcast Bot mentioned this pull request Aug 6, 2026

fix(slack-plugin): scope interactive surface credentials per delivering company (BLO-21083) #1090

Open

13 tasks

Navigation Menu

Uh oh!

fix(slack-plugin): register scheduled jobs before the credential-gated early return (BLO-20959) - #974

fix(slack-plugin): register scheduled jobs before the credential-gated early return (BLO-20959)#974
allyblockcast[bot] wants to merge 3 commits into
masterfrom
blo-20959-slack-job-registration

allyblockcast Bot commented Aug 2, 2026 •

edited

Loading

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

kkroo commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

allyblockcast Bot commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thinking Path

Linked Issues or Issue Description

What happened

Expected behavior

Repro steps

Paperclip version or commit

Deployment mode

What Changed

Verification

Risks

Model Used

Checklist

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Ally — Consolidated PR Review

Important Issues (2)

Suggestions (1)

Strengths

Recommended Action

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Uh oh!

allyblockcast Bot commented Aug 2, 2026

Ally — Consolidated PR Review

Prior Findings Dispositioned (2)

Important Issues (1)

Strengths

Recommended Action

Uh oh!

kkroo commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

allyblockcast Bot commented Aug 2, 2026 •

edited

Loading