Skip to content

fix: wire communicate-score cycle-boundary sentinels#448

Closed
cdunda-perchwell wants to merge 1 commit intopeteromallet:mainfrom
cdunda-perchwell:fix/communicate-score-cycle-boundaries-447
Closed

fix: wire communicate-score cycle-boundary sentinels#448
cdunda-perchwell wants to merge 1 commit intopeteromallet:mainfrom
cdunda-perchwell:fix/communicate-score-cycle-boundaries-447

Conversation

@cdunda-perchwell
Copy link
Contributor

Summary

  • add a communicate_score_resolved_this_cycle workflow guard so workflow::communicate-score does not re-inject after resolve + rescan in the same cycle
  • clear that sentinel at the same true cycle boundaries as the sibling workflow sentinels
  • add focused regressions for scan reconcile, trusted import, and helper-level sentinel clearing

Problem

workflow::communicate-score had a resolved-this-cycle sentinel pattern, but the boundary reset wiring was incomplete. That meant the framework could either:

  • re-inject the workflow unexpectedly after resolve + rescan in the same cycle, or
  • suppress it too long if the new sentinel was not cleared at the next real cycle boundary

Fix

  • gate sync_communicate_score_needed() on communicate_score_resolved_this_cycle
  • set the sentinel at injection time
  • clear it at:
    • force-rescan reset
    • plan-start score seeding
    • queue-drain / plan-start clear
    • trusted review import

Tests

  • uv run python -m pytest -q desloppify/tests/plan/test_reconcile_pipeline.py -k 'clear_communicate_score_sentinel or clear_score_communicated_sentinel or communicate_score'
  • uv run python -m pytest -q desloppify/tests/commands/scan/test_plan_reconcile.py desloppify/tests/commands/scan/test_plan_reconcile_postflight_and_reconcile.py desloppify/tests/commands/review/test_review_importing_support_direct.py desloppify/tests/commands/test_lifecycle_transitions.py desloppify/tests/plan/test_reconcile_pipeline.py -k 'communicate or sentinel or plan_start_scores or force_rescan or trusted_internal_clears'

Fixes #447.

@cdunda-perchwell
Copy link
Contributor Author

Status update

This PR is incomplete — it partially addresses the issue but does not fully fix it.

What is in this PR

  • communicate_score_resolved_this_cycle sentinel added to sync_communicate_score_needed
  • clear_communicate_score_sentinel() helper wired at force-rescan, score seeding, queue drain, trusted import

What is missing

Path 1 fix (scan path): _clear_plan_start_scores_if_queue_empty in plan_reconcile.py still calls clear_communicate_score_sentinel(plan) when the queue drains. This clears the sentinel too early — on the next scan, both guards (previous_plan_start_scores and communicate_score_resolved_this_cycle) are gone and sync_communicate_score_needed re-injects.

The fix is one line: remove clear_communicate_score_sentinel(plan) from _clear_plan_start_scores_if_queue_empty. The sentinel should survive until _seed_plan_start_scores clears it at the start of the next cycle.

Path 2 (resolve path): Re-injection also occurs immediately during the resolve itself (same timestamp as the done log entry), before any rescan. Root cause not yet confirmed — simulation does not reproduce it. See issue #447 for full details.

Tests

The existing test test_clears_and_copies_to_state was asserting the old (buggy) behavior (communicate_score_resolved_this_cycle not in plan). It needs to be updated to assert the sentinel IS preserved after queue drain.

A new regression test test_communicate_score_not_reinjected_after_resolve covering the two-scan scenario is written and passing locally but not yet pushed.

@peteromallet
Copy link
Owner

Hey, thanks for the sentinel fix — the cycle-boundary logic looks solid and I want to get it merged. However this PR also bundles the entire prr plugin (~19k lines), resolve hooks, scoring changes, and broader exception handling that aren't mentioned in the title. Could you split this into separate PRs? Happy to merge the sentinel fix immediately once it's isolated.

The prr plugin and resolve hooks framework deserve their own focused reviews. Mixing them in here makes it really hard to review either piece properly.

@cdunda-perchwell cdunda-perchwell force-pushed the fix/communicate-score-cycle-boundaries-447 branch from 03914ab to 871cefd Compare March 17, 2026 14:10
Add communicate_score_resolved_this_cycle sentinel that blocks
workflow::communicate-score from being re-injected after the agent
resolves it. The sentinel is set at injection time and cleared at
cycle boundaries (force-rescan, score seeding, trusted import) but
intentionally survives queue-drain so the post-drain rescan does not
re-inject.

Also clears scan_count_at_plan_start on force-rescan to fully reset
cycle state.

Fixes peteromallet#447
@cdunda-perchwell cdunda-perchwell force-pushed the fix/communicate-score-cycle-boundaries-447 branch from 871cefd to a4e5100 Compare March 17, 2026 14:37
@cdunda-perchwell
Copy link
Contributor Author

derp! Should be better now! @peteromallet

@peteromallet
Copy link
Owner

Fix cherry-picked into v0.9.10 — communicate-score sentinel resets are resolved. Thank you @cdunda-perchwell for identifying both the original issue (#447) and the sentinel reset bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix communicate-score cycle-boundary sentinel resets

2 participants