[propr-dev]

AI Implementation Summary

Closes #3442

Branch: 3442/claude-opus-5-5-flag-automation-runs-that-20261111-2200-km0
Commits: ✅ Changes committed (60cb085)

---

AI Processing Completed

Execution Details:

• Issue: Flag automation runs that repeat the same report or act without changing anything #3442
• Repository: integry/propr
• Status: Success
• Execution Time: 37m 44s
• Tokens used: 39.46M tokens [39.36M input + 101.75K output]
• API cost: $11.52
• Timestamp: Nov 11, 2026, 10:38 PM UTC
• LLM Model: Claude Opus 5.5
• Subscription usage: Weekly +4%, Fable +4%

Summary:
Runs that produce nothing are now flagged on every surface, and the owner gets one Inbox notification after N unproductive scheduled runs in a row. Typecheck and lint pass, and every test suite I ran passes. I didn't run the full test suite, and I only ran the migration against SQLite, not Postgres.

How runs are flagged

• Report novelty: when a report is stored, the run gets a hash of the normalised report text and a report_novelty of new, unchanged or empty. Normalisation collapses whitespace and replaces dates, times, weekdays, epoch timestamps, "3 hours ago"-style times, UUIDs and run ids. A report under 40 characters after normalisation is empty. The previous run's report is re-hashed rather than read from its stored hash, so runs from before this change compare correctly. Version numbers like 4.17.21 are left alone.
• Acting steps: the revoke endpoint now also returns how many mutating operations the grant performed. That's the count of mcp_operations rows for the grant, minus those that definitely failed. The acting job now revokes the grant before it marks the run completed, so the count is stored while the run is still acting. If the count can't be read it stays null, which is not treated as a no-op.
• What counts as unproductive: an empty or unchanged report, or an acting step with 0 changes. An acting step that changed something makes the run productive, whatever its report said. The state machine is unchanged.
• Where it shows: the REST run payload, MCP get_automation_run / list_automation_runs, the CLI (a new Change column in runs, a change: line in report), and a "No change" badge in the runs list and run detail. I also added a derived noChangeReason field to the payload so every client shows the same badge.

Streak, notification and auto-pause

• Only scheduled runs count. Manual runs are flagged but neither extend nor reset the streak.
• The notification fires once, when the streak reaches exactly N. A retry can't create a second one, and the next productive scheduled run resets the streak.
• N is a new instance setting, automation_unproductive_run_threshold (1–50, default 3). It's available through the API, MCP and the settings catalog, but not in the Settings UI, the same as the existing usage-pause setting.
• The per-automation option pauseWhenUnproductive (off by default) appears as a checkbox under the schedule. When the notification fires, it turns the schedule off through disableAgentDefinitionSchedule, logs the reason, and says so in the notification.
• The migration adds three nullable run columns and the definition flag, and reverses cleanly; existing runs keep null flags.

Decisions you may want to revisit

• Automations without a repository: the existing notification code skips these. I didn't want this one silently dropped, so they're notified under the system_failure category instead. Say if you'd rather they get no notification.
• Previous report always read: the novelty check reads the latest previous report even when "include previous reports" is off. I changed the one existing test that expected no read in that case.
• Layout fix: the badge pushed the Duration and Report columns out of line in the wide runs list, so I widened the State column. The automation-runs e2e specs still pass.
• Line cap: processAgentRunJob.ts was already at the 400-line lint limit, so I moved agentReportRecap into its own file and kept the old export.

Tests and docs

• New test files cover hashing and normalisation, the migration, the streak rules, the notification, auto-pause and the setting's validation.
• I extended the job, grant client, internal route, REST, MCP, CLI and UI tests, including the acting-step count.
• The automations docs page has a new section, and the settings-locations page was regenerated.

Previews of the runs list (desktop and phone), the run detail, and the new schedule checkbox were captured with Playwright.

Detailed Logs:

• Conversation: 53 messages
• Session: 77f36b09-bc1b-45e7-a1dd-661b41e959f2

Log files stored at:

• conversation: /usr/src/app/logs/executions/issue-3442-2026-11-11T22-38-41-631Z-conversation.json
• output: /usr/src/app/logs/executions/issue-3442-2026-11-11T22-38-41-631Z-output.txt

Latest Conversation Messages

ASSISTANT: [content unavailable]

ASSISTANT: Runs that produce nothing are now flagged on every surface, and the owner gets one Inbox notification after N unproductive scheduled runs in a row. Typecheck and lint pass, and every test suite I ran ...

---

This PR was created automatically by ProPR after processing issue #3442.

---

💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

---

Visual preview

Run detail of an unchanged run

The run header shows the same "No change" badge next to the Completed state.

Connect sign-in required. Original retained until 2027-02-09T22:38:46.148Z.

View the full-resolution original in ProPR Connect

Run history with "No change" badges (desktop)

Scheduled runs whose report was unchanged or empty, or whose acting step changed nothing, still show Completed but carry a "No change" badge; productive runs have none. Columns stay aligned.

Connect sign-in required. Original retained until 2027-02-09T22:38:47.565Z.

View the full-resolution original in ProPR Connect

Run history on a 390px phone

The badge sits beside the state on the stacked phone layout without horizontal panning.

Connect sign-in required. Original retained until 2027-02-09T22:38:48.991Z.

View the full-resolution original in ProPR Connect

Schedule option to pause unproductive automations

New per-automation checkbox under the cron schedule (off by default) that turns the schedule off when the unproductive-run notification fires.

Connect sign-in required. Original retained until 2027-02-09T22:38:50.156Z.

View the full-resolution original in ProPR Connect

[propr-dev]

/ultrafix goal=8 max=10
Triggered automatically by the requested execution settings.

[propr-dev]

🔄 Ultrafix loop started (goal: 8/10, max cycles: 10)

First action: /review

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds productivity flags across storage, workers, APIs, CLI and UI, with substantial focused test coverage. It needs changes before merge because streak evaluation can act on unfinished runs and miss qualifying no-op runs or notifications.

The supplied issue specification is truncated; this review uses its visible requirements and the correctness of the changed behavior. Verification was by static trace only; no commands were run. Current checks show 37 passed, none failed and one pending.

✅ Consistent presentation — REST and MCP expose the stored flags, and the UI uses the shared no-change classification.

✅ Backward-compatible storage — Existing runs retain nullable flags, and automatic schedule pausing defaults to off.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F1: 🔴 Awaiting approval is treated as final

• Required behavior: Streak decisions must respect acting steps that can still make a run productive; unfinished runs must not cause premature notifications or schedule pauses.

• Evidence: packages/core/src/services/agents/agentRunProductivity.ts:22–31, src/jobs/processAgentRunJob.ts:317 — the undecided-state set excludes awaiting_approval.

  1. A scheduled preview automation produces three unchanged reports and waits for approval after each report.
  2. advanceAfterReport returns each awaiting_approval run, and the report processor passes it to evaluateStreak.
  3. agentRunProductivity treats these runs as unproductive because only report_ready and acting are pending.
  4. The third report triggers a notification and, when opted in, disables the schedule before the owner approves any acting step.
  5. Approving those steps can subsequently produce mutations, making those same runs productive after the schedule has already been paused.

  The same path applies to auto runs held for approval by the cost gate. Notification deduplication does not protect against a premature verdict. static trace: the supplied autonomy transitions, new evaluation call and productivity predicate establish this sequence.

• Minimum fix: Treat awaiting_approval as pending while acting remains possible. Evaluate the final verdict after approval/action completion or rejection, including the relevant terminal decision paths.

F2: 🔴 Known action counts require unrelated report flags

• Required behavior: A scheduled acting step with zero mutating operations must count as unproductive independently of whether report comparison succeeded.

• Evidence: packages/core/src/services/agents/agentRunProductivity.ts:28–31,71–73, src/jobs/agentRuns/reportNovelty.ts:22–27.

  1. Reading the previous report fails during novelty classification; the explicitly supported fallback returns an empty patch, leaving reportNovelty null.
  2. The report is stored successfully, the acting step completes, and grant revocation records actingMutations: 0.
  3. The action processor evaluates the completed run, but agentRunProductivity immediately returns pending because report novelty is null.
  4. Future streak queries also exclude that run through whereNotNull('report_novelty'). Consequently, a qualifying no-op run never contributes to notification or pausing.

  A positive mutation count is likewise excluded instead of resetting a streak. The public shared classifier already handles both counts independently of report novelty, so the badge and streak can disagree. static trace: the novelty fallback, action-completion patch, early return and query filter establish the failure.

• Minimum fix: Use known acting counts independently of report novelty, and include rows with known acting counts in streak queries. Preserve pending behavior when neither source provides a final verdict.

F3: 🔴 A failed threshold notification is permanently skipped

• Required behavior: The owner must receive the streak notification; suppressing duplicates must not suppress an item that was never persisted.

• Evidence: packages/core/src/services/agents/agentRunProductivity.ts:82, src/jobs/agentRuns/unproductiveStreak.ts:79–80,93–118.

  1. Three scheduled runs become unproductive at the default threshold.
  2. Evaluation reaches createEvent, but notification persistence rejects.
  3. evaluateUnproductiveStreakQuietly catches the error, allowing the worker to finish without retrying the notification.
  4. The fourth and every later unproductive run produces a streak longer than the threshold. reachedThreshold is false, so notification creation is never attempted again during that streak.

  The owner receives no warning despite continued unproductive runs. The existing deduplication key prevents duplicate successful writes but cannot recover a write never attempted again. static trace: exact-threshold eligibility and swallowed notification errors demonstrate the failure without requiring concurrent execution.

• Minimum fix: Retain a durable retry obligation or allow later runs to retry notification using a stable streak identity until persistence succeeds. Continue deduplicating successful notifications.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Align mutation-count documentation

The new documentation says counting occurs before grant revocation, while the endpoint counts afterward. Align the description with the implementation. This wording correction is optional and does not itself establish a runtime defect.

Score

The implementation covers the main surfaces well, but three streak correctness defects remain despite the passing checks. These require focused corrections before merge.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 13s
• Tokens: 87,007 (85,541 in / 1,466 out)
• Cost: $0.80

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F1 F2 F3 S1

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 60bfaec

Addressed 1 AI review comment (IDs: 6809269774)

Addressed findings F1, F2, F3

Summary of Changes

I fixed F1, F2 and F3. Typecheck and zero-warning lint pass for the root, core and API. The streak, autonomy/approval route, both job processors, the automation REST routes and the MCP run tests pass: 202 tests, 0 failures. To check the new tests, I put back the old logic for each finding, confirmed the tests then fail, and restored the fix. I didn't run the full test suite.

F1 – a run waiting for approval was treated as final

• awaiting_approval now counts as pending, alongside report_ready and acting. This covers preview runs and auto runs held back by the cost gate, so they no longer extend a streak, notify the owner or pause the schedule.
• Once the acting step finishes, the action job evaluates the run as before. Rejection now also triggers an evaluation: the reject route (which MCP reject_automation_run also goes through) checks the streak after the run moves to rejected. A failure in that check never fails the reject request.

F2 – known action counts needed a report comparison

• A known actingMutations count now decides the verdict by itself, whether or not the report could be compared. Zero makes the run unproductive and more than zero makes it productive. A run is only pending when neither the count nor the report comparison gives an answer.
• The streak query now includes runs where either flag is set, not only report_novelty.

F3 – a failed notification was skipped for good

• The streak has a stable identity: the productive scheduled run just before it, or start if there is none. It no longer changes as the streak grows, and the notification's deduplication key is built from it.
• Any unproductive scheduled run in a streak at or past the threshold now checks whether that notification was actually saved, and sends it if not. So a failed write is retried by the next run, and once it is saved, later runs neither notify nor pause again.
• The schedule is now paused only after the notification is saved. In the old order, a paused schedule meant no later scheduled run would come along to retry a lost notification.
• The streak lookup now pages back through up to 1,000 flagged scheduled runs. A streak longer than that gets an identity from the oldest run read, so the owner could get a second notification after 1,000+ unproductive runs in a row.

Tests and docs

• New tests in test/agentRunProductivity.test.ts cover: waiting runs that are later approved or rejected, runs whose report couldn't be compared, a stable streak identity, and a notification that fails and is then retried with the pause applied only after it's saved. One test uses the real notification service on SQLite to show a saved notification is never repeated.
• test/agentAutonomy.test.ts checks that rejecting a run hands it to the streak check.
• The automations docs page now says a failed notification is retried and that runs waiting for approval aren't counted.

Separate issues I found but didn't fix (out of scope)

• If the notification is saved but pausing the schedule then fails, nothing retries the pause, and the notification still says the schedule was paused.
• Runs that fail or are cancelled after their report is stored are counted using their report flag, but no streak check runs at that moment. With the F3 retry, the next unproductive scheduled run picks them up.

This doesn't change anything visible in the UI, so I didn't capture previews.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (medium)
• Requested By: @propr-ultrafix
• Time: 15m 29s
• Tokens: 3,470,114 (3,439,563 in / 30,551 out)
• Cost: $1.85
• Subscription usage: Weekly +2%, Fable +2%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

This PR adds report novelty, acting-operation counts, client-visible flags, and notifications with optional schedule pausing. It needs changes before merge: notification deduplication breaks for long streaks, and a failed schedule pause cannot recover.

✅ Consistent presentation — REST, MCP, CLI, and UI expose productivity flags while preserving run states.

✅ Unknown counts remain unknown — Revocation failures retain a null mutation count instead of falsely recording zero.

✅ Approval-aware classification — Pending acting steps do not receive a final streak verdict until completion or rejection.

The issue specification is explicitly truncated; this review uses its visible requirements and the supplied changed behavior. The authoritative checks show 37 passed, none failed, and one pending. No commands or tests were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F4: 🔴 Recover an unsuccessful schedule pause

• Required behavior: An opted-in automation must have its schedule paused when the unproductive-streak notification fires; notification persistence must not permanently suppress an unfinished pause.

• Evidence: src/jobs/agentRuns/unproductiveStreak.ts, evaluateUnproductiveStreak — notification-existence guard and subsequent pauseSchedule call.

  1. An opted-in automation reaches the threshold, and createEvent successfully persists the notification claiming its schedule has been paused.
  2. The subsequent awaited disableSchedule operation fails, or the worker stops between notification persistence and that operation.
  3. The schedule remains enabled and produces another unproductive run.
  4. Its evaluation finds the persisted notification and returns immediately, before attempting the unfinished pause.

  The owner receives an incorrect pause confirmation, and scheduled spending continues indefinitely through the same streak. Notification deduplication protects the message but also suppresses recovery of the separate schedule update. The existing retry test covers notification-write failure, not failure after persistence. static trace: the notification is committed before the disable call, and every subsequent evaluation exits at hasNotification.

• Minimum fix: Persist whether the pause obligation completed and retry an unfinished pause independently of notification deduplication, or commit notification persistence and schedule disabling together. Preserve the existing behavior that an explicit user re-enable after a successful pause is respected.

F5: 🔴 Keep streak identity stable beyond the scan limit

• Required behavior: An uninterrupted unproductive streak must generate only one notification until a productive scheduled run resets it.

• Evidence: packages/core/src/services/agents/agentRunProductivity.ts, findUnproductiveScheduledStreak — STREAK_SCAN_LIMIT and the scan:${oldestRead?.id} fallback.

  1. A scheduled automation with pausing disabled produces only empty reports. Its initial threshold notification uses streak key start.
  2. At 1,000 flagged runs, the scan consumes ten full pages without finding a productive breaker or a short page. It returns scan:<oldest-run-id> instead of start.
  3. That new key has no notification, so the same uninterrupted streak generates another Inbox item.
  4. Each following run shifts the 1,000-row window, changes the oldest ID, and generates another notification.

  This turns once-per-streak notification into notification on every run; at the supported 15-minute interval, the boundary arrives after roughly ten days. Database deduplication cannot prevent duplicates because each key differs. static trace: the bounded keyset walk derives identity from the moving scan boundary; supplied tests exercise short streaks only.

• Minimum fix: Derive the streak identity independently of the moving scan boundary, such as by persisting its identity or locating the actual productive predecessor separately. Add coverage across the 999-, 1,000-, and 1,001-run boundary.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation covers the main surfaces and includes substantial tests, but two persistent-state defects undermine notification and pause guarantees. Both appear fixable without broad redesign.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 53s
• Tokens: 89,685 (88,389 in / 1,296 out)
• Cost: $0.84

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F4 F5

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit e0dc8ec

Addressed 1 AI review comment (IDs: 6810128226)

Addressed findings F4, F5

Summary of Changes

I fixed F4 and F5. Typecheck and zero-warning lint pass for the root, core and API. The streak tests, the two job processors, the autonomy, definition-route, MCP-run and grant tests all pass: 233 tests, 0 failures. To check the new tests, I put back the old logic for each finding, saw 9 of them fail, and restored the fix. I didn't run the full test suite, and I only ran the migration against SQLite, not Postgres.

F5: streak identity past the 1,000-run scan limit

• When the scan runs out before finding the run that ended the previous streak, findUnproductiveScheduledStreak now looks that run up with one extra query: the newest productive scheduled run older than the scan window. The scan:<oldest-id> fallback is gone. A streak longer than 1,000 runs keeps the same key it had when it was shorter: start, or the id of the productive run before it.
• That query restates in SQL the same "productive" rule the code applies: more than zero changes from the acting step, or no acting count and a new report once no acting step can still run.
• New tests:
  • streaks of 999, 1,000, 1,001 and 1,150 runs, with and without a productive run before them.
  • pending and manual runs beyond the window, which don't change the key.
  • a test that checks every combination of run state, report novelty and acting count, so the SQL rule and the code can't drift apart.
  • a full evaluation across the boundary, which sends exactly one notification.

F4: a pause that didn't finish

• The definition now records which streak's pause was carried out, in a new agent_definitions.unproductive_paused_streak column. I added it to this PR's existing migration rather than a new one.
• pauseScheduleForUnproductiveStreak writes that record in the same transaction as turning the schedule off. The write is conditional, so if two evaluations run at once only one of them pauses.
• When a saved notification says the schedule was paused, the evaluator no longer just stops. It tries the pause again, and the record makes that a no-op once the pause has happened. So:
  • a disable that failed, or a worker that stopped after the notification was saved, gets finished by the next unproductive run, with no second notification.
  • an owner who turns the schedule back on after a successful pause isn't overridden.
  • an owner who turns pausing off after a failed pause isn't overridden either.
  • a notification that didn't say "paused" never leads to a pause later.
• To know what the saved notification said, the evaluator now reads its schedulePaused value through findUnproductiveStreakNotification, instead of only checking that it exists. The REST route's version of the evaluator now uses the new functions too.
• New tests cover the failed disable, the interrupted worker, the owner turning the schedule back on, the owner turning pausing off, a notification that didn't claim a pause, and two pauses racing.

I also updated the pausing paragraph in docs/docs/features/agents.md to describe the retry, and adjusted the existing streak tests to the renamed parts. The new tests are in test/agentRunUnproductiveStreakRecovery.test.ts.

Something else I noticed: a notification couldn't be updated to mark the pause as done, because saved notifications can't be changed after they're written. That's why the record lives on the definition instead. Nothing here changes what users see, so there are no previews.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (medium)
• Requested By: @propr-ultrafix
• Time: 14m 15s
• Tokens: 2,696,696 (2,669,154 in / 27,542 out)
• Cost: $1.56
• Subscription usage: Weekly +1%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, shared presentation, and owner notifications with optional schedule pausing. It needs changes before merge: normalization can hide meaningful report differences, and schedule pausing can override a concurrent opt-out.

✅ Consistent presentation — REST, MCP, CLI, and UI expose the productivity flags, with shared logic for the no-change verdict.

✅ Durable pause recovery — Notification deduplication and transactional pause records cover interrupted pauses and subsequent schedule re-enablement.

✅ Broad regression coverage — Added tests exercise migration compatibility, unknown mutation counts, approval states, notification failures, and long streaks.

The issue specification is explicitly truncated; this review assesses its visible requirements and the correctness of the supplied changes. Review was static, with no commands executed. Current checks show 37 passed, none failed, and one pending.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F6: 🔴 Preserve meaningful UUIDs in reports

• Required behavior: Report normalization should remove volatile run identifiers without erasing substantive differences between reports.

• Evidence: packages/core/src/services/agents/agentRunNovelty.ts:11, VOLATILE_PATTERNS and normalizeAgentReport.

  1. A scheduled report says Unresolved incident 3d934921-c7ae-4cb4-add8-b82d109ed330 requires investigation and escalation to the owner.
  2. The next report has identical wording but identifies a different incident, 1f731cb6-c2ad-4aa4-b8c4-ad5fcbf92f1c. Neither identifier is an automation run ID.
  3. The unconditional UUID replacement converts both incident identifiers to <id>. Both reports exceed the minimum length and produce identical hashes, so the second is classified as unchanged.
  4. Repeating this pattern with distinct incidents accumulates an unproductive streak and can disable an automation that is reporting new incidents.

  The explicit runIds replacement does not protect other identifiers: the subsequent UUID pattern replaces every UUID regardless of its role. Existing tests expressly confirm that broad replacement but do not test meaningful entity changes. static trace: deterministic evaluation of the replacement and classification code, followed by the streak evaluator.

• Minimum fix: Normalize known run identifiers without blanket replacement of unrelated UUIDs. Add a regression asserting that changing an incident or task UUID produces new, while changing the report’s run ID remains unchanged.

F7: 🔴 Recheck opt-in when pausing

• Required behavior: Automatic schedule disabling must respect the definition’s current pauseWhenUnproductive setting, including an owner turning it off during evaluation.

• Evidence: src/jobs/agentRuns/unproductiveStreak.ts, evaluateUnproductiveStreak; packages/core/src/services/agents/agentRunProductivity.ts, pauseScheduleForUnproductiveStreak.

  1. An unproductive scheduled run reaches the threshold. Evaluation loads a definition with pauseWhenUnproductive: true and saves that value in pause.
  2. Evaluation awaits createEvent. Notification creation persists the event and then awaits announcement delivery, as shown in the supplied notification-service context.
  3. During that window, the owner successfully updates the definition to pauseWhenUnproductive: false, leaving its schedule enabled.
  4. Notification creation returns. Evaluation uses the previously captured pause value and calls pauseScheduleForUnproductiveStreak.
  5. The transaction checks only the definition ID and previous streak marker. It clears the pending slot and disables the schedule despite the now-disabled opt-in.

  The same stale decision exists between getDefinition and carryOutPause on the stored-notification recovery branch. The transaction serializes competing pause attempts, but never checks the current opt-in; the owner’s revision update therefore cannot prevent this mutation. The existing opt-out test changes the setting before evaluation starts and misses this sequence. static trace: the awaited notification path, supported definition-update operation, and unconditional transactional pause predicates.

• Minimum fix: Check pause_when_unproductive within the pause transaction before claiming the streak, clearing pending slots, or disabling the schedule. Apply that protection to both initial and recovery calls, and test an opt-out committed during notification creation.

Suggestions

These are optional follow-ups and are not sent to /fix.

S2: 🟢 Correct mutation-count timing documentation

docs/docs/features/agents.md says operations are counted before grant revocation, while the endpoint revokes first and then counts. Align the description with the implementation. This wording correction is optional and does not itself affect runtime behavior.

Score

The implementation has substantial coverage and no current failed checks, but two correctness defects can incorrectly pause productive or opted-out automations. Both admit localized fixes.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 18s
• Tokens: 96,668 (94,952 in / 1,716 out)
• Cost: $0.90

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F6 F7 S2

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 63e86a5

Addressed 1 AI review comment (IDs: 6810756588)

Addressed findings F6, F7

Summary of Changes

I fixed F6 and F7, and each fix has regression tests. The 6 related test files pass (192 tests, 0 failures), and typecheck and zero-warning lint pass for the root and core. I didn't run the full test suite, and nothing ran against Postgres. To check the new tests, I put back the old logic for each change and confirmed they fail, then restored the fix.

F6: other UUIDs in reports are kept

• Change: agentRunNovelty.ts no longer replaces every UUID. It only replaces the run ids it is given, ignoring upper/lower case, and longest first so one id never cuts into another. Any other id is kept, so a report naming a different incident or task is new.
• Same problem one step back: the report prompt shows the current run id plus up to 5 earlier run ids. So the previous report can quote a run one further back than the new report can see. Only the blanket UUID rule had been hiding those ids. reportNovelty.ts now reads the last 6 reported runs, compares against the newest, and treats all their ids as run ids. Without this, reports that only quote a different run id would have turned new.
• Tests:
  • a different incident or task UUID gives new, while a different run id stays unchanged.
  • run ids in different upper/lower case are still treated as the same run, and other UUIDs are kept.
  • in the job processor, the previous report can quote a run from beyond the current prompt and is still unchanged. A different incident is new, and the test fails if the lookup is cut back to one report.

F7: the pause checks the opt-in at the moment it runs

• Change: pauseScheduleForUnproductiveStreak now only claims the streak if pause_when_unproductive is still true, inside the same transaction. If it isn't, it returns a new result, opted_out, and changes nothing: no claim, the pending slot is kept, and the schedule stays on.
• Both paths covered: the check is inside the transaction, so it applies to the first pause after the notification and to the later retry of a failed pause. In unproductiveStreak.ts, opted_out now gives the outcome notified on the first path and null on the retry. A pause that was already made still counts as made, even if the owner opts out afterwards.
• Tests:
  • the owner opts out during notification delivery, through the real NotificationService with an opt-out hooked into its update-publishing step. The schedule stays on, the pending slot is kept and the streak is not claimed.
  • the owner opts out between the retry reading the definition and the pause.
  • the pause function on its own: opting out, opting back in, and opting out after the pause was made.
• Lint fix: the new branch pushed evaluateUnproductiveStreak over the complexity limit (21, max 20). I replaced the branching with two small outcome tables, one per path.

I also added a sentence to each of the two matching paragraphs in docs/docs/features/agents.md. Nothing here changes what users see in the UI, so there are no preview images.

Found but not changed:

• If the owner opts out while the Inbox item is being delivered, the item has already been saved saying the schedule was paused. Its text will then be wrong, even though the schedule correctly stays on. Fixing that would mean editing a saved notification, which is outside F7. The owner caused this themselves during a very short window.
• The only per-run ids the agent can see in its prompt are run ids, so I found no other changing id that needs replacing.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (high)
• Requested By: @propr-ultrafix
• Time: 12m 31s
• Tokens: 3,406,897 (3,382,652 in / 24,245 out)
• Cost: $1.61

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds productivity flags across storage, workers, API/MCP, CLI and UI, plus streak notifications and optional schedule pausing. It needs changes before merge because report selection can misclassify repeated reports, and a stale streak evaluation can pause a schedule after a productive run.

✅ Consistent client presentation — REST and MCP expose the derived no-change reason, with CLI output and UI badges covered by tests.

✅ Durable pause recovery — The schedule change and pause marker share a transaction; tests cover interrupted pauses, opt-out changes and manual re-enabling.

The supplied checks show 37 passed, none failed and one pending. This review is a static analysis of the supplied code; no commands were run. The issue specification is explicitly truncated, so requirements beyond its visible portion could not be assessed.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F8: 🔴 Compare against the actual previous reported run

• Required behavior: Report novelty must compare with the previous reported run of the definition.

• Evidence: src/jobs/agentRuns/reportNovelty.ts:32 — reportNoveltyPatch reuses the prompt-history lookup.

  1. A run stores report A and completes.
  2. A subsequent acting automation stores a different report B, then its acting step fails.
  3. The next run stores B again. listPreviousReports excludes failed runs through its state filter, so recent[0] is the run containing A.
  4. The new classifier records B as new, although the immediately previous reported run already stored B. Repeated acting failures can therefore keep identical reports classified as new.

  The same lookup also filters on created_at < current.createdAt and sorts by creation time. An older deferred run that reports after a newer run excludes that newer report entirely, producing the same incorrect comparison.

  static trace: the changed caller passes the creation-time boundary directly into the supplied agentRunReports.ts:25–43 implementation. Its eligibility and ordering serve prompt history; neither checks the latest stored report by reporting time. Existing novelty tests mock the lookup and do not exercise these cases.

• Minimum fix: Select the previous stored report independently of the current run’s creation time and the previous run’s subsequent acting outcome, using reporting order and a deterministic tie-breaker. Keep prompt-history selection separate and add regressions for failed acting steps and deferred runs completing out of creation order.

F9: 🔴 Revalidate the streak before pausing

• Required behavior: Automatic pausing must apply to the current unproductive streak; a productive scheduled run resets that streak.

• Evidence: src/jobs/agentRuns/unproductiveStreak.ts:127–168 and packages/core/src/services/agents/agentRunProductivity.ts:172–195 — The pause checks the opt-in and prior pause marker, but never checks whether the evaluated streak remains current.

  1. A definition has pausing enabled and enough unproductive scheduled runs to meet its threshold. A newer scheduled run is acting, so the streak scan skips it as pending.
  2. An unproductive run’s evaluation finds that streak and awaits createEvent. Notification creation commits the item and then awaits announcement delivery.
  3. During that await, the newer acting run completes with a positive mutation count. It is now productive and ends the current streak; its own evaluation returns without pausing.
  4. The earlier evaluation resumes and calls pauseSchedule. The definition still opts in and the old streak has no pause marker, so the transaction disables the schedule despite the newer productive run.

  The persistent result is an incorrectly disabled schedule. The pause transaction serializes definition updates and protects opt-out changes, but contains no condition on run productivity or the current streak. The competing action completion updates a run and is not excluded by that transaction.

  static trace: this follows the supplied notification commit/announcement boundary and the changed acting-completion path. The recovery tests exercise changes during announcement delivery, but only revalidate opt-in changes.

• Minimum fix: Revalidate the streak identity and threshold when committing the pause, with database coordination that prevents a concurrent productive completion from invalidating that check before the schedule update. Apply this to both initial and recovered pauses, and test productive completion during notification delivery.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation has broad integration and recovery coverage, and current checks show no failures. Two correctness defects remain in the decisions that classify reports and disable schedules.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 59s
• Tokens: 99,664 (97,993 in / 1,671 out)
• Cost: $0.95

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F8 F9

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit ef36871

Addressed 1 AI review comment (IDs: 6811342808)

Addressed findings F8, F9

Summary of Changes

I fixed F8 and F9, and each fix has regression tests. Root, core and API typecheck pass, and zero-warning lint passes for root src/, core and the API files I touched. The 9 related root test files pass (223 tests, 0 failures), and so do the API definition-route, grant and MCP-run suites (28, 15 and 16 tests). To check the new tests, I put back the old logic for each finding: the new tests then fail, and pass again with the fix. I didn't run the full test suite, and nothing ran against Postgres; production here is SQLite only.

One test-run problem: the MCP-run suite passed all 16 tests but then stayed open until my 500s timeout. With --test-force-exit it ends normally with the same 16 passes. My changes don't touch that file or add timers. I didn't run it on a clean checkout, so I haven't confirmed the hang is already there without my changes.

F8: novelty compares with the report stored last

• New listLatestStoredReports in packages/core/src/services/agents/agentRunReports.ts picks reports by when they were stored, newest first, with run id as the tie-breaker. It has no creation-time cutoff and keeps runs whose acting step later failed.
• The prompt-history lookup (listPreviousReports) is unchanged and still feeds the prompt.
• reportNoveltyPatch compares against the first result of the new lookup. It uses both lookups only to collect the run ids a report may quote.
• New tests in test/agentRunReportNoveltyPrevious.test.ts run against a real SQLite store:
  • A report B from a run whose acting step then failed makes the next B unchanged, including after repeated failures.
  • An older, deferred run that reports after a newer run is compared with that newer run's report.
  • Prompt history still leaves out failed runs.
  • Reports stored at the same moment are ordered by run id.
• There is also a processor-level test in test/processAgentRunJob.test.ts.

F9: the streak is rechecked inside the pause transaction

• pauseScheduleForUnproductiveStreak now receives the streak the evaluation found (its identity and threshold). After the opt-in claim, it re-reads the streak inside the same transaction. If the streak changed or no longer meets the threshold, the transaction rolls back, including the claim, and returns a new result, streak_ended. That result maps to notified after a new Inbox item and to nothing after a stored one.
• The claim is the transaction's first write, so it takes SQLite's write lock before the re-read. A run that completes productively either commits before the re-read, and is seen, or waits until the pause commits.
• This applies to both the first pause and a recovered one. The default caller and the API route caller pass the streak through.
• New tests in test/agentRunUnproductiveStreakRecovery.test.ts:
  • An acting run that completes with changes during Inbox delivery keeps the schedule on, with no claim and the pending slot kept.
  • The same completion during a recovered pause keeps the schedule on.
  • A direct transaction check covers a changed streak and an unmet threshold.
• I updated the existing direct pause tests to build real streaks, and added one sentence each for F8 and F9 to docs/docs/features/agents.md.

Separate findings, not fixed:

• When the pause is skipped after the Inbox item was created (opted_out, and now streak_ended), the item has already said the schedule was paused.
• Two runs of the same automation that store reports at almost the same moment can each classify before the other's report is stored. Both can then come out new. The window is only between the lookup and the store; closing it would need the classification inside a locked write, so I didn't pursue it.

There are no visual changes, so I made no preview.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (high)
• Requested By: @propr-ultrafix
• Time: 35m 21s
• Tokens: 6,807,341 (6,806,622 in / 719 out)
• Cost: $2.04
• Subscription usage: Weekly +1%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, consistent client flags, and streak notifications with optional schedule pausing. It needs a targeted correction before merge: notifications can permanently claim that a schedule was paused when the pause was explicitly refused.

✅ Consistent presentation — REST, MCP, CLI, and UI expose the new productivity information without adding run states.

✅ Pause recovery — The transactional pause record prevents repeated disabling after an owner re-enables the schedule, while allowing interrupted pauses to be retried.

✅ Focused coverage — Added tests exercise migration rollback, report ordering, streak identity, notification persistence, and opt-out races.

This is a static review; no commands were run. No current-head checks were reported. The supplied issue excerpt is truncated, so requirements beyond its visible portion could not be assessed.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F10: 🔴 Notify the actual pause outcome

• Required behavior: Notifications describing the newly introduced automatic pause must accurately represent whether the schedule was disabled.

• Evidence: src/jobs/agentRuns/unproductiveStreak.ts: evaluateUnproductiveStreak, streakBody, and OUTCOME_AFTER_NEW_ITEM.

  1. An opted-in automation reaches the threshold. Evaluation reads pauseWhenUnproductive: true.
  2. await createEvent(...) persists and announces a title saying “paused,” a body saying “Its schedule has been paused,” and schedulePaused: true, before attempting the pause.
  3. During notification announcement, the owner disables the pause option, or an outstanding scheduled run completes productively. Both interleavings are explicitly exercised in test/agentRunUnproductiveStreakRecovery.test.ts.
  4. carryOutPause correctly returns opted_out or streak_ended, leaving the schedule enabled. Evaluation returns notified, but never corrects the persisted notification.

  The owner is therefore told that future scheduled execution has stopped even though it remains enabled. The transactional rechecks protect the schedule, but they do not protect the notification’s accuracy. Notification deduplication retains the original message; the stored-item branch also contains no correction when pausing is no longer applicable.

  static trace: verified against the notification-before-pause ordering, the outcome maps, the supplied notification persistence implementation, and the existing race-test scenarios. Those tests check that the schedule remains enabled but do not assert that the resulting notification is truthful.

• Minimum fix: Persist an accurate initial notification describing the unproductive streak and intended pause, keeping recovery intent separate from confirmed pause status. Claim that the schedule has been paused only after a successful pause outcome; refusal or failure must leave truthful notification text.

Suggestions

These are optional follow-ups and are not sent to /fix.

S3: 🟢 Exercise overlapping report storage

reportNoveltyPatch reads the previous report separately from the transition that stores the new report. A focused concurrency test would establish whether callers serialize reports sufficiently to preserve “previous reported run” semantics. The supplied context does not establish whether simultaneous report storage is reachable through production admission paths, so this is a verification follow-up rather than a demonstrated blocker.

Score

The implementation has broad integration coverage and thoughtful recovery tests, but the verified notification inconsistency needs correction before merge. Runtime validation remains unperformed as requested.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 11s
• Tokens: 103,105 (101,717 in / 1,388 out)
• Cost: $0.96

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F10 S3

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit b81c76c

Addressed 1 AI review comment (IDs: 6811949698)

Addressed finding F10

Summary of Changes

I fixed F10. An Inbox item now says a schedule was paused only after the pause has actually turned the schedule off. Typecheck passes for root, core and API, zero-warning lint passes on the two changed source files, and the six related test suites pass (213 tests, 0 failures). I didn't run the full test suite.

Why the first item can't be corrected later. Inbox items are immutable: a database trigger rejects any update. The streak's item also has to be saved before the pause is tried, so that a failed pause can be retried by the next run. So the only way to keep it truthful is to word it so it stays true whatever the pause does.

What changed (src/jobs/agentRuns/unproductiveStreak.ts):

• First item: the title is now "Automation X keeps producing no change". The body says the automation is set to pause its schedule, "so the schedule will be turned off unless pausing is turned off or a newer scheduled run produces something first." That stays true whether the pause succeeds, is refused, or fails.
• Stored flag: this item's metadata now records only pauseIntended, which a later run uses to finish a failed pause. It no longer carries schedulePaused, so nothing stored claims an outcome before it happens. The core reader in agentRunProductivity.ts reads the new field.
• Second item: only the evaluation that actually turned the schedule off saves an item titled "Automation X paused: no change in N runs", with schedulePaused: true. It has its own key and goes to the Inbox without a push, because the first item already pushed. No paused item is sent when:
  • pausing was turned off first.
  • a newer scheduled run produced something first.
  • the schedule was already off.
  • another evaluation made the pause.
• If the paused item fails to save: the schedule stays paused, a warning is logged, and the item isn't retried. A late one could arrive after the owner had turned the schedule back on, and the first item is still true without it.
• Docs: docs/docs/features/agents.md now describes the two items.

One trade-off to decide on: the issue asks for one Inbox notification per streak. Automations that opt into pausing now get a second, push-free item when the pause happens. If you'd rather keep exactly one item, the alternative is to drop the paused item and leave the automation page as the only place showing the schedule is off.

Tests:

• The two race tests the finding cites now also check the saved text: no "paused" claim and no paused item. Pausing turned off during delivery is one; a newer run finishing productively during delivery is the other.
• The failure-and-recovery tests check that the paused item appears only once the next run actually makes the pause, and that the owner gets a push for the first item but not the second.
• New tests cover: a schedule the owner turned off during delivery, a paused item that fails to save, and two evaluations racing to pause.
• With the old code put back, 12 tests fail, and both race tests fail on the "paused" title. With the fix restored they pass.

Separately (not changed): the comment on unproductivePauseReason says the reason is "repeated in the Inbox item", but it only goes to the log.

I didn't make a visual preview: the Inbox screen itself is unchanged, only the wording the server stores.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (xhigh)
• Requested By: @propr-ultrafix
• Time: 19m 4s
• Tokens: 5,340,610 (5,287,880 in / 52,730 out)
• Cost: $2.81

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, client-visible badges, and scheduled-run notifications with optional pausing. It needs corrections before merge.

✅ Consistent presentation — REST, MCP, CLI, and UI expose the productivity flags, with a shared rule for deriving “no change.”

✅ Pause recovery — The pause transaction rechecks the opt-in and streak, and records completed pauses so re-enabling a schedule is respected.

✅ Focused coverage — Tests cover migration compatibility, notification persistence failures, and competing pause evaluations.

The supplied issue specification is truncated; this review relies on its visible requirements and the changed implementation. Current checks show 33 passed, four pending, and no failures. Verification below is static; no commands were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F11: 🔴 Concurrent reports can both be classified as new

• Required behavior: A stored report must be classified against the previous reported run of its definition.

• Evidence: src/jobs/processAgentRunJob.ts:538, src/jobs/agentRuns/reportNovelty.ts:reportNoveltyPatch

  1. Two runs of the same definition are executing concurrently; the latest stored report contains A.
  2. Both runs produce identical report B. Each awaits reportNoveltyPatch, which reads A and classifies B as new.
  3. The first run stores B through transitionRun.
  4. The second run then stores B with its already-computed new classification, although its previous reported run now contains B.

  The second run incorrectly appears productive and can reset the unproductive streak instead of extending it. The per-run transition guard protects each run’s state, but does not serialize comparison and storage across the definition. The added out-of-order reporting tests establish that multiple runs can exist, but exercise their report writes sequentially.

  static trace: The history lookup and report transition are separate awaited operations with no shared transaction or definition-level serialization.

• Minimum fix: Serialize comparison and report persistence per definition, or transactionally validate the predecessor before committing the report and its flags. Add a regression with two concurrent identical reports.

F12: 🔴 Failed acting steps lose performed mutations

• Required behavior: Acting runs must retain the number of mutating MCP operations performed under their grant; a run that performed mutations must not be counted as producing nothing solely because its report was unchanged.

• Evidence: src/jobs/processAgentActionJob.ts:302, packages/core/src/services/agents/agentRunProductivity.ts:agentRunProductivity, test/processAgentActionJob.test.ts — “a failed acting step stores no count”

  1. A definition has two unproductive scheduled runs.
  2. Its next scheduled acting step successfully performs a mutating MCP operation, then its execution fails. Its report was classified as unchanged.
  3. The failure path revokes the grant but discards its mutation count; the added test explicitly verifies that failed acting steps store no count.
  4. A subsequent unproductive scheduled run evaluates the streak. agentRunProductivity classifies the failed run as unproductive because its count is null, its report is unchanged, and failed is outside UNDECIDED_STATES.
  5. The owner receives an unproductive-streak notification, and an opted-in schedule is paused, despite the intervening run having performed a mutation that should reset the streak.

  Revocation cleanup does not persist the returned count. The streak’s transaction faithfully rechecks the same incomplete flags, so it cannot prevent this incorrect pause.

  static trace: Count persistence occurs only after successful execution and action-summary extraction. The failure-path test confirms the omission; the shared productivity rule demonstrates its downstream consequence.

• Minimum fix: Capture and persist available grant mutation counts on failed acting steps as well as successful ones, preserving the existing terminal state. Cover a successful mutation followed by execution failure and verify that it breaks the streak.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation has substantial coverage and careful pause recovery, but concurrent classification and failure-path accounting can produce incorrect streak decisions. These localized correctness defects should be fixed before merge.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 21s
• Tokens: 107,739 (106,277 in / 1,462 out)
• Cost: $1.03

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F11 F12

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 5129966

Addressed 1 AI review comment (IDs: 6812510895)

Addressed findings F11, F12

Summary of Changes

I fixed F11 and F12, and each fix has regression tests. Root, core and API typecheck pass, and zero-warning lint passes on the changed files. The 9 related root test files pass (226 tests, 0 failures). I didn't run the full suite, and nothing ran against Postgres. To check the new tests, I put back the old logic for each finding: they fail, and pass again with the fix.

F11: two runs storing the same report at once

• The worker still compares the report with the latest stored one. The store now commits the report and its flags only if that latest report hasn't changed since the comparison. This step is serialized per definition: on Postgres it locks the definition row, and on SQLite the transaction already holds the write lock. The new storeAgentRunReport is in agentRunReports.ts.
• If another run stored a report in between, the worker compares again. After 3 failed attempts it stores the report with no flags rather than a classification that may be out of date. If the run is no longer running, the report is discarded as before.
• The new test runs two identical reports concurrently against real SQLite. The old logic gives new, new; the fix gives new, unchanged.

F12: acting steps that end without completing lose their count

• The acting step's grant now ends as soon as the agent finishes, before the run's final state is written. The count is saved with that state whether the run completes, fails or is cancelled from the Tasks UI. The run's state itself doesn't change.
• I found the same problem in one more place: a run cancelled through the API while its agent is running. The count now gets written onto the cancelled run. That write only touches a failed or cancelled run claimed by this acting step, and never replaces a count already stored.
• An end-to-end test covers the finding's case. Two unchanged scheduled runs are followed by an acting step that makes a change and then crashes, and then a third unchanged run. With the old code that third run paused the schedule. Now the failed run breaks the streak and the schedule stays on. The same holds when the run is cancelled elsewhere.
• I replaced the existing test "a failed acting step stores no count", because it asserted the bug.

Decision for you: a failed or cancelled acting step whose grant changed nothing now stores 0. It therefore shows the "no change" badge and extends the streak, even if its report was new. This matches the acceptance criterion ("zero is recorded as a no-op acting step") and how completed runs already behave. If failures shouldn't count that way, store only counts above zero on that path.

Related gaps I left alone (not in scope):

• If the API cancels an acting run and the scheduler's leftover-grant cleanup revokes the grant before the worker does, the worker gets no count and the run's count stays unknown. Closing this would mean having the cleanup record counts too.
• If the database write that fails the run itself errors, the run stays acting. The redelivery that then fails it has no count to store.

Nothing here is visible in the UI, so there are no previews.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (xhigh)
• Requested By: @propr-ultrafix
• Time: 22m 43s
• Tokens: 10,002,220 (9,923,184 in / 79,036 out)
• Cost: $4.56

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 72337ab

Addressed 1 AI review comment (IDs: 6823495579)

Addressed finding F20

Summary of Changes

I fixed F20. A cancelled run whose acting step made changes no longer gets stuck with its count marked as pending when the database write for that count fails. The run stays cancelled, the delivery fails so BullMQ retries it, and the retry saves the count. The next unchanged scheduled run then sees the productive run and does not count it toward a streak.

The fix:

• Failed write (keepMutationsOfEndedRun, src/jobs/processAgentActionJob.ts): it used to swallow the error. Now the task is still closed to match the cancelled run, but the delivery then throws a new AgentRunMutationsSettlementError instead of returning normally.
• Keeping the count: the revoked grant can't report the count a second time, so it is saved in the job's own data (job.updateData, new optional actingMutationsOwed field on AgentRunJobData). This happens whenever a delivery that claimed the step is about to fail, after the grant has ended.
• Retry: a retry of the same job reads that field, takes over the claim and saves the count. A separate delivery for the same run doesn't have the field, so it still can't settle the count.

