Skip to content

Follow-ups from runtime expiry (#340) and identity activation (#341) reviews #343

Open

Description

@myobie

Non-blocking follow-ups from the #340 and #341 reviews. Each was verified in code at the PR heads; none blocks the merge, but each should be fixed.

Runtime expiry (#340):

  • compose builds a full composed view per distinct deadline, on every read under the runtime mutex (up to ~600 deadlines times up to 1024 records). Build the views once and store only per-deadline deltas, or skip the generation build in receipt() and snapshot.
  • Mass expiry commits one record per iteration while holding policy_update, so an emergency remove_peer waits behind every pending commit. Batch removals per owner and release the lock between commits.
  • expiry_last_seen is written only inside commit. Crash between a deadline elapsing and its cleanup commit, restart with the wall clock still before the deadline but at or above the stored anchor: the record is live again until the deadline. Persist the anchor when the sweeper first observes an elapsed deadline, and at clean shutdown.
  • Rollback below expiry_last_seen expires every expiring record silently. Log a warning with the observed rollback. The reconcile path compares raw wall time even in the same daemon lifetime (a 1 s NTP step back expires all), and it mutates the shared clock for the rest of the process. Skip the raw check in-process.
  • After a forward clock spike set_expiry returns expires_at must be within the next 600 seconds; say the clock is ahead of wall time instead.
  • Edits (set_grants, add_peer) to an expired, not-yet-cleaned record succeed; refuse them.
  • close_when_revoked waits on peer_book.read() before closing the connection, so closing waits behind a writer holding the publication lock. Authorization stays denied, but docs say retirement is independent of writers.
  • The cleanup retry logs a warning ten times a second when it fails persistently; rate limit it.
  • Cleanup that hits RuntimeCommitIndeterminate quarantines with cancel_existing, which cancels all peers' work from a background timer.
  • Document that set_expiry skips the sync-selector guard that remove_peer enforces.

Identity (#341):

  • No test drives restart_identity_service or the supervised path. Add at least a unit-level seam test.
  • identity activate hands the replacement daemon DaemonOptions::default(), so --server-session-* caps and TTL are dropped. Document it or carry them over.
  • fabric identity status takes the identity lock and returns identity_busy for the whole restart (up to ~45 s).
  • Retired-secret retention uses wall time; a forward jump deletes retired keys early.
  • The unsupervised replacement daemon is spawned without setsid.
  • ensure_engine cancels the old sync engine without waiting for its state lease; the heartbeat retries, so it self-heals.
  • Tests: the "old tunnel remained usable" and "old NodeID refused" checks pass on any timeout or failure; assert a close code. The sync-author test asserts author_from_node_id(prev) != author_from_node_id(new), which is tautological; check the running engine's author.

Added after the #341 round-2 review:

  • Fresh-daemon poll in reconcile is 2 s; a supervisor-started daemon still starting after that is shut down.
  • Recovery discards ephemeral exposures (document, or write a snapshot with next: current).
  • recover_identity_activation uses load_or_create_identity; use read_id.
  • warn! for sweep and corrupt-pending is invisible in the CLI (no subscriber).
  • Tests that cover only the helper or only the Status timeout: Snapshot/Shutdown/Abort timeouts, identity_stop_lease timeout, runtime ON-stays-ON, the CLI recovery branch with a live daemon.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions