test(plugin-linear): requestId fixtures + getLinkByLinear mock-leak fix; scripts: ensure-build-deps freshness check by kkroo · Pull Request #1 · Blockcast/paperclip

kkroo · 2026-05-14T03:32:04Z

Summary

Re-base of two commits from PR #148 (lost in v513 rebase cleanup) onto current master. The back-link feature commit from #148 is dropped — master already implements it via writePaperclipBackLink helper.

Commit 1 — `test(plugin-linear)`: requestId fixtures + mock-leak fix

PluginWebhookInput.requestId: string (SDK define-plugin.ts:122) is required, so all 10 onWebhook test fixtures need it; without it, pnpm typecheck fails in paperclip-plugin-linear.

Separately, the BLO-2973 idempotency test uses mockResolvedValue on syncModule.getLinkByLinear, which vi.clearAllMocks() in beforeEach does not reset (only call history is cleared, not implementations). The mocked link leaked into the BLO-2350 projectId-resolution tests, where the webhook handler at worker.ts:1291 short-circuits when getLinkByLinear returns a link and never reaches ctx.issues.create — so the createSpy assertion failed with "expected to be called once, but got 0 times".

The BLO-2350 tests have been latent-broken since 2026-04-30 (commit 16676113); prior type errors caused vitest to skip them, so the leak was invisible.

Commit 2 — `fix(scripts)`: `ensure-plugin-build-deps` freshness check

The script's allOutputsExist() guard only checked whether dist/index.js exists, never whether it is up-to-date relative to src/. A stale @paperclipai/plugin-sdk dist silently fed downstream plugin typechecks the wrong .d.ts shapes, surfacing as "Property X does not exist on type Y" errors for symbols that were in fact present in src/ but absent from the cached dist.

Added an mtime-based freshness check: output mtime must be at least as recent as the newest .ts/.tsx in the package's src/ and the tsconfig.json itself.

Test plan

pnpm typecheck in packages/plugins/paperclip-plugin-linear exits 0
pnpm test (vitest) in paperclip-plugin-linear shows 60/60 passing (or whatever the post-v513 count is)
Touch a .ts file in packages/plugins/sdk/src/ then run pnpm typecheck in any consumer plugin → confirm SDK rebuild fires before downstream tsc --noEmit
Fresh dist (no touch) → typecheck runs in <100ms overhead (pre-tsc), confirming no regression

🤖 Generated with Claude Code

…ByLinear leak PluginWebhookInput now requires requestId (SDK define-plugin.ts:122), so nine onWebhook test fixtures fail to type-check. Add a literal value; neither the plugin nor the tests read it. Separately, the BLO-2973 comment-idempotency test was using mockResolvedValue on syncModule.getLinkByLinear, which vi.clearAllMocks() in beforeEach does not clear (only call history is cleared, not implementations). The mocked link leaked into the BLO-2350 projectId-resolution tests, where the webhook handler at worker.ts:1291 short-circuits when getLinkByLinear returns a link and never reaches ctx.issues.create — so the createSpy assertion failed with "expected to be called once, but got 0 times". Restore the default at the end of BLO-2973. These three BLO-2350 tests have been latent-broken since 2026-04-30 (commit 1667611); the prior type errors caused vitest to skip them, so the leak was invisible. After this fix: vitest 60/60 passing, tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The script's allOutputsExist() guard only checked whether dist/index.js exists, never whether it is up-to-date relative to src/. That let a 5-day stale @paperclipai/plugin-sdk dist silently feed downstream plugin typechecks (plugin-linear, ccrotate, etc.) wrong .d.ts shapes, surfacing as "Property X does not exist on type Y" errors for symbols that were in fact present in src/ but absent from the cached dist. Add an mtime-based freshness check: compare the output's mtime against the newest .ts/.tsx in the package's src/ and the tsconfig itself. Treat the output as fresh only when it is at least as recent as both. Verified locally: fresh dist → ~70 ms no-op (same as before); deleted dist or touched src → ~8 s SDK rebuild (matches the manual `pnpm build` cost). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ock reset Addresses code-review feedback on the rebased PR. scripts/ensure-plugin-build-deps.mjs - isFresh() now warns + forces rebuild when the expected src/ dir is missing, instead of silently letting `newestMtimeInDir` return 0 (which made stale dist pass freshness — the exact bug class the script exists to prevent). - newestMtimeInDir() now follows symlinks (via statSync) so workspace setups that symlink shared sources are scanned. Per-entry readdir/stat failures (concurrent delete, EACCES, dangling symlinks) log the path and skip the entry rather than crashing the entire pre-build. tests/plugin.spec.ts - Extracted restoreSyncModuleDefaults() and called it after vi.clearAllMocks() in beforeEach. Replaces the per-test getLinkByLinear cleanup at end of BLO-2973 with a systemic reset that prevents any future test's mockResolvedValue override from leaking, regardless of test ordering or which sync export is overridden. - BLO-2973 duplicate-delivery test and the issue.create duplicate-prevention test now use distinct requestId values between the two onWebhook calls. The dedup path under test (comment-id sentinel; linearIssueId) is what the assertions exercise; if the plugin ever adds requestId-based idempotency, identical requestIds would mask a regression in the actually-tested mechanism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The UI workspace's vitest config inherited the same 5s test / 10s hook defaults that bit server tests pre-PR #56. CompanyAccess.test.tsx test #1 ("keeps the page human-focused...") timed out at 5545ms on verify_canary run 26011982864 — barely over the 5s cap, classic cold-import-on-first-test flake. Mirrors the server-side bump in PR #56: testTimeout: 30_000 hookTimeout: 60_000 teardownTimeout: 30_000 Doesn't address CompanyAccess test #4 ("shows protected member removal reasons from the API"), which renders empty container.textContent — that's a genuine logic failure (component crashed silently when mock returns a member with `removal` field), not a timeout. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…minated union Addresses review on PR #227 (BLO-8188). - Zero-rows bypass (review #1): the fail-loud guard previously lived only inside `if (projectWorkspaceRows.length > 0)`, so a non-primary target that resolved to zero workspace rows (deleted, or target belongs to a different project) fell through to the managed-default branch and ran silently on the wrong source. The primary/non-primary decision is now computed once from the unordered rows and applied in BOTH the populated and zero-rows branches. - Type enforcement (review #3): `ResolvedWorkspaceForRun` is now a discriminated union. The failure variant carries no `cwd`/`source`, so "must not execute on the fallback cwd" is enforced by the compiler rather than a doc comment; the caller narrows on `realizationFailure` before reading `cwd`. - Primary determination (review edge a/b): non-primary is decided from the `is_primary` flag (legacy projects fall back to earliest-created) via the new pure helpers `resolveProjectPrimaryWorkspaceId` + `isNonPrimaryWorkspaceTarget`. A project with multiple `isPrimary` rows no longer false-fails a legitimately primary target; legacy earliest-created behavior is pinned by tests. - Tests (review #2): added a caller-level embedded-Postgres integration test asserting the run fails with `preferred_workspace_unrealizable`, the adapter never executes, and no execution-workspace row is persisted. Expanded pure unit coverage for the two new helpers (multi-primary, legacy non-row[0], zero-rows/ghost). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ption-A lock (BLO-9117) (#309) * review: coverage forward-only honesty signal + window-clip note + option-A lock + drop dead code (BLO-9117) Addresses the design-author review on PR #308: - #1 (coverage vacuous ~100% under forward-only capture): CoverageReport now carries reconciledTailObserved + forwardOnly. The forward webhook only stores ref-linked rows, so a window with no reconciler tail is flagged forwardOnly so a consumer can't mistake a vacuous 100% for measured coverage. (Reconciler repo-discovery/scheduling remains the tracked follow-up.) - #2: doc note that rollup cost is intentionally NOT window-clipped (full issue cost vs window-bounded LOC). - #3: extracted applyIssueIdentifierToBranchName + a unit test asserting the enforced branch is extractor-matchable (locks option A against a future lowercasing sanitizeBranchName). - #4: dropped the unused resolvePrLinks/ResolvedPrLink dead code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: trigger PR checks (bot-opened PR does not auto-run) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kkroo <kkroo@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…(BLO-10777) (#452) * fix(blocked-inbox): classify in_review blocker with valid monitor as covered (BLO-8072) classifyPath treated every in_review node with no live run and no user assignee as stalled_review, even when the issue was sitting in a legitimate monitor wait. Add hasValidBlockerMonitor helper that mirrors hasScheduledMonitor from issue-graph-liveness.ts: returns true when monitorNextCheckAt is in the future, the monitor hasn't timed out, and attemptCount < maxAttempts. Extend IssueBlockerAttentionNode type and both SELECT queries in listIssueBlockerAttentionMap to project monitorNextCheckAt, monitorAttemptCount, and executionPolicy. Use hasValidBlockerMonitor in the in_review branch of classifyPath so a valid scheduled monitor yields covered instead of stalled. Add three unit tests: valid monitor → covered, past nextCheckAt → stalled, exhausted maxAttempts → stalled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(heartbeat): add crashloop circuit-breaker for consecutive adapter_failed runs Adds `adapterFailedAutoPauseAfter` to the heartbeat policy. When N consecutive `adapter_failed` runs occur (tracked in `agentRuntimeState.stateJson` as `consecutiveAdapterFailedRuns`), the timer scheduler skips the next wakeup — same pattern as the existing `idleAutoPauseAfter` idle circuit-breaker. The counter is updated atomically via nested `jsonb_set` inside the existing idle counter update, so both keys land in the same `db.update()` call. Also adds a regression fixture for `parseOpenCodeJsonl` documenting that events with a missing, null, or empty `type` field are silently skipped (BLO-10651: opencode binary crashes on OpenAI gpt-5.5 Responses-API items with no type). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(roadmap): milestone primitive + issue target-date + linear sync (BLO-10777) Fix #1 — Milestone primitive - Migration 0115: creates milestones table (company_id, project_id FK, name, target_date, sort_order) and adds milestone_id + target_date columns to issues - Drizzle schema: packages/db/src/schema/milestones.ts - Shared types: Milestone, CreateMilestoneInput, UpdateMilestoneInput - Zod validators: createMilestoneSchema, updateMilestoneSchema - Server service: createMilestonesService (list, getById, create, update, remove) - REST routes: GET/POST /companies/:companyId/milestones, GET/PATCH/DELETE /milestones/:id - Issue list select and GET /issues/:id response include milestoneId + targetDate Fix #2 — Issue target-date → Linear dueDate - Linear worker issue.updated handler maps targetDate → dueDate when no explicit dueDate change is present - SyncChanges interface extended with milestoneId stub (full milestone↔Linear milestone map is a follow-on) Fix #3 (managedByPlugin for Linear project bindings) is out-of-scope for this PR — tracked as follow-on. * feat(projects): surface Linear project binding in GET /projects/:id (BLO-10777) Add LinearProjectLink type and query plugin_state for the linear sync plugin's project-link:{id} keys in attachWorkspaces so that both GET /projects/:id and the projects list return a populated linearProjectLink field for any project bound to a Linear project. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sdk): add milestoneId + targetDate to Issue test fixture (BLO-10777) The testing.ts issue create fixture was missing the two new Issue fields added by the milestone/target-date migration, breaking the SDK build and all CI test suites that depend on it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): wire milestone routes into openapi spec + backfill Issue test fixtures (BLO-10777) The milestone feature added two required Issue fields (milestoneId, targetDate) and a new mounted route file, but several cross-package consumers weren't updated, turning CI red: - UI: 21 test/story fixtures construct full `Issue` literals and were missing the two new required fields → ui build (tsc) failed (Build + Canary Dry Run). Added `milestoneId: null` / `targetDate: null` to each Issue fixture (anchored on the Issue-only `hiddenAt` field). - plugin-llm-wiki: its `paperclipIssue` fixture was missing the two fields → typecheck:build-gaps failed. - openapi-routes.test.ts asserts every mounted route is documented: the new `milestones.ts` routes were mounted (app.ts) but absent from the OpenAPI spec and the test's prefix map. Registered the 5 milestone paths in routes/openapi.ts and added the `milestones.ts` prefix entry. No source/behavior change to the feature itself — these are the consumer-side updates the type/route additions require. server + ui + plugin-llm-wiki tsc clean; openapi-routes 3/3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: re-trigger commitperclip review gate (PR body restructured to template) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): wire milestones to OpenAPI spec + fix test fixture gaps (BLO-10777) - Register milestones routes in openapi.ts + add milestones.ts to test apiPrefixes - Make issues_company_milestone_idx a partial index (WHERE milestone_id IS NOT NULL) to prevent query-planner interference with evidence-verdict index tests - Update 0115_milestones.sql migration to match partial index - Add milestoneId/targetDate to all remaining UI/plugin test Issue fixtures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): remove duplicate milestoneId/targetDate fields post-rebase * chore: re-trigger commitperclip (inline issue description -> bug_report template shape) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): de-duplicate milestoneId/targetDate in optimistic-issue-comments fixtures (BLO-10777) Concurrent fixture edits (the openapi/fixture pass + the post-rebase dedup) both inserted milestoneId/targetDate into the 4 Issue literals here, leaving duplicate properties in 3 of them → ui build TS1117 (object literal cannot have multiple properties with the same name). Drop the redundant copies; each fixture keeps one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kkroo <kkroo@paperclip.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Omar Ramadan <omar@blockcast.net>

…2990 Fix #1) A running run that has been silent for > EXTERNAL_LIFECYCLE_STALE_MS (15 min) was being counted as consuming a slot by countRunningRunsForAgent, starving all higher-priority queued work indefinitely when the only active runs were stale. Root cause: startNextQueuedRunForAgent used a raw count of all status='running' rows for both (a) the external-lifecycle hard gate (if runningCount > 0 return []) and (b) the availableSlots = maxConcurrentRuns - runningCount calculation. A k8s Job that is technically still running but has gone silent for hours continued to consume a slot and block all queued dispatch — confirmed fleet-wide on both MulticastEngineer and Staff Engineer (BLO-12825 / BLO-12738). Fix: replace countRunningRunsForAgent with listRunningRunsForAgent (fetches full rows with signal timestamps) then partition into nonStaleRunningRuns. A run is stale when lastUsefulActionAt > lastOutputAt > startedAt is older than EXTERNAL_LIFECYCLE_STALE_MS. Only non-stale runs count toward runningCount and inFlightIssueIds — the same silence metric the reaper already uses. Bonus: consolidates two DB round-trips into one (the old code queried running count then re-queried contextSnapshot for inFlightIssueIds; now one fetch serves both purposes). Regression test: maxConcurrentRuns:2 agent with 2 stale "running" runs (slots full under old code) + 1 queued high-priority run. Fix #1 drops runningCount to 0, availableSlots to 2, and the todo run dispatches. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

… path The owner!=assignee regression test added for Ally's Important #1 was failing on the branch head, and was not testing what it claimed. With a second agent in the company, the stranded escalation hands BOTH the recovery action and the issue to that manager, so the post-escalation row has owner == assignee == manager. The test then called issueService.update() as the original agent, who was no longer the assignee, so the guard skipped via its assignee predicate rather than exercising the owner-scoped join at all -- and the `assigneeAgentId: agentId` assertion failed outright. Re-pin the assignee to the original agent after escalation so the row actually models the assignee-fallback wake branch (woken agent is the assignee, action owned by someone else), and assert the owner really differs. Verified red against the owner-scoped join and green with the issue-scoped one. Also addresses two review suggestions: - Gate the `for update` row lock and recovery-action join on the cheap "patch is not taking a run lock" condition first. A patch that takes a run lock can never trip the guard, so the normal checkout path now skips the extra lock round trip and join entirely. Behaviour-preserving. - Document `reassertSourceScopedRecoveryBlockedAfterWake` as a backstop rather than the primary guard, and give it `.returning()` + a warn log so a non-zero match is visible instead of healing silently. Co-Authored-By: Paperclip <noreply@paperclip.ing>

…gate verify (BLO-20733) Both Ally Important findings on PR #973 @ 1dd3154. 1. deployment-api.yaml copied all of pod.annotations into the map it then stamps, so the release-controlled marker key could arrive from chart values. Two silent failure modes, neither covered by the existing tests (they use an ordinary `example.com/team` key): - api.approvalPlanSha256 UNSET: the value passes straight through, so the render the release job treats as "unstamped" already carries a marker. The hash taken from render #1 is then computed over a document containing a marker and can never match what the approve script recomputes from render #2 -- every release dies at "planned Deployment pod template must carry ...". - api.approvalPlanSha256 SET: `set` silently overwrote the caller's value, hiding a conflict rather than reporting it. The key is release-controlled, so reject it outright instead of picking a winner. That is the only behaviour that keeps render #1 genuinely unstamped. 2. helm_chart ran but did not gate. `verify` is the required context and neither listed helm_chart in `needs` nor asserted its result, so a red Helm lane could sit beside a green required check. Both halves matter and fail independently: without the `needs` entry `needs.helm_chart.result` renders empty and the lane silently never gates; without the map entry the result is collected and ignored. Verified locally (helm + kubectl present): - chart marker suite 7/7, full chart suite 28/28, verify lane suite 11/11 - mutation-proven three ways, each restoring to green: remove the hasKey guard -> 2 chart tests red drop helm_chart from verify.needs -> 1 lane test red drop the lane_results entry -> 2 lane tests red Rebased onto master, which had since rewritten the verify step for cancelled-vs-failed lanes (BLO-20867 #964); the new entry follows that shape. Co-Authored-By: Claude <noreply@anthropic.com>

…sking faults as evidence-gate rejections (BLO-18829) Carries the recovery-side work out of PR #811 (BLO-18760), which is now scoped to the project-inference fix alone, plus fixes for two of the three Important findings from Ally's 5th review on 9fd8f89. Finding #2 (native-codex, service.ts:4418) -- the CAS accepted both `input.previousStatus` and "blocked", so an `in_progress` reread still matched an issue a human moved to `blocked` afterwards, and recovery overwrote that human's blocker set and assignee with its own. That is precisely the stale write the CAS exists to reject. Pinned to `[fresh.status]`: the single status observed under the advisory lock. Steady-state retries (reread already found `blocked`) keep working; a transition into `blocked` landing after the reread is now rejected. Finding #3 (pr-review-toolkit/errors, service.ts:4030) -- the blanket catch around the `in_review` park treated every exception as an evidence-gate rejection and escalated the issue to `blocked` on that basis, so a programming error or DB fault was relabelled as the business state "nothing to review". It also could not deliver the sweep continuation it claimed: a failed statement leaves the surrounding transaction aborted, so db.transaction's COMMIT throws anyway. Both park paths now absorb exactly `unprocessable("missing-evidence", { code: "missing-evidence" })` via isEvidenceGateRejection and rethrow everything else. Also moves the `expectedStatus` option on issuesSvc.update here from #811, since this is now its only consumer. Not yet addressed, deliberately: finding #1 (atomicity -- one transaction across status claim, recovery action, quota monitor, and a durable wake outbox dispatched post-commit). That is this issue's headline scope and needs tx threading through ensureSourceScopedStrandedRecoveryAction / enqueueSourceScopedStrandedRecoveryWake. The per-issue sweep boundary in reconcileStrandedAssignedIssues lands with it -- that loop body is 840 lines, so wrapping it re-indents ~1700 lines and would bury this diff. server/src tsc --noEmit: 0 errors. Co-Authored-By: Paperclip <noreply@paperclip.ing>

kkroo and others added 3 commits May 14, 2026 03:31

kkroo merged commit ce3f854 into master May 14, 2026
4 of 8 checks passed

kkroo deleted the omar/blo-3691-paperclip-backlink-v2 branch May 14, 2026 04:18

kkroo mentioned this pull request May 14, 2026

v513 test-fallout cleanup batch 2: codex-local SSH dispatch + company-portability mock/expectations #3

Merged

6 tasks

This was referenced May 19, 2026

fix(heartbeat): skip next-queued dispatch when executeRun finalizes as cancelled #80

Merged

Revert "test(stale-queue): port watchdog 3-confirm idle pattern to close 40P01 deadlock (#92)" #94

Merged

kkroo mentioned this pull request May 29, 2026

chore(test): fix pre-existing TS diagnostics in 3 environment-* test files #223

Merged

3 tasks

blockcast-ci-packages Bot mentioned this pull request May 30, 2026

fix(heartbeat): fail loud when preferred non-primary workspace can't be realized (BLO-8188) #227

Merged

8 tasks

kkroo mentioned this pull request Jun 5, 2026

feat(heartbeat): escalate ccrotate capacity exhaustion as a coalesced issue (PEN-382) #307

Merged

This was referenced Jun 5, 2026

feat(efficiency): run→PR linkage + authored-LOC + apportioned $/output rollup (BLO-9117) #308

Merged

review-followup(efficiency): coverage forward-only honesty signal + option-A lock (BLO-9117) #309

Merged

This was referenced Jun 13, 2026

build(deps): bump mcp-gateway vitest to ^3.2.6 (CVE-2026-47429) #387

Merged

fix(heartbeat): GitHub-verify PR-review evidence before flagging pr_review_output_missing (BLO-10448) #427

Merged

allyblockcast Bot mentioned this pull request Jun 18, 2026

feat(roadmap): milestone primitive + issue target-date + linear sync (BLO-10777) #452

Merged

1 task

allyblockcast Bot mentioned this pull request Jun 19, 2026

[codex] Deduplicate webhook-driven @ally review wakes #453

Merged

18 tasks

allyblockcast Bot mentioned this pull request Aug 5, 2026

docs(skills-catalog): name the cluster SSH key in the credential model (BLO-21854) #1053

Open

13 tasks

Navigation Menu

Uh oh!

test(plugin-linear): requestId fixtures + getLinkByLinear mock-leak fix; scripts: ensure-build-deps freshness check - #1

test(plugin-linear): requestId fixtures + getLinkByLinear mock-leak fix; scripts: ensure-build-deps freshness check#1
kkroo merged 3 commits into
masterfrom
omar/blo-3691-paperclip-backlink-v2

kkroo commented May 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kkroo commented May 14, 2026

Summary

Commit 1 — test(plugin-linear): requestId fixtures + mock-leak fix

Commit 2 — fix(scripts): ensure-plugin-build-deps freshness check

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Commit 1 — `test(plugin-linear)`: requestId fixtures + mock-leak fix

Commit 2 — `fix(scripts)`: `ensure-plugin-build-deps` freshness check