The same defect in a second place: if the claiming delivery's final write (the run's completed or failed state) failed after its grant ended, the retry marked the run as failed with no count. That threw away a known count, and the run's unchanged report then made it look unproductive. The saved job data now covers this case too, and the retry stores the count.

Tests:

• New regressions: three in test/processAgentActionJob.test.ts. One covers a failed write, then the retry settling the count. One checks that a separate delivery without the saved count does not settle it. One covers the failed final write. One in test/agentRunActingMutationsStreak.test.ts runs against the real SQLite store: two unchanged runs, a third cancelled during its step that made one change, a failed count write, the retry, then another unchanged scheduled run. The run is marked productive, the streak holds only the new run, no Inbox item is sent and the schedule stays on.
• Checking the tests: with the old processor code put back, all four new tests fail; with the fix they pass.
• Results: the 8 related root test files pass (221 tests, 0 failures). Root and core typecheck and zero-warning lint on the changed files pass. I didn't run the full suite, and nothing ran against Postgres.

What this doesn't cover:

• Retry limit: agent run jobs get 3 attempts with exponential backoff (5s, then 10s). If the count write fails on every attempt, the run stays pending, though the count is still in the failed job's data, so manually retrying that job would settle it. If saving to the job's data fails too (Redis unavailable), the count is lost and only an error is logged.

Separate issue, not fixed: while a run's count is pending, the streak query skips it rather than letting it end the streak. If another unchanged scheduled run finishes during the retry backoff, the streak can still be counted across the productive run and reach the threshold. This comes from the earlier F17 design, not from F20, so I left it alone.

There's no visual change, so I didn't make previews.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (high)
• Requested By: @propr-ultrafix
• Time: 13m 7s
• Tokens: 2,516,100 (2,490,464 in / 25,636 out)
• Cost: $1.53
• Subscription usage: Weekly +2%, Fable +4%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, client badges, and streak notifications with optional schedule pausing. It needs a localized recovery fix before merge.

✅ Consistent presentation — REST, MCP, CLI, and UI expose the productivity flags, with a shared rule for deriving “no change.”

✅ Ordered report comparison — Report storage checks the predecessor under a definition lock and maintains strictly increasing report timestamps.

✅ Substantial regression coverage — Tests cover cancellation, mutation-count persistence, concurrent report storage, notification deduplication, and pause recovery.

This review is a static trace of the supplied code; no commands were run. The issue specification is explicitly truncated, limiting validation of its unseen requirements. Current checks show 33 passed, four pending, and no failures.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F21: 🔴 Retry skips the streak evaluation

• Required behavior: A finalized unproductive scheduled run must trigger the threshold notification and optional pause; a recoverable task-store failure must not permanently skip that evaluation.

• Evidence: src/jobs/processAgentActionJob.ts, runAction, failRunAndTask, taskFollowsRun, and keepMutationsOfEndedRun.

  1. The third unproductive scheduled acting run revokes its grant and successfully stores completed, actingMutations: 0, and actingMutationsPending: false.
  2. The subsequent awaited settleTask(...markTaskCompleted...) fails. It throws AgentRunSettlementError before evaluateStreak(completed) executes.
  3. The outer handler preserves the count on the job, and the retry reconciles the terminal run through taskFollowsRun.
  4. keepMutationsOfEndedRun calls recordMutations, but recordAgentRunActingMutations only updates failed or cancelled runs. It therefore returns null for this completed run, and evaluateEndedRun(recorded) does nothing.
  5. Task reconciliation succeeds and the retry returns without evaluating the completed run. The threshold notification and schedule pause remain missing until another qualifying run happens.

  The failed-run sibling has the same gap: failRunAndTask stores a known count before settling the task, but the retry's mutation recorder refuses to overwrite that count and therefore never evaluates it. Preserving actingMutationsOwed does not recover the skipped evaluation. static trace: the changed processor orders evaluation after task settlement, while its retry path evaluates only a newly updated mutation record. The supplied recovery tests cover failed count writes, not this successful-count-write/failed-task-write sequence.

• Minimum fix: After successful terminal task reconciliation, evaluate the current run when its productivity fields are settled, including when recording mutations returned null because the count was already stored. Preserve the pending-count guard and existing notification deduplication. Add regression coverage for both completed and failed runs whose task settlement initially fails.

Suggestions

These are optional follow-ups and are not sent to /fix.

S7: 🟢 Align grant-counting documentation

The new documentation says operations are counted before revocation, while agentRunInternalRoutes.ts explicitly counts after revocation. Align the wording with the implementation so maintainers understand the intended ordering. This is a documentation clarification, not a separate behavioral blocker.

Score

The implementation has broad coverage and careful persistence handling, but terminal-task recovery can bypass the notification and pause behavior. The remaining pending checks are not themselves blockers.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 58s
• Tokens: 129,115 (127,841 in / 1,274 out)
• Cost: $1.23

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F21 S7

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 58a883d

Addressed 1 AI review comment (IDs: 6824152351)

Addressed finding F21

Summary of Changes

I fixed F21. If a task write fails after a run's end has been stored, the retry now runs the streak check, so the owner still gets the Inbox notification and an opted-in schedule is still paused. The fix covers the completed and failed acting runs from the finding, plus the same gap in the report step.

Action step (src/jobs/processAgentActionJob.ts)

• Once taskFollowsRun has brought the task in line with the run, it now checks the run's streak if this delivery recorded no count but the run is already final. "Final" means completed, failed or cancelled, with actingMutationsPending false.
• That covers the case from the finding, where the retry records nothing new because a completed run takes no count and a stored count is never replaced.
• It also covers a retry that couldn't keep the count with its job, so it doesn't own the claim.
• The pending-count guard still holds. A run whose count is still owed is skipped; the delivery that holds the count checks it once it saves it. If saving the count fails, the delivery still throws before any check.
• The existing check that runs straight after a count is recorded stays where it was, so a later failed task write doesn't delay it.

Report step (src/jobs/processAgentRunJob.ts), same defect

• A run can fail right after its report is stored, for example when the acting step can't be queued. If the task write then failed, the retry went through reconcileEndedRun, which ended the task but never checked the run.
• It now checks the run once the task has been ended.
• The file was already at the 400-line lint limit, so I moved that function's existing return earlier to stay under it.

A repeated check is safe: the stored notification is looked up first, and a pause already made is not made again.

Tests: I added 8 tests across processAgentActionJob, agentRunActingMutationsStreak and processAgentRunJob:

• Completed and failed runs whose task write fails once: the first attempt fails and checks nothing; the retry checks the run after its task is settled.
• Retry with no saved count: it still checks the run.
• Run whose count is still owed: it is not checked.
• Count recorded before the task write failed: it is checked straight away.
• Real SQLite store and real streak check: the third unproductive completed or failed run notifies the owner and pauses the schedule on the retry, and a further delivery doesn't notify again.
• Report step: the failed-after-report retry is checked.

Test setup changes:

• The action test setup now marks a run's count as owed when the step claims it, as the real store does.
• One existing test comment said an "already stored" count needs no check. That was the wrong assumption, so I corrected it.
• In the SQLite test, the record of sent notifications used to start empty on every check, so a repeated check could look like a duplicate notification. It is now kept per inbox.

Verification:

• With the old source files put back, 7 of the new tests fail; the one that checks the pending guard passes on both versions, as it should.
• With the fix, these nine suites pass, 250 tests and 0 failures: action 54, report 79, mutation streak 12, recovery 27, cancel streak 4, productivity 21, novelty 14 + 13, autonomy 26.
• Root typecheck and zero-warning lint on src/ pass.
• I typechecked the edited test files separately because the root typecheck skips test/: they add no new type errors (21 are already there at HEAD).
• I didn't run the full suite or anything against Postgres. There's no visual change, so no preview.

Separate finding, not fixed: in approveRun (packages/api/routes/automationDefinitionRoutes.ts), if queuing the acting step fails, enqueueAgentRunActionOrFail fails the run and nothing checks its streak. An approved run with an unchanged report that ends up failed this way only counts when a later scheduled run is checked. No task write is involved, so it's outside F21.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (xhigh)
• Requested By: @propr-ultrafix
• Time: 22m 58s
• Tokens: 6,363,530 (6,307,862 in / 55,668 out)
• Cost: $3.21

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, notifications, and optional schedule pausing across the backend and clients. It needs a localized correctness fix before merge.

✅ Durable settlement — Acting counts survive task-write retries through actingMutationsOwed, with regression coverage for failed and cancelled runs.

✅ Consistent presentation — REST, MCP, CLI, and UI expose productivity information without changing completion states.

✅ Pause recovery — Persisted notification intent and pause records support retrying interrupted pauses while respecting subsequent owner changes.

The issue specification is explicitly truncated; this review assesses its visible requirements and the changed behavior. Current checks show no failures and two pending checks. No commands were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F22: 🔴 Normalize whitespace before matching timestamps

• Required behavior: Reports differing only in whitespace and dates must normalize identically and be classified as unchanged.

• Evidence: packages/core/src/services/agents/agentRunNovelty.ts, VOLATILE_PATTERNS and normalizeAgentReport.

  1. A scheduled run stores a sufficiently long report containing Checked 2026-11-11 22:00 UTC. Dependency watch: lodash remains outdated in acme/web.
  2. The next run produces the same report with the next date and a line break between the date and time: Checked 2026-11-12\n22:00 UTC. Dependency watch: lodash remains outdated in acme/web.
  3. The ISO pattern accepts a literal space between date and time, so the first report becomes Checked <date>. .... It does not accept a newline, so the second becomes Checked <date> <time>. ....
  4. Whitespace collapsing happens afterward and cannot reconcile these different placeholders. The hashes differ, so the repeated report is classified as new. Alternating these layouts repeatedly resets the unproductive streak and prevents the intended notification.

  static trace: The same failure occurs with two spaces instead of a newline. Rehashing the predecessor and locking report storage preserve this erroneous comparison; existing whitespace tests do not combine wrapping with timestamp normalization.

• Minimum fix: Collapse whitespace before applying volatile timestamp patterns, or make timestamp matching invariant to equivalent whitespace. Add regression coverage comparing single-space, multiple-space, and newline-separated timestamps.

Suggestions

These are optional follow-ups and are not sent to /fix.

S8: 🟢 Reduce streak query payloads

findUnproductiveScheduledStreak selects complete run rows for up to 1,000 runs, including reports and definition snapshots. Consider selecting only the fields needed for classification and notification construction. This is an optional efficiency improvement; the supplied evidence does not establish a production performance failure.

Score

The implementation has substantial recovery coverage and consistent client integration, but timestamp normalization still violates a central acceptance criterion. The remaining checks are pending, with no reported failures.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 7s
• Tokens: 134,182 (132,880 in / 1,302 out)
• Cost: $1.28

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F22 S8

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 8b7c483

Addressed 1 AI review comment (IDs: 6825121468)

Addressed finding F22

Summary of Changes

I fixed F22. A report whose only differences from the previous one are its date and the whitespace inside the timestamp is now classified as unchanged.

Fix (packages/core/src/services/agents/agentRunNovelty.ts): normalizeAgentReport now collapses whitespace to single spaces and trims before it replaces run ids and applies the timestamp patterns. Before, it did that last. The ISO pattern's literal-space date/time separator now matches no matter how the timestamp was wrapped. Both reports in the finding now normalise to Checked <date>. Dependency watch: ….

Same defect elsewhere in the patterns: the time-zone suffix (22:00 UTC), the clock-time am/pm and zone suffixes, and the named-month dates (Nov\n12,\n2026) had the same problem. The reordering fixes all of these. I also collapse whitespace in the run ids before matching, the same way as the report text, so run-id replacement works as before.

Already-stored reports: no migration is needed. The previous report is re-hashed at every comparison rather than reusing its stored hash, so older runs are compared under the new normalisation automatically. No test pins a literal hash.

Regression tests (test/agentRunNovelty.test.ts):

• Normaliser test: single-space, multiple-space, newline, CRLF and tab layouts of ISO timestamps (with UTC and +01:00 suffixes), clock times with pm UTC, and named-month dates all normalise to the same string as their single-spaced form.
• Classifier test: this is the F22 scenario at the default 40-character minimum. The next night's report with a newline, two spaces or one space before the time is unchanged, and its hash equals the previous report's.

Verification:

• Both new tests fail when I put back the old normaliser and pass with the fix.
• The novelty, previous-report and process-agent-run/action suites pass: 162 tests, 0 failures. They need PROPR_DEMO_MODE=true; without it they stop at the known GitHub-auth banner.
• Core tsc --noEmit and zero-warning ESLint on the changed source file pass.
• I didn't run the full suite.

The change has no visible effect in the UI, so I made no previews. I found nothing else to report.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (xhigh)
• Requested By: @propr-ultrafix
• Time: 6m 9s
• Tokens: 907,919 (899,561 in / 8,358 out)
• Cost: $0.59
• Subscription usage: Weekly +1%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, consistent client flags, and notifications with optional schedule pausing. It needs changes before merge because a worker crash can leave an ended run’s productivity permanently unsettled.

✅ Consistent presentation — REST, MCP, CLI, and UI expose productivity flags, with shared logic for the no-change reason.

✅ Careful report ordering — Report storage checks the predecessor under the definition lock and assigns increasing report timestamps.

✅ Recovery coverage — Added tests cover cancellation, failed persistence, notification deduplication, and interrupted schedule pausing.

The supplied checks show 37 passed, none failed, and one pending. The issue specification is explicitly truncated; this review assesses its visible requirements and the changed behavior. No commands were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F23: 🔴 Recover pending counts after worker crashes

• Required behavior: An ended acting run must eventually settle its productivity flags; a worker crash must not permanently exclude a potentially productive run from streak calculation.

• Evidence: src/jobs/processAgentActionJob.ts, actionContext, keepMutationsOfEndedRun, and taskFollowsRun; packages/core/src/services/agents/agentRunStore.ts, claimAgentRunAction; packages/core/src/services/agents/agentRunProductivity.ts, decided.

  1. An acting delivery claims its run, persisting acting_mutations_pending = true, and performs a mutating MCP operation.
  2. The owner cancels the run during execution. The cancellation preserves the pending marker so the executing worker can later settle its count.
  3. That worker process terminates before executing endGrant or the outer catch that writes actingMutationsOwed to the job.
  4. On redelivery, actionContext sees no saved obligation and leaves ownsClaim unset. Because the run is already cancelled, reconciliation reaches taskFollowsRun; keepMutationsOfEndedRun refuses to settle it, and withFinalFlags refuses to evaluate it.
  5. Subsequent deliveries repeat this behavior. The cancelled run remains permanently pending, and decided excludes it from every streak query. Two earlier unproductive runs and a later unproductive run can therefore trigger notification and schedule pausing across this run despite its mutation.

  static trace: The claim is durable, but recovery ownership is saved only by a catch handler that cannot execute after process termination. The supplied cancellation tests rely on the original worker surviving to settle the count; persistence-retry tests rely on updateData having saved the obligation. Neither covers this interruption. No crash regression was executed.

• Minimum fix: Add recovery for ended runs with an outstanding acting count but no saved job obligation. Once the original execution is confirmed stopped, recover its grant’s count where available, or settle it as unknown after ending the grant, then evaluate the run. Preserve the protection against a concurrent delivery prematurely settling a still-active worker’s count.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation has substantial coverage and strong handling of ordinary retries, but abrupt worker termination leaves a durable recovery gap that can affect automatic schedule pausing. Current checks show no failures.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 8s
• Tokens: 134,015 (132,770 in / 1,245 out)
• Cost: $1.28

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F23

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit ef3306d

Addressed 1 AI review comment (IDs: 6825910458)

Addressed finding F23

Summary of Changes

F23 is fixed in the acting-step job, with regressions. No visual preview applies, since the change is worker-side settlement behavior with no UI effect.

What was wrong. A delivery only took ownership of a claimed step's count from the obligation its own catch handler saved to the job. After a worker crash that handler never ran, so a redelivery for a run cancelled or failed while the lost worker held the grant reached taskFollowsRun without ownership. Nothing settled the count, and the run stayed pending and excluded from every streak query.

The correction in src/jobs/processAgentActionJob.ts:

• New claim takeover. adoptAbandonedClaim revokes whatever action grant the run's phase still holds, by phase rather than grant id. It takes the count the API reports from that revoke, settles unknown when the grant is already gone or the revoke fails, and marks the context as owing the settlement. Ending the grant also stops any container the lost worker left behind before the count is fixed.
• Ended-run path. When the run already ended with its outstanding marker set and no delivery in this process is executing the step, the redelivery adopts the claim before taskFollowsRun, which then records the count and evaluates the run.
• Sibling occurrence. recoverClaimedRun, which fails a still-acting run left behind by a stopped worker, had the same gap: it cleared the pending marker without ending the grant or recovering its count. It now adopts the claim first, so the failed run carries the recovered count, and a cancel racing its write still lands the count through the existing settlement path.

Protection preserved. A duplicate delivery while the step executes in this process is still left alone, since the active-run check gates adoption. Across processes, a redelivery only happens after the executing worker lost its lock. If that worker is in fact still alive, its revoke by grant id finds the grant gone and reports no count, and the store never writes a count over one already stored, so the adopter's count, taken at the moment the grant died, stands.

Tests in test/processAgentActionJob.test.ts:

• Redelivery for a cancelled and for a failed run with a crashed claimant settles the recovered count, evaluates the run once, and never re-runs the step.
• Grant already gone, or the revoke failing, settles the run with an unknown count.
• A recovered count whose write fails is kept with the job, and the retry settles it without ending a grant again.
• The abandoned still-acting run is failed with its recovered count, including the owner-cancel race.
• The earlier test asserting a redelivery never settles a pending run was reworked to the real protection: a duplicate delivery while the step executes here leaves it to that step.
• The earlier F20 test was updated. Since action jobs are deduplicated per run, a delivery without the kept obligation is always a post-lock-loss redelivery. It now settles unknown, and the claimant's retry still stores the known count over that.

Verification. The action job suite passes at 59 tests, the run job and streak recovery suites pass at 106, the root typecheck is clean, and the source file passes zero-warning lint. The report step holds no outstanding-count obligation, so it has no equivalent gap.

One residual window, not pursued. If a worker only lost its lock but is still running, a takeover can settle an unknown count first and the claimant's retry then stores the known count, producing two evaluations. That is a race between BullMQ redelivery and a worker the queue considers dead, and the store's write ordering already prevents a known count being lost.

---

🤖 Implementation Details

• Model: Claude Fable 5.1 (medium)
• Requested By: @propr-ultrafix
• Time: 9m 27s
• Tokens: 1,718,358 (1,694,239 in / 24,119 out)
• Cost: $2.72

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR implements productivity flags across storage, workers, REST, MCP, CLI and UI, with substantial recovery coverage. It needs changes before merge because mutation counts can be lost during a worker crash, and the PostgreSQL pause transaction does not fully serialize against productive completions.

✅ Consistent presentation — Shared productivity rules drive the server’s noChangeReason and UI badges.

✅ Ordered report comparison — Report storage checks the predecessor under a definition lock and assigns strictly increasing report timestamps.

The issue excerpt is truncated; this review uses its visible requirements and the changed behavior. The supplied checks show 35 passed, none failed and two pending. No commands were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F24: 🔴 Preserve mutation counts across revocation crashes

• Required behavior: Acting runs must retain the mutating operations performed under their grant; recovery must not classify a run that changed something as unproductive merely because revocation already completed.

• Evidence: src/jobs/processAgentActionJob.ts, endGrant, adoptAbandonedClaim, and keepOwedMutationsWithJob.

  1. Two scheduled runs have unchanged reports. A third scheduled acting run also has an unchanged report but performs one mutation.
  2. Its worker awaits endGrant(). Revocation deletes the phase’s grant record, and the returned count is held only in context.actingMutations.
  3. The worker process terminates before the subsequent run transition. A process termination does not execute the outer catch that saves actingMutationsOwed.
  4. Redelivery finds the existing action claim and calls adoptAbandonedClaim. Revocation now returns no grant, so the recovered count becomes null.
  5. Recovery fails the run with actingMutationsPending: false. Its unchanged report now completes the unproductive streak, potentially notifying the owner and disabling the schedule despite the mutation.

  The grant receipt remains insufficient for this recovery path because the deleted phase record supplied its identity. The job obligation is saved only after a caught error, leaving this crash window uncovered. static trace: the changed processor and supplied revokeAgentRunPhaseGrant implementation establish this sequence; it was not executed.

• Minimum fix: Persist the count or a recoverable grant identity durably before deleting the phase record. Make redelivery recover that evidence before clearing the pending flag or evaluating productivity.

F25: 🔴 Serialize pausing against productive completions

• Required behavior: A productive scheduled run that commits before the schedule change must prevent pausing an obsolete unproductive streak.

• Evidence: packages/core/src/services/agents/agentRunProductivity.ts, pauseScheduleForUnproductiveStreak and recordAgentRunActingMutations.

  1. On PostgreSQL, an opted-in definition has enough unchanged runs to reach the threshold, with an intervening acting run still pending.
  2. The pause transaction updates the definition’s pause marker and awaits findUnproductiveScheduledStreak, which skips the pending run and confirms the old streak.
  3. After that read, while the transaction awaits its subsequent updates, the acting worker commits a positive mutation count through transitionAgentRun, or through recordAgentRunActingMutations for a cancelled run.
  4. The pause transaction then disables the schedule using its stale streak result, although the productive run has already ended that streak.

  The definition-row lock does not block updates to existing agent_runs rows; neither count-settlement path acquires that lock. SQLite’s database-wide write serialization therefore does not establish the claimed PostgreSQL protection. static trace: the changed pause transaction and both mutation-settlement paths show the missing shared serialization; no database test was executed.

• Minimum fix: Use shared serialization between the pause transaction and writes that finalize productivity, covering both normal terminal transitions and late mutation settlement. Add a PostgreSQL interleaving regression test.

Suggestions

These are optional follow-ups and are not sent to /fix.

S9: 🟢 Read lighter streak records

findUnproductiveScheduledStreak selects complete run rows for up to 1,000 runs, including reports and definition snapshots. Selecting only productivity, ordering and notification fields would reduce memory and database traffic. This is an optional performance improvement.

Score

The implementation has strong surface coverage and extensive tests, but two recovery and concurrency defects can incorrectly pause productive automations. Pending checks alone do not lower the score.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 35s
• Tokens: 138,523 (136,589 in / 1,934 out)
• Cost: $1.33

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F24 F25 S9

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit c6fcadc

Addressed 1 AI review comment (IDs: 6826674087)

Addressed findings F24, F25

Summary of Changes

Both findings are fixed, each with regressions that fail on the old logic and pass on the new one. No visual preview applies: both changes are server-side settlement and locking behaviour with no UI effect.

F24: the mutation count survives a crash after revocation

