Skip to content

Refactor: breadcrumb-nonreentrant-upgrade-lifetime-guard #655

Description

@drmoisan
  • Work Mode: full-feature

Problem / Why

BreadcrumbCoordinatorUpgradeLifetime guards population work by entry-time lease currency, through
TryRunCurrent and RunSynchronous. That verdict is deliberately an entry-time check only: a guarded
action that re-entrantly begins another population, or that calls back into the lifetime, is not
prevented from doing so.

Feature #501 relied on that property and added a test pinning it,
TryRunCurrent_ReentrantInvalidateStillReportsEntryTimeInvocation, so the present behavior is
specified rather than accidental. Research section 6.2 option C proposes the alternative: a guard that
refuses, rather than permits, a nested population under the same lifetime.

This was out of scope for #501. That issue fixed four ordering and lifetime defects (#462, #500, #501,
#502) without changing the guard's re-entrancy contract. Adopting option C changes the contract for
every caller of the lifetime, which carries its own regression surface; folding it in would have
widened a four-defect correctness fix into an API redesign.

Proposed Behavior

Decide whether nested population under a single lifetime is ever legitimate. If it is not, make the
guard non-re-entrant so a nested population is refused, and convert the existing entry-time test into
the negative case.

Acceptance Criteria

  • A decision record states whether nested population under one lifetime is legitimate.
  • If refused, TryRunCurrent and RunSynchronous reject a nested population deterministically.
  • Every TryRunCurrent and RunSynchronous caller is audited for a nested-population path.
  • TryRunCurrent_ReentrantInvalidateStillReportsEntryTimeInvocation is updated to match the chosen contract.

Constraints & Risks

  • Changes a contract every caller of the lifetime depends on; the blast radius is the whole breadcrumb
    coordinator surface.
  • Feature Bug: breadcrumb-hub-postjson-caches-before-broadcast-starves-attachments #501 shipped a test that pins the CURRENT entry-time semantics. That test must be updated
    deliberately, not deleted, or the change will look like a regression.
  • Touches files owned by sibling features; coordinate ownership before editing.

Test Conditions

  • Unit coverage for a nested population attempt under one lifetime
  • Unit coverage for the existing non-nested path, proving no behavior change
  • Regression coverage for every audited caller

Source

From: docs/features/potential/2026-08-27-breadcrumb-nonreentrant-upgrade-lifetime-guard.md

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions