feat(#12284): reminderIntensity-driven no-reply/escalation policy + quiet-watcher softening (WI-6, WI-8) - #13237
Conversation
…eminderIntensity lookup (WI-6+WI-8) A >=3-day ignored-checkin streak, derived from the recent-task-states log, steps the effective reminderIntensity one notch down before the existing intensity->no-reply-policy lookup shapes the follow-up ladder — one structural mechanism, two signals. The scheduled-task tick now feeds that log for real (fires, completions, no-reply terminals), the applied intensity + softening decision are persisted on the task's noReplyState so the ladder choice is auditable, and approvals are exempt (they gate agent-side actions). Owner replies through both completion seams append the streak-breaking entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Independent review vs D3-intensity/quiet design: MERGE-WITH-NITS — land third, re-sync first (branch behind develop). Closes the real D3 gap: silent-streak → gentler escalation via the #12952 intensity table, and adds the missing production writer for recent-task-states ( Nits:
|
|
Reviewed (adversarial workflow): reminderIntensity-driven no-reply/escalation policy (#12284); 431-line real-runtime (no-mock) test dominates the diff. Verified against the actual diff; no failing CI. Queuing auto-merge on green. |
lalalune
left a comment
There was a problem hiding this comment.
Cross-review (W1-C, #12284 WI-6+WI-8) — APPROVE
Checked out head 56f48cf (merge-base 76c9253) in an independent review lane and re-ran the claims rather than trusting them. (PR was merged by the owner mid-review; the notes below are follow-up material, none would have blocked.)
Verified
- Targeted suites reproduced green:
scheduler.quiet-streak.test.ts6 +no-reply-intensity.test.ts8 +scheduler.no-reply-policy.test.ts9 → 23/23 in one vitest run;inbound-reply-completion.integration.test.ts5/5;test/recent-task-states.integration.test.ts2/2; default-pack spine-seeding + smoke 9/9;plugin-scheduling241/241 across 19 files (package untouched by the branch; +3 tests vs the PR body's 238 from develop drift, all green); PAtypecheckexit 0. - Pre-existing-failure claim spot-checked:
reminders-service.process-scheduled-work.test.tsfails 5 with exactly the documentedtravel_reconcile/cache.getCache is not a functionsignature — and that suitevi.mocksprocessDueScheduledTasksentirely, so this PR's new cache read inside the real tick cannot be the cause. None of the 4 claimed-failing files appears in the 10-file diff. - Structural-not-prompt discipline (AGENTS.md:156-171): branching is on
task.kind, the owner-fact enum, and log-derived terminal streaks — nopromptInstructionstext branching, no persona-name branches, no second scheduler; frozenScheduledTaskcontract intact (onlymetadata.noReplyState/noReplyPolicybag fields + an optional providerasOfoption).console.*absent from the diff. - Done-when met: different
reminderIntensityfacts → measurably different retry/ladder behavior (kept-green #12952 cases + new auditability case assertingappliedReminderIntensitypersisted on retry); 3-day-silent persona → next reminder structurally softened (fire-once,maxRetries: 0), asserted from the persisted policy object and driven through REAL ticks that exercise the log's new production writer on both fire and terminal paths; a reply through the realMESSAGE_RECEIVEDseam appends the streak-breaking entry. - Adversarial probes (all held): at the day-3 timeout tick the streak is still 2 (terminal trail excludes that tick's own not-yet-appended expiry), so the day-3 checkin itself is correctly unsoftened;
readNoReplyStatewhitelists fields, so stale softening flags cannot leak into the terminal record via the...statespread; the twoMESSAGE_RECEIVEDcompletion seams run sequentially and the second only lists still-firedtasks, so no duplicatecompletedappend in the normal path; a cache outage insideresolveQuietStreakDaysis contained by therunSubsystem("scheduled_tasks", …)guard in reminders-service.ts (subsystem-pass fallback + logged failure, loop survives) — same failure envelope as the pre-existing unguarded owner-facts read two lines above it; the log-append catch is genuinely J7-shaped (reportError, never undoes a committed transition). - Deviation claim accurate:
ReminderIntensityon develop isminimal | normal | persistent | high_priority_only(fact-store.ts:96-100) — noescalatedmember exists, so the issue-textescalatedhandling is genuinely N/A. PA-side seam is explicitly allowed by the WI ("PA-side … and/or a spine-side hook");plugin-schedulinguntouched as stated.
Notes (low severity — follow-up hardening, none merge-blocking)
scheduler.ts:295—quietStreakSoftened: truepersisted even when the soften is a fixed point. For an owner atminimalorhigh_priority_only, a quiet streak stampsquietStreakSoftened: truealthough the resolved ladder is identical to the unsoftened one, while the field's doc says "True when a quiet streak stepped the effective intensity down". Audit metadata over-claims for those owners. Fix: flag only when the notch actually changed the effective intensity, or re-document the field as "quiet streak active during resolution".scheduler.ts:469— degraded telemetry degrades too much.resolveQuietStreakDaysis unguarded, so a cache failure aborts the wholescheduled_taskssubsystem pass for the tick (no reminders/approvals processed until cache recovers) instead of just skipping softening — while the WRITE side of the same log is deliberately J7-guarded. Contained byrunSubsystemand matching the pre-existing owner-facts read's envelope, hence low; still, a try/catch →undefined+reportErrorwould make the failure mode "no softening" rather than "no pass".providers/recent-task-states.ts:147— every log entry counts as a "fire". Now that the production writer appends both afiredand a terminal entry per lifecycle, the provider's "(over N fires)" summary text double-counts (the quiet-streak test's own 5-entry log reads as "over 5 fires" for 3 check-ins). Streak derivation is terminal-only and unaffected; this is planner/brief-facing text accuracy only. Fix: count onlyoutcome === "fired".providers/recent-task-states.ts:98-110— read-modify-write append race.appendScheduledTaskLogEntrydoesgetCache→ push →setCachewith awaits in between; aMESSAGE_RECEIVEDcompletion append racing a tick append can drop an entry (worst case the streak-breakingcompleted, leaving softening active one extra evaluation). Tiny single-process window and self-healing on the next reply, but worth compare-and-append or per-runtime serialization as the log gains writers.- Cosmetic: the evidence file header says "based on develop @ b24dec8" — stale after the rebase to 76c9253 (the PR body is correct).
Not re-run here: the full PA suite and full bun run verify (the PR documents byte-identical reproductions of the drift set on clean develop; the touched package is 100% green above, and the failing packages/files are untouched by this diff — spot-checked one of the four failing files as described).
Verdict: approve.
…gagement) (#13335) * test(#12775): author SR slice of persona pack E1 (low-activation-reengagement) Completes the scenario-runner slice of LifeOps persona pack E1 — low-activation-reengagement (persona tara_low / P6, #12775), mirroring the A1 (#13286) and B1 (#13312) pack conventions. Coverage gate: E1 2/28 -> 28/28 authored (22 verified). Authored 10 new SR scenarios on top of the 2 pre-existing E1 scenarios: pr-deterministic (keyless, real lifeops_scheduler tick, no LLM): - lowact-quiet-streak-softens-next-nudge — three check-ins are fired + terminally expired through REAL ticks so the production state-log lays down a checkin/expired streak; the next reminder times out and the quiet-streak softener (#12779/#13237) steps intensity normal->minimal, read back off the persisted task (noReplyState.quietStreakSoftened=true, quietStreakDays>=3, appliedReminderIntensity=minimal, emptied ladder). - lowact-morning-single-priority-fires-once — one gentle high-priority morning pick fires once inside quiet hours while a low-value whole-list ping is HELD (quiet_hours gate); single-delivery finalCheck. - lowact-values-anchored-activity-fires-in-window — a during_window values-anchored evening activity fires inside the seeded eveningWindow and defers outside it. - lowact-micro-step-deferred-not-dropped — a captured one-small-step is parked (snooze override), does not resurface early, and resurfaces exactly once (scheduled_override_due) at the promised gentle time. Added to EXPECTED_PR_DETERMINISTIC_SCENARIO_IDS in the same commit (G1). live-only (status: authored, live-verify deferred to #12781): - lowact-lapse-return-triage-no-guilt (definitionCountDelta + shame-free judge) - lowact-quiet-user-reengagement-tone - lowact-crisis-language-safe-reengagement - lowact-make-whole-list-smaller-bulk-shrink - lowact-celebration-without-infantilizing - lowact-cannot-choose-single-option (definitionCountDelta{delta:1}) Each has effect-reading finalChecks, non-echo-satisfiable, personas-as-data in turns[].text (never promptInstructions). Crisis-boundary handling: #12780 crisis guard is CLOSED/NOT_PLANNED, so no scenario asserts a crisis-guard/988 effect. lowact-crisis-language-safe- reengagement takes the #12280 crisis-adjacent premise and asserts only the SAFE behavior per the A1 adhd-task-initiation convention: warm non-clinical stand-down, NO productivity push (definitionCountDelta{delta:0} proves no task/schedule created against her "not right now"), and does NOT assert a 988/crisis-guard side-effect fires. Catalog: 10 new surface:scenario-runner rows + 16 registered surface:lifeops-bench Python ids (append-only; no lifeops-bench/** file touched). 12 SR + 16 bench = 28 = target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: repair wave-2 fallback-sweep regressions (test mocks + typecheck) (#13336) Two of the five wave-2 fallback-slop-sweep PRs left real reds on develop: - #13278: database-rows-compat-routes.test.ts failed 5 OWNER-gate tests in the full app-core suite. app-core runs vitest with isolate:false, so a preceding suite's cached ./auth/sessions mock leaked into the real ensureRouteMinRole. Fix: vi.resetModules() in the hoisted block (the same immunization ensure-route-min-role.test.ts already uses). - #13277: useWhatsAppPairing.test.tsx had a TS2556 — the zero-arg onWsEvent vi.fn spread unknown[]. Fix: type the mock with a rest-param signature. Findings #13271, #13270, #13287 were already green on current develop and are left unchanged. No fail-fast/reportError conversion reverted. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci: exhaustive-lane matrix proof + vacuous-green guard, GitHub-native turbo cache foundation, Linux Bun cache (advances #12342/#12341/#12338) (#13332) * ci(#12342): add exhaustive-lane matrix proof + vacuous-green guard Additive proof infrastructure for the exhaustive develop lane. Nothing here touches a PR-critical workflow or renames a required status check. - packages/scripts/ci-lane-manifest.json: committed source of truth enumerating every expected exhaustive lane plus plan floors. - packages/scripts/ci-full-matrix-proof.mjs: cross-checks the manifest against test.yml (job present, not pinned pull_request-only) and against `run-all-tests.mjs --plan=json` (task/package floors, required core packages, non-empty per-script lanes). Fails on a missing lane, a lane pointed at a nonexistent glob, or a whole script lane collapsing to zero. Emits a GitHub step summary enumerating every lane. - run-all-tests.mjs: `--min-tasks`/`MIN_TEST_TASKS` vacuous-green guard. A filter/shard/glob that collapses a lane to (near-)zero tasks, or a run whose every task skips (no test files), now exits 3 instead of green. Strictly additive: default 0 preserves historical behaviour. - .github/workflows/ci-full-matrix-proof.yml: un-cancellable (cancel-in-progress: false) scheduled proof job, twice daily + workflow_dispatch, plus a path-scoped PR trigger on its own inputs only. - Tests: negative cases for the proof (missing lane, PR-only pin, each plan-floor breach) and the guard (collapsed filter, env parity, usage error, historical exit preserved), all deterministic and dependency-free. Advances #12342 (mechanism only). The DoD's "≥ twice daily for 7 consecutive days without cancellation" is observation-gated and requires post-merge live-CI sampling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(#12341): add pinned GitHub-native turbo cache shim + migration contract Lays the additive foundation for the move off the Vercel remote Turbo cache without doing the risky removal across ~13 workflows in this change. - .github/actions/turbo-cache-github/action.yml: pinned composite action that restores/saves the local .turbo cache via actions/cache (SHA-pinned) keyed on the deterministic turbo-cache-key.mjs hash. No SaaS, no secrets. Nothing adopts it yet — migration of individual workflows is proven safe one at a time under #12341. - packages/scripts/ci-turbo-cache-contract.mjs: static contract. (1) the shim exists, is composite, keys off turbo-cache-key, pins actions/cache to a full SHA, and carries no SaaS env; (2) no workflow that ADOPTS the shim also wires TURBO_TOKEN/TURBO_TEAM/TURBO_CACHE: remote:rw — re-adding SaaS env to a migrated workflow fails the contract. Deliberately silent about not-yet-migrated workflows; the existing dedup contract still pins the SaaS wiring that remains live (nightly/release). - Wired the contract into test.yml's `changes` job (one additive step beside the existing dedup contract) and the new proof workflow. - Negative tests: clean adopter passes; SaaS re-adder fails; floating (unpinned) actions/cache fails; SaaS-in-shim fails. Advances #12341 (mechanism + contract). Removing SaaS env from live workflows and moving PR lanes to `turbo --affected` is deferred — it edits PR-critical workflows and its "cache hit rate >= baseline" DoD is observation-gated on representative post-merge PR runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(#12338): cache the Bun install store on Linux too, pin the cache action The shared setup action skipped the Bun install-store cache on Linux (`if: runner.os != 'Linux'`), so every Linux job re-downloaded the whole dependency tree cold. #12338 explicitly asks to enable Linux Bun store caching in the shared setup action; this does exactly that and pins the cache action by SHA (it was floating `@v5`) to kill the drift the issue targets. Additive and low-risk: a cache miss is just no speedup and a cache write failure is non-fatal, so a cold Linux runner behaves exactly as before while warm runners skip the re-download. The cache key already includes this action's own content, so entries rotate correctly on this edit. Advances #12338 (this DoD item: "Linux jobs restore the Bun store cache"). The "cache logs show Linux Bun store restore" verification is observation-gated on a real post-merge Linux run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(#12342): capture the plan via a file, not a pipe; add spawn-test timeouts The matrix-proof's default plan discovery spawned run-all-tests and read its stdout through a pipe. The plan JSON is >64KB and the runner calls process.exit(0) immediately after writing it, so a piped stdout gets truncated mid-flush (SyntaxError: Unterminated JSON at byte 65536). Redirect the runner's stdout to a temp file instead — a file descriptor is flushed on close, so capture is lossless. This mirrors how the CI workflow already invokes it (`> plan.json`, which was never affected). Also give the runner-spawning tests explicit 60s timeouts. They do whole-repo workspace discovery and were tripping bun's default per-test timeout on a cold/contended runner, producing a flaky red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: cover remote deep-link profile registry sync * style: format CI lane manifest --------- Co-authored-by: Shaw <shawgotbags@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * test: verify A1 task initiation live proof --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Shaw <shawgotbags@gmail.com>
|
❌ PR title does not match the required pattern. Please use one of these formats:
|
What & why
Advances #12284 (parent #12186) — work items 6 + 8. Item 6's intensity lookup landed in #12952 (
reminderIntensity→ no-reply ladder, PA-sideresolveNoReplyPolicyseam). What was still missing:The change
One mechanism, two signals: the quiet streak modulates behavior through the same
reminderIntensitylookup #12952 added, not a second policy engine.no-reply-intensity.ts— new puresoftenReminderIntensityForQuietStreak: a ≥3-day ignored-checkin/followup streak steps the effective intensity one notch down (persistent→normal,normal/unset→minimal;minimalandhigh_priority_onlyare fixed points). Never a guilt-framed extra poke — strictly less chasing.scheduler.ts—processDueScheduledTasksderives the quiet streak once per tick (via the samederiveQuietObservationshelpers the watcher uses,asOf-pinned to the tick'snow) and threads it intoresolveNoReplyPolicy. Softening applies toreminder/checkinonly; approvals keep full cadence (they gate agent-side actions; sensitive approvals fail closed). The applied intensity + softening decision are persisted onmetadata.noReplyState(appliedReminderIntensity,quietStreakSoftened,quietStreakDays) so the ladder choice is auditable in the task record, not just in behavior.recordTaskStateEntry, J7-guarded — a failed telemetry append never undoes a committed transition). Owner replies append the streak-breakingcompletedentry through both completion seams (processScheduledTaskInboundMessageandinbound-reply-completion.ts).recent-task-states.ts—summarize({ asOf })pins the lookback window for deterministic tick-time evaluation; the log gets a 500-entry retention cap now that production writes exist.quiet-user-watcher.ts— exportsquietStreakDaysFromObservations;runQuietUserWatcheracceptsasOf. Brief-text behavior unchanged.Frozen
ScheduledTaskcontract intact: no trigger-union or schema changes — this is policy-selection lookup only, exactly as AGENTS.md:156-171 requires (structural fields, neverpromptInstructionstext; no second scheduler).Deviations from the issue text
escalatedintensity; theReminderIntensityenum on develop isminimal | normal | persistent | high_priority_only— there is noescalatedvalue to handle.resolveNoReplyPolicy, notplugin-scheduling's escalation ladder — same rationale as feat(#12284): no-reply loop respects owner reminderIntensity (item 6) #12952: the spine escalation ladder drives dispatch-failure channel advance, a delivery concern owner intensity must not touch.plugin-schedulingis untouched.Verification
scheduler.quiet-streak.test.ts(new, real repository-backed runtime, no scheduler mocks) — 6 passed: 3 ignored check-ins driven through REAL ticks (log's production writer exercised on fire + terminal paths) soften the next reminder to fire-once with the decision visible innoReplyState; contrast persona with no history keeps the 60-min re-nudge; a reply breaks the streak (no softening); explicitpersistentowner softens one notch tonormal; approvals not softened; owner reply through the realMESSAGE_RECEIVEDseam appends the streak-breaking log entry.no-reply-intensity.test.ts— 8 passed (existing 6 kept green + 2 softener unit cases).scheduler.no-reply-policy.test.ts— 9 passed (existing 8 kept green + new auditability case assertingappliedReminderIntensitypersisted on retry). Two otherwise-identical tasks with differentreminderIntensityfacts → measurably different retry/ladder behavior (pre-existing feat(#12284): no-reply loop respects owner reminderIntensity (item 6) #12952 cases, still green).inbound-reply-completion.integration.test.ts— 5 passed;recent-task-states.integration.test.ts— 2 passed; default-pack spine-seeding + smoke — 9 passed.bun run --cwd plugins/plugin-scheduling test— 238 passed (19 files), untouched package stays green.bun run --cwd plugins/plugin-personal-assistant typecheck— exit 0; packagelint:check— 0 errors (17 pre-existing warnings).bun run --cwd plugins/plugin-personal-assistant test): 1144 passed | 9 failed | 7 skipped (1160). All 9 failures are pre-existing on cleanorigin/develop(b24dec8) in 4 files this PR does not touch — reproduced verbatim on a detached clean checkout (same 9 tests, same 4 files):life-reminder-datetime.test.ts(1 — the test(#12283): A2 rage-quit delete-trap regression (pending) — surfaces a destructive-action safety gap #12998 rage-quit delete-trap known-gap regression),reminders-service.process-scheduled-work.test.ts(5 —travel_reconcilesubsystem from feat(#12284 item 3): real travelActive owner-fact drives the during_travel gate #13211,cache.getCache is not a functionagainst that test's mock runtime),signature-deadline-scheduler.test.ts(1),BlockerSettingsCards.test.tsx(2). Every scheduled-task/no-reply/quiet-streak/recent-task-states suite is green.Full verify gate (rebased on develop @ 76c9253)
bun run verifycomponent-by-component:check:agents-claudePASS ·audit:type-safety-ratchetPASS ·audit:error-policy-ratchetPASS ("no new fallback-slop in touched files") · turbotypecheck lint --continue: 571/578 tasks green — the 7 failures (app#typecheck,electrobun#typecheck= the known-red baseline, pluselectrobun#lint,plugin-capacitor-bridge#typecheck,plugin-computeruse#lint,plugin-sub-agent-claude-code#typecheck,tui#lint) reproduce as the byte-identical failure set on a clean detached checkout oforigin/develop@ 76c9253 (this branch touches onlyplugins/plugin-personal-assistant+ one evidence file; every failing package is byte-identical to develop here) ·audit:build-modelPASS ·audit:turbo-build-depsFAIL — pre-existing, reproduced identically on clean develop (phantom turbo.json#buildoverrides for example packages left by the recent script-normalization merges; turbo.json and those packages are untouched by this branch) ·audit:tee-secret-leakPASS ·audit:scriptsPASS ·audit:test-realnessPASS (report-only) ·typecheck:distPASS (28 configs).So: verify green except pre-existing develop failures app#typecheck/electrobun#typecheck (untouched by this PR), plus the five newer pre-existing develop-drift failures and the pre-existing
audit:turbo-build-depsbreak listed above, each reproduced on clean develop.noReplyPolicy/noReplyState(incl.appliedReminderIntensity,quietStreakSoftened,quietStreakDays) + recent-task-states log entries — asserted against the real repository in the integration tests above.[lifeops-scheduled-task]warn +runtime.reportErrorpath on log-append failure (J7).Advances #12284. Relates to #12186.
🤖 Generated with Claude Code