• The API now remembers which grant a phase ended. Revocation in packages/api/mcp/agentRunGrants.ts writes an ended-grant record in the same transaction that deletes the phase record, so the grant's identity never has a gap. The record is purged once the grant TTL has passed since the revoke.
• A revoke that finds no live grant now answers with the grant the phase ended before, with revoked: false and the count of that grant's operations. A cleanup fenced by expiry is never answered with it, and a stale grant id names nothing. The sweep only reads the live kind, so ended grants are not revoked again every pass.
• The worker's claim adoption in src/jobs/processAgentActionJob.ts takes the count whenever the API names a grant, ended now or before. The crash window itself is now a unit test: the first delivery stops right after its revoke, and a second processor instance recovers the count of 1 and fails the run as productive instead of as the third unchanged run.

F25: pausing is serialized with productivity-final writes

• A new module, packages/core/src/services/agents/agentRunDefinitionLock.ts, locks the run's definition row through a subquery on the run id. Every terminal transition, every transition carrying an acting count, and the late settlement in recordAgentRunActingMutations now take that lock first, in a transaction. The pause transaction already holds the same row lock from its claim update, so a productive write either commits before the streak read or waits until the pause commits.
• Lock order is definition then run everywhere, matching the existing report store, so no deadlock cycle is introduced.
• The interleaving test runs against a real PostgreSQL 15 that I extracted from Debian packages into /tmp. It injects a competing completion after the pause's streak read: the competitor hits a lock timeout, the pause commits, and the write lands afterwards. The reverse order yields streak_ended. The same holds for a late count settlement on a cancelled run. The test skips unless PROPR_TEST_POSTGRES_URL is set, because the repository ships only better-sqlite3 and its migration chain is SQLite-only, so the Postgres tables are built from the SQLite schema. An always-on SQLite test asserts the lock statement and transaction boundaries instead.

Verification

Check	Result
Root, core, API typecheck	pass
Zero-warning lint on changed source files	pass (core store needed the helpers split out to stay under 400 lines)
9 related root suites plus the new locking suite	244 tests, 0 failures
API grant, MCP run and definition-route suites	60 tests, 0 failures
Postgres interleaving suite	4 tests, 0 failures
Negative checks	all 4 locking tests and the F24 worker test fail with the fix reverted

Setup change. The pg driver is added as an agent-only helper in .propr/package.json, and .propr/setup.sh installs it. I ran only that new install step, not the whole setup script. A PostgreSQL server is a Debian system package, so it belongs in the agent image at the installation level. Without both, the Postgres suite skips, as it does in CI today.

Independent observations, not acted on. The pre-existing API test asserted that an already-gone grant reports no count. That assertion was the F24 gap itself, so I updated it. The store's header comment says it is the only module writing agent_runs, which the earlier recordAgentRunActingMutations already made untrue.

---

🤖 Implementation Details

• Model: Claude Fable 5.1 (medium)
• Requested By: @propr-ultrafix
• Time: 31m 46s
• Tokens: 7,567,022 (7,487,496 in / 79,526 out)
• Cost: $8.22
• Subscription usage: Weekly +2%, Fable +2%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR implements productivity flags across storage, REST, MCP, CLI and UI, with substantial recovery and concurrency coverage. It needs changes before merge because mutation accounting can still lose evidence and incorrectly pause productive automations.

✅ Consistent presentation — REST and MCP expose a shared no-change classification, which the UI also uses.

✅ Serialized report storage — The predecessor check under the definition lock prevents concurrent identical reports from both being classified as new.

✅ Recoverable schedule pauses — Notification persistence and pause completion have separate durable records, preserving retries and respecting schedules subsequently re-enabled by owners.

The supplied issue specification is truncated; this review assesses its visible criteria and the changed behavior. No commands were run. Reported checks show 37 passed, none failed, one pending and 11 neutral/skipped.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F26: 🔴 Revocation does not finalize mutation admission

• Required behavior: The stored acting mutation count must capture operations performed under the run’s grant; a productive acting step must not be recorded as a no-op.

• Evidence: packages/api/routes/agentRunInternalRoutes.ts, revoke handler and countGrantMutatingOperations; src/jobs/processAgentActionJob.ts, adoptAbandonedClaim.

  1. An acting container submits a mutating MCP request. Authentication succeeds, but the API request is still awaiting repository authorization before receipt insertion.
  2. The worker process terminates. A subsequent delivery enters adoptAbandonedClaim while that independent API request remains outstanding.
  3. Revocation updates the grant’s revoked flag. The new count query finds no operation receipt yet and returns zero.
  4. Recovery stores zero, fails the abandoned run and evaluates its streak.
  5. The previously authenticated request finishes authorization, inserts its receipt and performs its mutation. The run permanently retains zero and can complete a streak that pauses the automation.

  The supplied toolExecution.ts:141–176 performs awaited authorization before calling McpOperations.run, whose receipt insertion occurs at operations.ts:143–145. The supplied revocation implementation only updates the grant record; it neither fences receipt admission nor waits for authenticated requests. Definition locks protect run writes, not this admission window.

  static trace: traced the supplied authorization, receipt insertion, revocation, counting and takeover paths. No concurrency test was executed.

• Minimum fix: Serialize mutation admission with grant revocation: recheck grant validity and register the operation under a shared transactional guard before invocation, so revocation either observes the admitted receipt or prevents the operation from executing.

F27: 🔴 Recovery evidence expires before settlement

• Required behavior: An acting step that performed mutations must retain that evidence through the crash recovery introduced by this PR.

• Evidence: packages/api/mcp/agentRunGrants.ts, purgeEndedGrantRecords and revokeAgentRunPhaseGrant; src/jobs/processAgentActionJob.ts, adoptAbandonedClaim.

  1. After two unproductive scheduled runs, a third run performs a mutation and successfully revokes its grant.
  2. Its worker terminates before storing the returned count or updating the job. The ended-grant record is now the recovery path’s only grant identifier.
  3. Recovery is delayed beyond two hours. Revocation of another phase executes purgeEndedGrantRecords, deleting that identifier solely because its TTL expired.
  4. The original job is redelivered. Its phase has neither a live grant nor an ended-grant record, so adoption obtains an unknown count.
  5. Recovery fails the abandoned run and clears its pending-count flag. Its unchanged report now classifies it as unproductive, allowing notification and schedule pausing despite its actual mutation.

  Purging never checks whether the run still owes its count. The operation receipts cannot help this recovery path after their grant identifier is discarded, and the job contains no saved count in this crash window.

  static trace: followed the explicit TTL deletion and the unknown-count takeover path. Existing tests cover immediate post-revocation recovery and TTL deletion separately, not their combination.

• Minimum fix: Retain ended action-grant identifiers while their runs have outstanding mutation settlement, or durably settle the count before allowing those identifiers to be purged.

Suggestions

These are optional follow-ups and are not sent to /fix.

S10: 🟢 Exercise PostgreSQL interleavings in CI

The PostgreSQL tests skip unless PROPR_TEST_POSTGRES_URL is supplied. A dedicated CI job would strengthen regression detection for the new row-locking behavior; the SQLite statement assertions cannot demonstrate PostgreSQL interleavings. This is optional validation infrastructure, separate from the defects above.

Score

Broad implementation and substantial tests are strengths, but two mutation-accounting recovery defects can produce incorrect flags and automatic schedule pauses. The pending check alone does not lower the score.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 2m 16s
• Tokens: 150,260 (147,627 in / 2,633 out)
• Cost: $1.50

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F26 F27 S10

_{🤖 Review by ProPR}

[propr-dev]

✅ Applied the requested follow-up changes in commit 9c73778

Addressed 1 AI review comment (IDs: 6828105466)

Addressed findings F26, F27

Summary of Changes

Nothing else is pending; everything left depends on that run finishing.

---

🤖 Implementation Details

• Model: Claude Fable 5.1 (high)
• Requested By: @propr-ultrafix
• Time: 16m 39s
• Tokens: 4,804,968 (4,803,634 in / 1,334 out)
• Cost: $3.27
• Subscription usage: Fable +1%

View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}

[propr-dev]

⚠️ Ultrafix stopped before reaching its goal. Requested goal: 8/10. Last score: unknown. Max cycles were exhausted, so manual review and merge are now required.

[integry]

ProPR runs only on SQLite (better-sqlite3); PostgreSQL is not a supported database and pg is not a dependency of any package. Please remove everything in this PR that exists only for PostgreSQL:

1. Agent setup. Revert .propr/setup.sh to its state on main and delete .propr/package.json and .propr/package-lock.json. The setup script must not gain an extra npm ci step.

2. PostgreSQL-only test. Remove the PostgreSQL: ... describe block and every PROPR_TEST_POSTGRES_URL / client: 'pg' reference from test/agentRunProductivityLocking.test.ts. Keep the SQLite tests in that file; if nothing else remains, delete the file.

3. Row locks that do nothing on SQLite. Remove the .forUpdate() calls this PR added (in the agent-run grant code, agentRunProductivity.ts, the unproductive-streak code and anywhere else in the diff), together with the lock-only queries whose result is unused and the comments that describe PostgreSQL behaviour. Keep the mechanism that actually provides the guarantee on SQLite: the transaction that takes the write lock up front (BEGIN IMMEDIATE). Comments should describe only that.

Constraints:

• Do not change behaviour on SQLite. The existing SQLite tests for report ordering, mutation-count settlement, streak evaluation and pause recovery must still pass unchanged, including the one that covers a productive write landing between the streak read and the pause commit.
• Do not add new functionality or new race handling in this change; it is a removal only.
• Make sure no documentation, changelog entry or comment in the diff still mentions PostgreSQL.

[propr-dev]

✅ Applied the requested follow-up changes in commit 7a4a7f5

Summary of Changes

I've removed everything in this PR that existed only for PostgreSQL. One .forUpdate() remains, and two SQLite tests had to change because they checked for the lock query itself; details below.

1. Agent setup

• .propr/setup.sh now matches main exactly, so there is no extra npm ci step.
• .propr/package.json and .propr/package-lock.json are deleted. I also deleted the ignored .propr/node_modules that held the pg packages.

2. PostgreSQL-only test

• In test/agentRunProductivityLocking.test.ts I removed the PostgreSQL: ... describe block, its proxy helper, PROPR_TEST_POSTGRES_URL, both client: 'pg' connections and the fixture helper and imports only that block used.
• Changed tests: the two SQLite tests that stay in that file asserted that the definition lock query ran before the write. That query is now gone, so they check instead that the write runs inside its own transaction (begin before it, commit after it). That transaction is what BEGIN IMMEDIATE applies to.

3. Row locks

• agentRunReports.ts: removed the agent_definitions … forUpdate() query, whose result was never used, from the report-storing transaction. The helper is renamed storeReportUnderWriteLock.
• agentRunDefinitionLock.ts: replaced by agentRunFinalTransaction.ts. The lock query is gone, and inTransactionIfFinal still wraps writes that can make a run's verdict final in a transaction.
• agentRunProductivity.ts:
  • recordAgentRunActingMutations keeps its transaction without the lock query.
  • The comments in pauseScheduleForUnproductiveStreak now describe only the write lock.
• oauth.ts: lockGrantRow is now readGrantRow, without .forUpdate(). Its result is still used, both when an operation is admitted under a grant and when a grant is revoked.
• Comments: I rewrote the ones in policy.ts, agentRunInternalRoutes.ts, agentRunStore.ts and test/processAgentRunJob.test.ts to describe only SQLite's write lock (BEGIN IMMEDIATE).
• Still on the branch: one .forUpdate() in revokeAgentRunPhaseGrant (packages/api/mcp/agentRunGrants.ts). It was already on main (the PR only rewrote that line), and main's lockPhaseRecord uses the same lock. I left it alone because you asked me to remove only what this PR added. Say if you want it taken out too; it does nothing on SQLite.

Nothing in the PR's diff (docs, changelog or comments) mentions PostgreSQL, pg, FOR UPDATE or row locks any more. SQLite behaviour is unchanged: these transactions already start with BEGIN IMMEDIATE.

Verification

• Typecheck passes for root, core and api, and eslint passes on the changed files.
• The root suites all pass, 266 of 266. That covers report ordering, mutation-count settlement, streak evaluation and pause recovery, including the test where a productive write lands between the streak read and the pause commit. None of those tests were edited.
• On the API side, agentDefinitionRoutes exited cleanly with no failures. agentRunGrants, mcpAgentRuns and mcpMergePreconditions passed every test, then hung at exit and were killed by the timeout. That exit hang is a known issue with these test files.

There's no visual preview, since nothing here changes the UI.

---

🤖 Implementation Details

• Model: Claude Opus 5.5 (medium)
• Requested By: @integry
• Time: 50m 11s
• Tokens: 2,696,295 (2,679,340 in / 16,955 out)
• Cost: $1.31
• Subscription usage: Weekly +2%, Fable +1%

Undo Changes • View Task Execution

---

💡 ProPR Slash Commands

Command	Description	Example
/merge	Merge target branch into this PR and resolve conflicts	/merge
/review	Request an AI code review	/review or /review claude-sonnet
/fix	Address review blockers, or blockers and suggestions with /fix all	/fix, /fix all, or /fix F3 S5
/switch	Change the AI model for this PR	/switch claude-opus
/use	Override the model for a single follow-up run	/use claude-sonnet
/ultrafix	Loop review→fix cycles until score goal is met	/ultrafix or /ultrafix goal=8 max=10

_{🤖 Generated by ProPR}
Processing comment ID: 6828835260✓

[integry]

/review astra fable

[propr-dev]

✅ AI Code Review Complete requested by @integry

Posted 2 reviews:

• GPT-6 Astra
• Claude Fable 5.1

View Task Details

[propr-dev]

🔍 AI Code Review — astra

Overall Evaluation

The PR adds report novelty, acting-operation counts, consistent client flags, and recoverable streak notifications and schedule pauses. It needs changes before merge because recovery can finalize productivity while an abandoned acting grant remains usable.

✅ Consistent presentation — REST, MCP, CLI, and UI expose the productivity flags without changing successful runs’ terminal state.

✅ Durable recovery — Ended-grant records preserve mutation-count recovery, and the pause marker prevents repeatedly disabling a schedule the owner re-enabled.

✅ Report ordering — Report storage checks the predecessor transactionally and assigns increasing storage timestamps.

The original objective is explicitly truncated; this review assesses its visible requirements and the complete authorized follow-up. Supplied checks show 30 passed, six pending, and no failures. No commands were run.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F28: 🔴 Keep settlement pending when revocation fails

• Required behavior: A claimed acting step must not receive a final productivity verdict while its grant can still admit mutations; otherwise the new streak logic can pause an automation that is still changing things.

• Evidence: src/jobs/processAgentActionJob.ts, adoptAbandonedClaim, mutationsPatch, and keepMutationsOfEndedRun; packages/core/src/services/agents/agentRunProductivity.ts, recordAgentRunActingMutations.

  1. A worker stops after claiming an acting step and issuing its grant, leaving its container running—the abandoned-container case explicitly described by adoptAbandonedClaim.
  2. BullMQ redelivers the job to another worker. During await deps.mcpGrants.revoke(runId, { phase: 'action' }), a transient request failure prevents revocation from reaching the API.
  3. adoptAbandonedClaim catches that failure, sets the count to null, and returns successfully. Recovery then fails the still-acting run with actingMutationsPending: false. For an already-cancelled run, keepMutationsOfEndedRun instead clears the pending flag through recordAgentRunActingMutations.
  4. With an unchanged report and two preceding unproductive scheduled runs, the recovered run now completes the streak and can disable its schedule.
  5. The abandoned container can subsequently perform a mutation with the still-valid grant. The stored verdict and pause are already final, and this delivery has completed without arranging another settlement attempt.

  The admission check only rejects revoked or expired grants; revocation never occurred here. SQLite transactions protect the database writes but cannot establish that the grant ended. activeRuns is local to the replacement processor and does not protect a container left by another worker.

  static trace: Both recovery branches explicitly turn a failed revocation into an unknown-but-settled count. This differs from successful revocation followed by an unreadable count: here, future mutations remain authorized. No test was executed.

• Minimum fix: Distinguish failed revocation from successful revocation with an unknown count. On revocation failure, retain the pending obligation and retry revocation before evaluating productivity, for both still-acting and already-ended recovery paths. Do not persist a null job obligation that causes the retry to skip revocation.

Suggestions

These are optional follow-ups and are not sent to /fix.

S11: 🟢 Assert the actual transaction mode

The new locking tests accept any statement beginning with begin, although the documented guarantee specifically requires BEGIN IMMEDIATE. As an optional improvement, assert the rewritten statement under the production SQLite configuration; this would catch accidental loss of the configuration on which the new read-before-write transactions depend.

Score

The implementation has substantial coverage and coherent cross-surface behavior, but failed-revocation recovery can produce an incorrect durable verdict and schedule pause. The pending checks are not themselves blockers.

Score: 6/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 1m 14s
• Tokens: 150,490 (148,820 in / 1,670 out)
• Cost: $1.46

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix F28 S11

_{🤖 Review by ProPR}

[propr-dev]

🔍 AI Code Review — fable

Overall Evaluation

This PR implements #3442 end to end: a normalised report hash and reportNovelty on every stored report, an actingMutations count derived from the acting grant's mcp_operations receipts (fenced by re-reading the grant under the receipt transaction and remembered in an agent_run_ended_grant record so a crashed worker's redelivery can still recover it), an automation_unproductive_run_threshold setting, a once-per-streak Inbox item keyed on a stable streak identity, an opt-in pauseWhenUnproductive schedule pause with its own durable "pause made" record, and the flags on REST, MCP, CLI and the run history/detail UI. The follow-up request is honoured: the diff (64 files, complete) contains no .propr/ setup changes, no pg/PROPR_TEST_POSTGRES_URL references, no PostgreSQL mentions in comments or docs, and the only remaining .forUpdate() calls in agentRunGrants.ts are pre-existing on the base (the removed - line already carried one).

The design is unusually careful about the awkward paths (owner cancel while the step holds its grant, lost task writes, lost count writes, worker crash between revoke and count store, pause interrupted after the item is stored), and each is backed by a test against the real SQLite store. I could not drive any path to an incorrect persistent state. The main residual risks are verification gaps (three test shards and Validate Changes still pending) and a few deliberate design trade-offs noted below as suggestions.

✅ Compare-and-store under one write lock — storeAgentRunReport re-reads the definition's latest report inside the transaction and refuses the write when the predecessor changed (previous_changed), and storeReportWithNovelty reclassifies; agentRunReportNoveltyPrevious.test.ts proves two concurrent identical reports end as one new and one unchanged, and that same-millisecond stores keep storing order.

✅ Count survives the revoke→store crash window — revokeAgentRunPhaseGrant writes the ended-grant record in the same transaction that deletes the phase record, purgeEndedGrantRecords keeps it while acting_mutations_pending is set, and the redelivery path (adoptAbandonedClaim → by-phase revoke → revoked:false + count) is exercised in agentRunGrants.test.ts (F24/F27) and processAgentActionJob.test.ts.

✅ Pause evidence separated from notification intent — the immutable Inbox item records only pauseIntended; pauseScheduleForUnproductiveStreak claims unproductive_paused_streak conditionally, re-reads the opt-in and the streak under the write lock, and rolls the claim back on streak_ended, so an owner's opt-out or a newer productive run between item and pause keeps the schedule on (covered in agentRunUnproductiveStreakRecovery.test.ts).

Verdict: merge-ready within scope, conditional on the pending shards and Validate Changes finishing green.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S12: 🟢 Retry instead of settling an unknown count

adoptAbandonedClaim (src/jobs/processAgentActionJob.ts) sets ownsClaim = true and actingMutations = null when the by-phase revoke call fails (API unreachable), and keepMutationsOfEndedRun then stores that unknown with acting_mutations_pending = false. The ended-grant record and its receipts still exist at that moment, so a retry would have recovered the real count; after the settlement the run's verdict falls back to its report alone (a mutating step with an unchanged report then reads as unproductive). The normal endGrant path has the same trade-off and the docs state that null is never read as a no-op, so this is a deliberate limitation rather than a defect, but throwing AgentRunMutationsSettlementError on a failed revoke during adoption would make the recovery path strictly stronger.

S13: 🟢 Confirm no generic expiry sweep deletes ended-grant rows

F27 retention relies on purgeEndedGrantRecords being the only thing that removes agent_run_ended_grant rows past expires_at. If McpStore (or a daemon job) has a kind-agnostic purge of expired mcp_records, a run whose count is still owed would lose the grant's name after the TTL and adoptAbandonedClaim would settle unknown. The tests do not run such a sweep, so this is worth a quick check against the store rather than a code change in this PR.

S14: 🟢 API route depends on a worker module

packages/api/routes/automationDefinitionRoutes.ts now imports evaluateUnproductiveStreakQuietly from ../../../src/jobs/agentRuns/unproductiveStreak.js. The API already reaches into worker modules elsewhere (followupHandoffs), so this is not new in kind, but the streak evaluator has no worker-specific dependencies and could live in @propr/core next to agentRunProductivity.ts, which would keep the API package build independent of root src/.

S15: 🟢 Badge while the acting step is undecided

runNoChangeReason derives unchanged_report from the flags regardless of state, so an awaiting_approval preview run shows "No change" although its acting step may still change something, and agentRunProductivity deliberately treats that run as pending. Hiding the badge while the state is in UNDECIDED_STATES (or wording it as "Same report") would align the UI with the streak logic. Purely a UX choice.

S16: 🟢 Normalisation false positives

VOLATILE_PATTERNS in agentRunNovelty.ts treats the bare word "may"/"march"/"august" followed by a number as a date (\bmay\s+\d{1,2} → "may 2 of them" becomes <date>), and the clock-time pattern collapses ratios like "10:30 split". These only lower sensitivity (more unchanged), never cause a spurious new, and the tests pin the important cases, so a follow-up refinement is optional.

S17: 🟢 Cosmetic outcome for a deleted definition

pauseScheduleForUnproductiveStreak returns done_before when the definition row no longer exists (current undefined), which OUTCOME_AFTER_NEW_ITEM maps to 'paused'. Nothing is written, so this only affects the returned outcome and log line; returning opted_out or a distinct value would be clearer.

S18: 🟢 Watch file growth against the lint gate

src/jobs/processAgentActionJob.ts grows by ~190 net lines and now carries claim adoption, count settlement and job-data persistence alongside the lifecycle. The author reports lint passing and Validate Changes is still pending; if max-lines becomes tight, the adoption/settlement helpers (adoptAbandonedClaim, keepMutationsOfEndedRun, keepOwedMutationsWithJob) are a natural extraction.

S19: 🟢 Fixture fakes outside the changed tests

toolExecution.ts now calls deps.policy.admit on every mutating tool call and createAgentRunProcessor now routes report storage through deps.storeReport; mcpMergePreconditions.test.ts was updated for the former, and processAgentRunJob.test.ts for the latter. Any other suite that constructs a fake policy without admit or overrides transitionRun without storeReport would fail in the still-pending shards; confirming those shards is the only remaining verification step.

Score

The effective objective (including the PostgreSQL removal) is met, the changed paths hold up under tracing, and the new behaviour is thoroughly tested against the real store; readiness is conditional only on the pending shards and Validate Changes.

Score: 8/10

---

🤖 Review Details

• Model: Claude Fable 5.1
• Time: 8m 5s
• Tokens: 266,875 (231,166 in / 35,709 out)
• Cost: $4.68

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix S12 S13 S14 S15 S16 S17 S18 S19

_{🤖 Review by ProPR}