fix: gate off-orglist purge on governance freshness, not sync state - #1750
Merged
Conversation
Governance readiness was a boolean cleared at the start of every governance sync. The sync-governance-body and sync-default-organizations jobs share a default 120s interval and fire on the same scheduler tick, so the consumer read the cleared value for the whole cycle and the ONLY_CADT_SUBSCRIPTIONS purge was skipped every time. Track the timestamp of the last confirmed-good sync instead. Readiness now expires after five sync intervals, clamped to between 10 and 60 minutes, and never dips false merely because a refresh is underway. The clamp keeps its own floor of two intervals so a long configured interval cannot expire readiness before the next sync could renew it, which would disable reconciliation outright rather than pause it. A download that carries no orgList cannot vouch for the cached one, so it now revokes readiness rather than leaving the purge running against stale data until the window lapses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Governance readiness was a boolean cleared at the start of every governance sync.
sync-governance-body(producer) andsync-default-organizations(consumer) are separate jobs that share a default 120s interval and fire on the same scheduler tick, so the consumer read the cleared value for the entire cycle and theONLY_CADT_SUBSCRIPTIONSoff-orglist purge was skipped every time. On an affected node this leaves organizations that governance has dropped from the orgList subscribed and syncing indefinitely.GOVERNANCE_SYNC_TASK_INTERVALlonger than the ceiling would expire readiness before the next scheduled sync could renew it, disabling reconciliation outright instead of pausing it.src/utils/governance-readiness.js, since the failure mode is timing-dependent and not visible from the call sites.Reviewer note
ONLY_CADT_SUBSCRIPTIONSdefaults totrue, so this moves the off-orglist purge from "almost never runs" to "runs on every consumer tick following a good sync" on default-configured deployments. That is the intended fix, but it activates the unsubscribe-and-delete path broadly on upgrade and is worth an explicit ack rather than being treated as a routine bug fix.Related tuning question: the 10-minute staleness window spans five reconcile cycles while the purge needs only three grace cycles, so a purge can complete after governance sync has begun failing. That follows from choosing 10 minutes as the tolerance; lowering the interval multiplier or raising the grace cycles would tighten it.
Test plan
npm run test:v1— 155 passing, 5 pending (unchanged from base)npm run test:v2— 1756 passing (up 5 from the new tests), 0 failingnpx eslintclean on all changed files; the 5 dynamic-import errors ingovernance-sync-subscribe-first.spec.jspre-exist on the base branch and are not added toKnown gaps: the V1 no-orgList revocation branch is not reachable from the test runner because
GovernancecapturesUSE_SIMULATORat module import (a limitation already documented for V1 sync paths), so it is covered by the V2 equivalent plus code reading. The task-level call sites have no direct test; the regression guard sits at the model level.Note
High Risk
Changes when the destructive off-orglist unsubscribe-and-delete path runs on default-config nodes (
ONLY_CADT_SUBSCRIPTIONSdefaults true), so upgrades can activate broad reconciliation; the logic is safety-critical for local org data.Overview
Fixes
ONLY_CADT_SUBSCRIPTIONSoff-orglist purge being skipped on most scheduler ticks because governance readiness was cleared at the start of every governance sync while the purge task read that flag on the same interval.Governance readiness is now a last-confirmed-good timestamp (not a boolean cleared on sync start). It stays true during an in-flight refresh and expires after five
GOVERNANCE_SYNC_TASK_INTERVALperiods, clamped between 10 and 60 minutes, with a floor so long intervals do not expire before the next sync can renew.markGovernanceNotReadyis only used when a download carries no orgList; sync tasks no longer clear readiness when a refresh begins.The default-org reconcile tasks read
isGovernanceReadyonce per run and log that purge waits until a recent sync confirms a fresh orgList. README documents the freshness window and ties it toGOVERNANCE_SYNC_TASK_INTERVAL. Tests cover expiry, window scaling, in-flight sync, and no-orgList revocation (V1/V2).Reviewed by Cursor Bugbot for commit 5a98cbd. Bugbot is set up for automated code reviews on this repo. Configure here.