fix(issues): clear monitorNotes when a monitor is cleared (PEN-1995)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Issue monitors are the mechanism an agent uses to schedule its own next look at an external gate, and monitorNotes is the free-text it leaves for whoever reads that wake
• Every clear path nulls monitorNextCheckAt and monitorWakeRequestedAt, but none of them nulls monitorNotes — so once a monitor is retired, its notes stay on the row looking exactly like live instructions
• That is not cosmetic: the notes are what a reader consults to decide what to do next, and a retired monitor's notes assert a gate that nothing is watching
• Observed on PEN-1995: notes registering a discriminator "read X after the capacity reset" outlived their monitor by four days (cleared 2026-08-18T17:50:38.860Z, still on the row 2026-08-22, monitorNextCheckAt: null the whole time). The next reader correctly flagged them as unreadable-but-stale and could not clear them — the documented executionPolicy write path has no way to null the column once the policy is already null
• This pull request makes every monitor-clear path null the notes column, and routes all five clear sites through one helper so a future branch cannot forget one
• The benefit is that issues.monitorNotes means what it says — the notes of the monitor that is currently armed — and a cleared monitor stops leaving instructions behind

Linked Issues or Issue Description

Refs PEN-1995, PEN-1990 (Paperclip-tracked; no GitHub issue).

Duplicate / related PR search (monitorNotes, monitor clear notes, open + closed): no duplicate exists — no other PR nulls the notes column on clear. Related, non-overlapping:

• fix(issues): reject misplaced monitor input keys instead of silently stripping (BLO-18790) #813 (BLO-18790) — rejects misplaced monitor input keys. Adjacent to the "no supported way to clear the column" half of this bug; does not touch the clear paths. Merged.
• fix(monitors): stop re-arming when consecutive re-checks don't narrow the gate set (BLO-18294) #871 (BLO-18294) — added the convergence-stalled clear branch that this PR now routes through the shared helper. Merged.
• fix(recovery): bound how long a lapsed monitor counts as a live wake path (BLO-24782) #1330 (BLO-24782) — bounds how long a lapsed monitor counts as a live wake path. Touches only server/src/__tests__/lapsed-monitor-grace-bound.test.ts and recovery; no overlap with issue-execution-policy.ts. Open.
• feat(monitors): re-read terminated monitor gates board-side (BLO-27515) #1375 (BLO-27515) — board-side re-read of terminated monitor gates. No file overlap. Open.
• fix(heartbeat): stop a launch-stalled run absorbing its agent's wakes (PEN-1995) #1462 (PEN-1995) — the sibling fix from the same investigation (heartbeat coalesce deadlock). Different subsystem (heartbeat.ts), no shared files. Open.

Bug. What happened: after a monitor is cleared, issues.monitorNotes retains the retired monitor's text indefinitely. Because monitorNextCheckAt is null, nothing will ever fire to act on it, but the notes remain the row's most authoritative-looking statement of what to do next — and there is no supported way to clear them. applyMonitorTransition only ever writes the column when a monitor is armed (patch.monitorNotes = input.policy.monitor.notes ?? null); all four clear branches, plus buildIssueMonitorClearedPatch, omit it. Sending {"executionPolicy": {}} normalizes to null, which is a no-op when the policy is already null, so the stale text survives every documented clear.

Expected: clearing a monitor clears the notes that described it.

Workaround this replaces: arm a throwaway monitor with corrected notes purely to overwrite the column, then clear it again — two writes and a transient armed monitor to fix a text field.

What Changed

• server/src/services/issue-execution-policy.ts: added clearArmedMonitorColumns(patch), which nulls monitorNextCheckAt, monitorWakeRequestedAt, and monitorNotes together.
• Routed all four clear branches in applyMonitorTransition through it: invalid-for-issue-state, bounds-exhausted, convergence-stalled (BLO-18294), and monitor-removed-from-policy.
• buildIssueMonitorClearedPatch now returns monitorNotes: null for the same reason.
• Three unit tests in server/src/__tests__/issue-execution-policy.test.ts: notes nulled on the done/auto-clear path (assertion added to the existing test), on the manual monitor-removed path, and on the invalid-issue-state path. The manual-path test also asserts the audit copy is still on executionState.monitor.notes.

No schema change, no API-shape change: monitorNotes was already nullable and already returned on reads.

Verification

cd server && pnpm vitest run src/__tests__/issue-execution-policy.test.ts

Fail-first was checked, not assumed: with the source change reverted and the tests kept, the three new expect(result.patch.monitorNotes).toBeNull() assertions fail (the key is absent from the patch, so it reads undefined). With the change applied, the file passes.

Lossless-ness is the load-bearing claim, so it is asserted rather than argued: buildClearedMonitorState already carries notes: input.previous?.notes ?? null onto the cleared executionState.monitor, and derivePersistedMonitorState falls back to fromState?.notes. The manual-path test asserts both halves — column null, state copy intact — so a future change that drops the audit copy fails here.

Risks

Low. Behavior change is confined to the value of one nullable text column at monitor-clear time.

• A reader that used issues.monitorNotes to recover a retired monitor's text will now get null and must read executionState.monitor.notes. I grepped the writers (heartbeat.ts, issues.ts, issue-execution-policy.ts, execution-workspaces.ts, routes/issues.ts); the derivation path already prefers the state copy, and no consumer reads the column expecting post-clear content.
• Deliberately not changed: buildIssueMonitorTriggeredPatch keeps monitorNotes populated. A triggered monitor has nextCheckAt: null but is pending a wake that is supposed to read those notes — nulling there would break the mechanism this PR exists to protect.
• Not addressed here: rows that already carry stale notes from before this change. Clearing is idempotent, so any future clear fixes them; a backfill was not in scope and no migration is included.

Model Used

Anthropic Claude, model id claude-opus-5[1m] as reported by the harness (Opus tier, 1M-token context), agentic tool use with repository read/write and test execution. Change authored and verified by the model; the diagnosis originated from measured production heartbeat_runs data on PEN-1995.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work — ROADMAP.md has no monitor/wake-scheduling entry
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template — (b), bug_report shape above
• I have run tests locally and they pass — 59 passed in issue-execution-policy.test.ts, 45 across adjacent monitor suites, tsc --noEmit clean
• I have added or updated tests where applicable — 3 new assertions, verified fail-first against unfixed source
• If this change affects the UI, I have included before/after screenshots — N/A, server-side only
• I have updated relevant documentation to reflect my changes — no documented behavior or command changed; rationale is in the helper's doc comment
• I have considered and documented any risks above
• All Paperclip CI gates are green — this review gate was the only red; re-run after adding this checklist
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

Repo-specific gates (retained from the prior checklist)

• Behavior matches doc/SPEC-implementation.md (no spec'd behavior changed; this restores the stated meaning of an existing field)
• Contracts synced across db/shared/server/ui (no contract change — column already nullable and already exposed)

[allyblockcast]

🔗 Paperclip issue: BLO-18294
🔗 Paperclip issue: PEN-1990
🔗 Paperclip issue: PEN-1995

[allyblockcast]

🔗 Paperclip issue: BLO-18294
🔗 Paperclip issue: PEN-1990
🔗 Paperclip issue: PEN-1995

[allyblockcast]

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

Missing or incomplete:

• 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]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Centralizes the three persisted monitor-clear columns so the policy transition branches cannot clear scheduling state while leaving stale notes behind.
• Preserves the cleared monitor's notes in executionState.monitor.notes, and the tests cover both the database-column cleanup and audit-state retention.
• Keeps triggered-monitor notes intact, which avoids conflating a triggered wake with a fully cleared monitor.

Recommended Action

1. No Critical or Important issues found. This formal review is COMMENTED because the PR is authored by the Ally App.