docs: design and plan HumWatch 2.1 fleet hub - #17
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a2c28e124
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - [ ] **Step 5: Detect the role before applying any network security gate** | ||
|
|
||
| Move role detection ahead of the current unconditional `validate_security_config()` calls in `agent/main.py`. Existing databases with an auth token start as `legacy`. Fresh databases start as `unconfigured`. Unconfigured mode binds HTTP only to `127.0.0.1` and does not require a token or certificate. Legacy mode keeps the existing token, TLS, and CORS validation exactly. Hub and agent modes use loopback or explicitly stored private listeners until Task 9 adds the settings surface. No role may fall back to `0.0.0.0`. |
There was a problem hiding this comment.
For ordinary-LAN enrollment in Task 4, the target PC is still unconfigured, so binding that role exclusively to 127.0.0.1 makes the hub's request to the advertised private address unable to reach /api/internal/v1/enrollment/claim or /finalize. Private listener selection is deferred until Task 9, while Task 7 only displays addresses, so the planned Create/Join Fleet flows cannot work across machines in Tasks 4–8. Add a restricted enrollment listener or move listener configuration ahead of enrollment.
Useful? React with 👍 / 👎.
| await self.state.retire_local_hub(announcement, remove_signing_key=True) | ||
| await self.target_client.publish_hub(announcement) |
There was a problem hiding this comment.
If the process crashes or the target request fails after retire_local_hub(..., remove_signing_key=True) commits but before publish_hub() completes, the old hub can no longer retry and the target remains unannounced, so agents continue using the retired hub. Task 11 supplies only a manual dead-hub recovery path and no durable target-side publication retry. Persist a target-owned publish obligation or otherwise acknowledge publication before making retirement irreversible.
Useful? React with 👍 / 👎.
|
|
||
| Serialize fleet identity, machine registry, settings, integrations, schema version, generation, and timestamp. Encrypt with ephemeral X25519, HKDF-SHA256, and AES-GCM to the recovery public key. Copy the ciphertext to enrolled agents through signed internal requests. | ||
|
|
||
| Start `run_replication_loop()` from the hub lifespan. Create and distribute a bundle immediately on hub startup, after enrollment, revocation, listener or integration changes, and after a completed transfer. Also refresh every six hours. Creating a bundle upserts one `recovery_deliveries` row per active agent in the same transaction. On conflict at `(generation, machine_id)`, an unchanged bundle digest preserves its delivery state. A changed digest atomically replaces the digest and resets state to `pending`, attempt count to zero, next attempt to now, and acknowledgement to null. Failed deliveries durably update attempt count and next attempt time for retry after 1, 2, 4, 8, 16, then 30 minutes. A signed agent acknowledgement marks that machine and generation acknowledged only when its digest matches the current row. Startup resumes every pending or due row. Agents atomically replace only an older valid replica or a different authenticated bundle for the same generation, retain the newest valid bundle across restart, and return the accepted generation plus bundle digest so the hub cannot acknowledge the wrong ciphertext. |
There was a problem hiding this comment.
Because periodic and fleet-change refreshes create multiple digests under the same generation, allowing any different authenticated same-generation bundle to replace the replica permits an older in-flight delivery to overwrite a newer acknowledged bundle. The hub may already consider the newer digest delivered and therefore never resend it, leaving recovery with stale registry or revocation state. Include a monotonic bundle revision or authenticated creation ordering and accept replacements only when they are newer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 581b0e5575
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - [ ] **Step 5: Detect the role before applying any network security gate** | ||
|
|
||
| Move role detection ahead of the current unconditional `validate_security_config()` calls in `agent/main.py`. Existing databases with an auth token start as `legacy`. Fresh databases start as `unconfigured`. Unconfigured mode starts its setup surface on `127.0.0.1` and does not require a token or certificate. Task 4 adds a loopback-selected, restricted private enrollment listener before its hub-orchestrated claim runs. That listener may expose only `/api/internal/v1/enrollment/claim` and `/finalize`, never the setup page, pairing-code endpoint, dashboard, or any v1 route. Legacy mode keeps the existing token, TLS, and CORS validation exactly. Hub and agent modes use loopback or explicitly stored private listeners until Task 9 adds the owner-managed settings surface. No role may fall back to `0.0.0.0`. |
There was a problem hiding this comment.
Although Task 4 now gives the unconfigured target a restricted private enrollment listener, this rule still leaves a newly created hub on loopback because neither Task 3's Create Fleet transaction nor Task 7's setup UI persists a private hub listener before Task 9. In the ordinary two-PC flow, the enrolled agent therefore cannot send its first /api/internal/v1/reports request to the hub, so it remains enrolling; require Create Fleet to select, store, and bind at least one private hub address before enrollment is usable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Create Fleet now names, stores, and binds the first private hub listener inside the creation transaction, and rejects creation when no private address exists. Fixed in 7f4d54a (Task 3 Step 5, Task 7 Step 4, the role rule here, and the spec's fleet-creation list).
|
|
||
| The target imports into staging, persists the signed proposed generation, initializes that generation's `recovery_bundle_revision` to zero, and starts in unannounced hub mode on its configured private listener. Readiness must fetch the real static login page, prove an invalid login gets the expected generic `401`, and submit a valid signed report from an enrolled test machine under the committed proposed generation. The target returns a signed readiness receipt containing the generation and each check digest. | ||
|
|
||
| If staging, activation, or readiness fails before `enter_draining_transfer()` commits, the old hub sends a signed abort. The target deletes the staged authority and resumes agent mode while the old hub remains active. After readiness, the old hub durably enters a draining state that rejects new reports and fleet mutations but retains its key while the target activates a target-owned publication obligation. Activation is idempotent by the signed announcement digest. Once activation is requested, a timeout or lost response never calls `abort_pending_hub()`: the old hub remains draining and reconciles the target's signed obligation status, retrying the same activation request until it obtains the persisted receipt. It may send a signed abort only after the target reports that the digest was never activated. That obligation persists the signed announcement and a per-agent delivery outbox before any send attempt, retries after target restart, and returns a signed persistence receipt. Only after that receipt does the old hub atomically revoke sessions, persist retired agent state at the new generation, and remove its usable fleet signing key. If the old hub restarts while draining, its transfer journal requires the same status reconciliation before it can finish the accepted publication path or send the signed abort. A restarted old hub cannot mint a later generation, and a crash after local retirement leaves the target's durable delivery retry as the recovery path rather than relying on the retired hub. |
There was a problem hiding this comment.
The snapshot is created before target staging and readiness, while this step delays rejecting new reports until the old hub enters draining. Any report accepted and acknowledged by the old hub during that interval is absent from the target snapshot, but the agent has already advanced its acknowledgement watermark and will not resend it, permanently dropping those samples after transfer. Pause ingestion before the snapshot or replicate the post-snapshot delta and acknowledgement state before retiring the old hub.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The old hub now durably enters an ingestion freeze before the snapshot, so no acknowledgement watermark can advance past it. Agents buffer through their outbox, a signed abort lifts the freeze, and Step 1 gains a no-acknowledged-sample-lost test plus restart-while-frozen coverage. Fixed in 7f4d54a.
|
|
||
| - [ ] **Step 5: Implement create-fleet as one transaction** | ||
|
|
||
| Validate that role is `unconfigured`, request source is loopback, username is nonempty, and password is at least 12 characters. Create fleet identity, owner record, self machine row, and runtime role `hub` in one database transaction. Return the printable recovery key exactly once. |
There was a problem hiding this comment.
If fleet creation commits but the HTTP connection is lost before this response reaches the browser, the installation is already a hub, a retry is rejected because it is no longer unconfigured, and the recovery private key cannot be reconstructed because the plan persists only its public key. That permanently disables dead-hub recovery without any visible setup failure; retain a protected pending setup result until the browser acknowledges it or provide an explicit recovery-key rotation flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The recovery key now persists in a protected pending-setup record until the owner acknowledges saving it. GET /api/v2/setup/recovery-key (owner session, loopback) retrieves it after a lost response, and acknowledgement is the only deletion path. Fixed in 7f4d54a, spec retention wording updated to match.
|
|
||
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. Promotion creates a new owner password, revokes integrations, raises generation, switches to hub mode, and publishes a signed announcement. |
There was a problem hiding this comment.
After a hub transfer, an agent can accept generation G+1 through the durable announcement obligation while still holding its last recovery bundle from generation G if the new hub dies before bundle replication. Merely incrementing the bundle generation then publishes G+1, which every such agent rejects as equal generation. Promotion must choose max(bundle_generation, locally_accepted_generation) + 1, with a test covering recovery during the post-transfer replication window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Promotion now publishes at max(bundle_generation, locally_accepted_generation) + 1, with a Step 1 test covering recovery inside the post-transfer replication window. Fixed in 7f4d54a.
Create Fleet now stores and binds the first private hub listener, the recovery key survives a lost setup response behind an acknowledge-to-delete pending record, hub transfer freezes report ingestion before the snapshot so no acknowledged sample can miss the target, and dead-hub promotion picks a generation above both the bundle and the locally accepted announcement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f4d54aae6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ```python | ||
| @asynccontextmanager | ||
| async def transaction(db: aiosqlite.Connection): | ||
| await db.execute("BEGIN IMMEDIATE") |
There was a problem hiding this comment.
This helper is designated for every fleet repository, but the application currently exposes one module-level aiosqlite.Connection; concurrent ingestion from multiple agents, the hub's local collector, and background recovery or retention work can therefore interleave two BEGIN IMMEDIATE calls on the same connection, causing cannot start a transaction within a transaction and dropped/retried operations. Protect the complete transaction context with a shared async lock or allocate independent connections so concurrent repository calls cannot overlap transactions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The helper now holds a per-connection asyncio.Lock across the whole transaction span, with a required test running overlapping transactions on one connection. Fixed in 7b6f0eb.
|
|
||
| Validate that role is `unconfigured`, request source is loopback, username is nonempty, and password is at least 12 characters. The request also names the first hub listener, chosen from the machine's detected non-loopback private addresses and defaulting to the primary private address with the current port. Reject creation when no private address exists rather than silently creating an unreachable hub. Create fleet identity, owner record, self machine row, the stored hub listener, and runtime role `hub` in one database transaction, then bind the stored listener before the endpoint reports success, so enrollment claims and agent reports work without a restart. | ||
|
|
||
| Do not return the printable recovery key as the transaction's only copy. Persist it in a protected pending-setup record inside the same transaction. `GET /api/v2/setup/recovery-key` (owner session, loopback source) returns the printable key for as long as the record exists, and `POST /api/v2/setup/recovery-key/ack` deletes it, which is the only deletion path. A create-fleet response lost before the browser renders the key therefore never strands dead-hub recovery: the owner logs in at the hub PC and retrieves it, then acknowledges. After acknowledgement the private key is unrecoverable from the hub, matching the design's retention rule. |
There was a problem hiding this comment.
The required pending-setup record has no table in Task 2's exhaustive schema list, and Task 3 does not include agent/fleet/schema.py among its files even though this record must participate in the create-fleet database transaction. An implementation following the plan consequently cannot durably retain and transactionally delete the recovery private key, so the lost-response recovery guarantee remains unimplemented; add the protected record's schema and migration plus its acknowledgement lifecycle to the plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added pending_recovery_key(id, printable_key, created_at) to Task 2's schema list and agent/fleet/schema.py to Task 3's files, with Task 3 Step 5 naming the table so the create-fleet transaction can durably retain and transactionally delete it. Fixed in 7b6f0eb.
|
|
||
| - [ ] **Step 2: Implement bundle creation and replication** | ||
|
|
||
| Serialize fleet identity, machine registry, settings, integrations, schema version, generation, a transactionally incremented positive `bundle_revision`, and timestamp. Bind both generation and bundle revision into authenticated associated data as well as the encrypted payload. Encrypt with ephemeral X25519, HKDF-SHA256, and AES-GCM to the recovery public key. Copy the ciphertext to enrolled agents through signed internal requests. |
There was a problem hiding this comment.
X25519 encryption with an ephemeral sender key and AES-GCM does not authenticate the sender: any enrolled machine that knows the recovery public key can construct a valid higher-revision ciphertext, and the shared internal-request verifier only establishes that the caller is a known, non-revoked machine. Such a request can replace agents' replicas and later decrypt successfully into attacker-selected registry and fleet-signing-key state during recovery. Require a fleet-key signature over the bundle or explicitly authorize the stored current hub identity on the replica endpoint, and test forged bundles produced with the legitimate recovery public key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Bundles now carry a fleet-key signature over generation, revision, ciphertext digest, and associated data. Agents verify with the retained fleet public key before replacing a replica, recovery preview verifies before decrypting, and Step 1 gains a forged-bundle test using the legitimate recovery public key. Spec updated to match. Fixed in 7b6f0eb.
The transaction helper now serializes spans with a per-connection asyncio lock, the pending recovery key gets a real table in the Task 2 schema with Task 3 owning its lifecycle, and recovery bundles carry a fleet-key signature so an agent holding only the recovery public key cannot forge a replica. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b6f0eb5a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await self.state.enter_ingestion_freeze() | ||
| staged = await self.stage_encrypted_snapshot(preflight.target) |
There was a problem hiding this comment.
When the owner enrolls or revokes a machine, or changes listener or integration settings during staging/readiness, this ingestion-only freeze still permits that mutation until enter_draining_transfer() runs. Because the snapshot has already been taken, the target never receives the change and the old hub subsequently demotes, potentially restoring a revoked machine or losing newly enrolled machines and settings. Block fleet mutations as part of the durable pre-snapshot freeze, not only after readiness.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The freeze is now a transfer freeze that rejects fleet mutations (enrollment, revocation, listener and integration changes) as well as report batches from before the snapshot, with a required test covering the whole frozen and draining span. Fixed in 3b2513a.
| staged = await self.stage_encrypted_snapshot(preflight.target) | ||
| announcement = await self.prepare_new_generation(preflight.target) | ||
| await self.target_client.stage_pending_hub(staged, announcement) | ||
| try: |
There was a problem hiding this comment.
If snapshot creation, generation preparation, or stage_pending_hub() fails, execution never enters this try block, so lift_ingestion_freeze() is skipped even though the transfer has not reached draining. The old hub then continues rejecting reports after the failed transfer until an external restart or recovery action; wrap all fallible work after enter_ingestion_freeze() in the abort/cleanup path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Every fallible step after enter_transfer_freeze(), including snapshot creation and generation preparation, now runs inside the cleanup path that lifts the freeze on failure, with abort_pending_hub guarded for the pre-announcement case and a test for a snapshot failure before any target contact. Fixed in 3b2513a.
The pre-snapshot freeze now blocks fleet mutations as well as report ingestion, so the staged snapshot cannot go stale, and every fallible step after entering the freeze sits inside the cleanup path that lifts it, so a failed snapshot no longer strands the old hub rejecting work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b2513a21a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async with transaction(self.database): | ||
| await self.receipts.require_new_or_identical(upload) |
There was a problem hiding this comment.
When a report overlaps transfer startup, it can pass the pre-transaction checks, then enter_transfer_freeze() can commit and the snapshot can be taken before this transaction stores and acknowledges the batch. Task 10 does not modify agent/fleet/ingestion.py or prescribe an equivalent request-draining barrier, so the freeze does not actually guarantee that acknowledgements stop at the snapshot. The fresh evidence beyond the earlier freeze finding is this remaining check/write race in the prescribed ingestion boundary; test the freeze predicate in the same serialized transaction as the sample writes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
require_active and a new require_accepting_reports predicate now run inside the serialized ingestion transaction, with Task 5 introducing the predicate and an overlapping-coroutine race test, so a freeze that commits first is always seen. Fixed in HEAD.
| - [ ] **Step 3: Implement encrypted transfer staging** | ||
|
|
||
| Create a SQLite backup through the backup API. Package fleet config, owner hash, registry, history, integrations, and fleet signing key. Exclude sessions. Encrypt to the target X25519 public key with HKDF-SHA256 and AES-256-GCM. |
There was a problem hiding this comment.
While the old hub is frozen, the selected target is still an agent and buffers its own new samples in its local report_outbox. The transfer creates a backup of the old hub's database and later imports it into target staging, but the plan never excludes the old hub's local-only outbox or merges the target's existing outbox during activation; because that table is in the shared database and has no machine ID, replacing it with the snapshot can permanently discard the target's unacknowledged samples. Require explicit preservation of target-local state and a continuity test where the target collects during staging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Transfer now applies only hub-authority tables: the target's report_outbox, identity, and runtime_state survive activation, the old hub's outbox is excluded from the snapshot and delivered normally after demotion, and Step 1 gains a target continuity test collecting during staging. Fixed in HEAD.
| if announcement is not None: | ||
| await self.target_client.abort_pending_hub(announcement) | ||
| await self.state.lift_transfer_freeze() |
There was a problem hiding this comment.
If staging or readiness fails because the target is unreachable, abort_pending_hub() can raise as well, which skips the following lift_transfer_freeze() and leaves the authoritative hub rejecting reports and fleet changes until an external restart. The fresh evidence beyond the earlier cleanup finding is that the newly widened exception path still places a fallible network cleanup before the local unfreeze; make abort delivery retryable independently while guaranteeing the local cleanup executes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The failure path now enqueues the signed abort durably and lifts the freeze in one local transaction before any network work, with a retry loop delivering the abort and a test where the target is unreachable. Fixed in HEAD.
| async def ingest(self, upload: SignedReportUpload) -> IngestAck: | ||
| await self.machines.require_active(upload.machine_id) | ||
| async with transaction(self.database): |
There was a problem hiding this comment.
When revocation overlaps an upload, the report can pass require_active(), then the owner revocation can commit before this ingestion transaction starts, allowing telemetry from the now-revoked machine to be stored and acknowledged. This violates the stated independent-revocation behavior and is possible because repository transactions share a lock while this check is outside it; move the active-machine predicate into the same transaction as the receipt and sample writes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
require_active moved inside the same serialized transaction as the receipt and sample writes, with a revocation race test. Fixed in HEAD.
Ingestion checks machine-active and accepting-reports inside the serialized transaction so revocation and the transfer freeze cannot race a stored acknowledgement, the failure path enqueues a durable signed abort and lifts the freeze locally before any network work, and transfer explicitly preserves both machines' local report outboxes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dfd00b18e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| preflight = await self.preflight(target_id) | ||
| preflight.require_ready() | ||
| await self.state.enter_transfer_freeze() |
There was a problem hiding this comment.
If the selected machine is revoked after preflight() returns but before enter_transfer_freeze() commits, the revocation is valid and included in the snapshot, yet the transfer continues using the stale ready target and can promote that revoked machine. The freeze prevents later mutations but does not validate predicates checked before it; re-read and require the target to remain active in the same serialized transaction that enters the freeze.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
enter_transfer_freeze() now takes the preflight target and re-reads its machine row inside the freeze transaction, requiring it active, so a revocation between preflight and freeze aborts the transfer. Fixed in HEAD.
|
|
||
| - [ ] **Step 5: Implement create-fleet as one transaction** | ||
|
|
||
| Validate that role is `unconfigured`, request source is loopback, username is nonempty, and password is at least 12 characters. The request also names the first hub listener, chosen from the machine's detected non-loopback private addresses and defaulting to the primary private address with the current port. Reject creation when no private address exists rather than silently creating an unreachable hub. Create fleet identity, owner record, self machine row, the stored hub listener, and runtime role `hub` in one database transaction, then bind the stored listener before the endpoint reports success, so enrollment claims and agent reports work without a restart. |
There was a problem hiding this comment.
When binding fails because the address disappeared or the port is unavailable, this ordering has already committed the installation as hub; a retry is then rejected because it is no longer unconfigured, and a restart can fail on the same unusable listener. Reserve or bind the requested socket before committing the role, or roll the complete setup state back on binding failure, and cover that failure path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Create-fleet now binds and holds the requested socket before the transaction commits. A bind failure returns a visible error with the installation still unconfigured and retryable, with a test for that path. Fixed in HEAD.
| - [ ] **Step 3: Implement Create Fleet and Join Fleet conversions** | ||
|
|
||
| Create Fleet imports history under the stable self machine UUID. Join Fleet converts history to ordered outbox batches and uploads through Task 5. Neither path deletes original runtime files. |
There was a problem hiding this comment.
On an upgraded legacy installation reachable over its existing LAN listener, the new migration router has no stated loopback or bearer-token dependency even though Create Fleet and Join Fleet replace runtime authority and credentials before an owner account exists. The current application protects legacy API routers with require_bearer_token, so Task 13 must explicitly apply that credential (or require an actual loopback peer) to inspection and every conversion mutation, with unauthorized-route tests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Migration inspection and every conversion mutation now require the installation's existing legacy bearer token, matching the protected v1 routers, or an actual loopback peer, with unauthorized-route tests for tokenless LAN requests. Fixed in HEAD.
| - [ ] **Step 5: Implement transport and retention** | ||
|
|
||
| Use exponential reconnect delays of 1, 2, 4, 8, 16, 30 seconds with up to 20 percent jitter. Send a `SignedReportUpload` containing at most 100 report batches or 1 MiB per request. Delete acknowledged outbox rows only after seven days. Persist and upload an exact `GapRange` when local retention removed unacknowledged history. Do not advance the local acknowledgement beyond a missing sequence until the hub acknowledges the persisted gap. |
There was a problem hiding this comment.
Task 5 modifies agent/services/retention.py, but this retention step only specifies outbox deletion; the existing service deletes from the legacy metrics and process_snapshots tables and therefore will not age out the new fleet_metrics or fleet_processes rows. Every hub would retain central telemetry indefinitely instead of the required seven days, so add transactional cleanup and tests for both fleet history tables.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Task 5 retention now purges fleet_metrics and fleet_processes rows older than retention_days transactionally, with tests for both tables. Fixed in HEAD.
The transfer freeze revalidates the target machine in its own transaction, create-fleet binds and holds the socket before committing the hub role, migration routes require the legacy bearer token or a loopback peer, and hub retention ages out the fleet history tables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080bd5c9b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. Promotion creates a new owner password, revokes integrations, switches to hub mode, and publishes a signed announcement at `max(bundle_generation, locally_accepted_generation) + 1`. The second term matters after a hub transfer: the recovering machine may have accepted generation `G+1` through the announcement obligation while its newest bundle is still from `G`, and promoting to `G+1` would be rejected by every agent, including itself, as an equal generation. |
There was a problem hiding this comment.
When recovery runs on a different PC, the bundle's listener settings describe the dead hub's addresses and potentially its machine-local certificate paths, while promote() accepts no listener selection and immediately switches roles and publishes an announcement. The recovering machine may therefore fail to bind or advertise an unreachable old address, preventing agents and the owner from reaching the recovered hub. Require promotion to select, reserve, and persist a listener belonging to the recovering PC and build the announcement from that listener.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
promote() now takes a listener selection from the recovering machine's detected private addresses, binds and holds the socket before any role change (bind failure leaves it an agent), never reuses the dead hub's listeners or certificate paths, and builds the announcement from the newly bound listener. Spec recovery flow updated to match. Fixed in HEAD.
| await self.state.enqueue_signed_abort(announcement) | ||
| await self.state.lift_transfer_freeze() | ||
| raise | ||
| await self.state.enter_draining_transfer(announcement) |
There was a problem hiding this comment.
If enter_draining_transfer() fails after readiness—for example because its journal transaction cannot commit—the old hub remains durably frozen, but this call is outside the try that lifts the freeze and queues an abort. The request therefore exits with report ingestion and fleet mutations still disabled until a restart performs reconciliation. Fresh evidence beyond the earlier post-freeze cleanup finding is that this transition remains after the exception handler; include it in the pre-draining cleanup path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
enter_draining_transfer() moved inside the try block, so a failed draining commit follows the same enqueue-abort-and-lift cleanup as any pre-draining failure. Activation is only requested after draining commits, so everything the cleanup catches remains safely abortable. Fixed in HEAD.
Recovery promotion selects, binds, and announces a listener owned by the recovering PC instead of inheriting the dead hub's addresses, and the draining transition moves inside the transfer cleanup scope so a failed journal commit can no longer strand the freeze. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66271af93d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| claim = await hub_client.claim(address, code, hub_claim) | ||
| record = sign_enrollment_record(hub_claim, claim) | ||
| await repository.store_claimed_attempt(record, address, claim.claim_nonce) | ||
| await hub_client.finalize(address, claim.enrollment_id, claim.claim_nonce, record) |
There was a problem hiding this comment.
When the hub crashes after store_claimed_attempt()—a failure this task explicitly requires supporting—the only durable enrollment data is a SignedEnrollmentRecord, which contains the claim digest but not the agent's signing/encryption public keys, hostname, or addresses. A restarted hub can resend /finalize, but it cannot create the fleet_machines record needed to authenticate the agent's first report. Pass and persist the complete AgentClaim, or atomically create the enrolling machine record, before considering the claim restart-safe.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The claim transaction now stores the complete agent claim JSON and creates the fleet_machines row with status enrolling in the same commit, so a restarted hub can retry finalize and authenticate the first report. Fixed in HEAD.
| - [ ] **Step 5: Implement hub-orchestrated enrollment** | ||
|
|
||
| The hub validates `http` or `https` private addresses and calls claim. Before finalize, it durably inserts the hub claim JSON, signed record JSON, address, agent claim digest, and machine ID in `enrollment_attempts` with state `claimed`. Encrypt the raw claim nonce with AES-GCM under a key derived from the hub machine identity and enrollment ID, then store only nonce, ciphertext, and authentication tag in `claim_nonce_ciphertext`. The unique enrollment ID permits an identical retry and rejects changed bytes. Finalize verifies the fleet signature and agent claim digest, installs fleet state atomically, and changes the attempt to `finalized`. Finalize is idempotent by `enrollment_id` and record digest. The agent consumes the pairing code only after the transaction commits. The hub then marks its attempt finalized and clears the nonce ciphertext. A retry after either process restarts decrypts the nonce and reconstructs the next request entirely from the stored row. The machine remains `enrolling` until Task 5 commits its first signed report. |
There was a problem hiding this comment.
If an enrollment is already claimed when transfer freezing begins, its retry nonce is encrypted to the old hub's machine identity. The snapshot can carry that pending row to the target, but Task 10 deliberately preserves the target's machine identity rather than transferring the old hub's, so the new hub cannot decrypt the nonce and finish finalization; meanwhile the old hub is frozen and later demoted. Reject transfer while claimed attempts exist, finish them before the snapshot, or re-encrypt their resumable state to the target.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The freeze transaction now rejects transfer while any enrollment attempt is claimed. Claims resolve or expire within the ten-minute pairing window, and expired or failed attempts do not block. Fixed in HEAD.
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, exactly like Create Fleet: bind and hold the socket first, fail visibly without changing role when binding fails, and never carry the dead hub's listeners or certificate paths forward. Promotion then creates a new owner password, revokes integrations, switches to hub mode, and publishes a signed announcement built from the newly bound listener at `max(bundle_generation, locally_accepted_generation) + 1`. The second term matters after a hub transfer: the recovering machine may have accepted generation `G+1` through the announcement obligation while its newest bundle is still from `G`, and promoting to `G+1` would be rejected by every agent, including itself, as an equal generation. |
There was a problem hiding this comment.
Recovery imports a registry containing the dead hub's last_acked_sequence but intentionally imports no central history, while Task 5 normally sends only batches beyond the agent's acknowledgement watermark. After promotion, agents therefore regard their retained acknowledged outbox rows as already delivered and the recovered hub's history remains empty instead of being rebuilt. Define a generation-scoped replay/reset handshake that requests every still-retained batch from the oldest recoverable sequence, with idempotency tests for previously acknowledged rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Promotion zeroes imported watermarks and Task 11 adds a watermark endpoint: agents re-read it on generation change and resend every retained batch when the hub's watermark is lower, with Task 5 idempotency absorbing replays. Test added for replaying previously acknowledged rows. Fixed in HEAD.
|
|
||
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, exactly like Create Fleet: bind and hold the socket first, fail visibly without changing role when binding fails, and never carry the dead hub's listeners or certificate paths forward. Promotion then creates a new owner password, revokes integrations, switches to hub mode, and publishes a signed announcement built from the newly bound listener at `max(bundle_generation, locally_accepted_generation) + 1`. The second term matters after a hub transfer: the recovering machine may have accepted generation `G+1` through the announcement obligation while its newest bundle is still from `G`, and promoting to `G+1` would be rejected by every agent, including itself, as an equal generation. |
There was a problem hiding this comment.
If recovery commits the new hub role and generation but the process or response fails while publishes is running, agents remain pointed at the dead hub and this task defines no restart-resumable announcement obligation. Reuse Task 10's durable per-agent publication obligation and commit it atomically with promotion, so startup can retry the exact signed announcement after any interruption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Promotion now persists Task 10's durable per-agent announcement obligation atomically with the role commit, and startup resumes it, so a crash after promotion retries the exact signed announcement. Fixed in HEAD.
| - [ ] **Step 5: Implement create-fleet as one transaction** | ||
|
|
||
| Validate that role is `unconfigured`, request source is loopback, username is nonempty, and password is at least 12 characters. The request also names the first hub listener, chosen from the machine's detected non-loopback private addresses and defaulting to the primary private address with the current port. Reject creation when no private address exists rather than silently creating an unreachable hub. Bind and hold the requested socket before the transaction commits: a bind failure (the address disappeared, the port is taken) returns a visible error with the installation still `unconfigured` and freely retryable, instead of committing a hub role that can never come up on its stored listener. Only with the socket held does the transaction create fleet identity, owner record, self machine row, the stored hub listener, and runtime role `hub`, after which the held socket serves enrollment claims and agent reports without a restart. Cover the bind-failure path with a test proving the role stays `unconfigured`. | ||
|
|
||
| Do not return the printable recovery key as the transaction's only copy. Persist it in the `pending_recovery_key` row (Task 2 schema, service-owned database, same protection class as the owner password hash) inside the same transaction. `GET /api/v2/setup/recovery-key` (owner session, loopback source) returns the printable key for as long as the record exists, and `POST /api/v2/setup/recovery-key/ack` deletes it, which is the only deletion path. A create-fleet response lost before the browser renders the key therefore never strands dead-hub recovery: the owner logs in at the hub PC and retrieves it, then acknowledges. After acknowledgement the private key is unrecoverable from the hub, matching the design's retention rule. |
There was a problem hiding this comment.
Task 2 makes FleetIdentity.create() persist key files, but this transaction can fail after those files are created and before the owner, role, and pending_recovery_key rows commit. The database then remains unconfigured while retry encounters stale fleet identity files, and the corresponding recovery private key has disappeared with the failed request. Stage or explicitly clean up the filesystem identity on rollback, and test a database commit failure after key creation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
FleetIdentity.create() now stages key files in a temporary directory and moves them into place only after the transaction commits, deleting them on rollback, with a commit-failure test proving the identity directory stays clean and a retry succeeds. Fixed in HEAD.
Claims persist the complete agent claim and create the enrolling machine row atomically, transfer rejects while a claimed enrollment is pending, recovery rewinds acknowledgement watermarks through an explicit replay handshake and reuses the durable announcement obligation, and fleet identity files stage until the creation transaction commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3b094a3aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Produces: `UpgradeService.create_fleet(owner: NewOwner) -> UpgradeResult` | ||
| * Produces: `UpgradeService.prepare_join() -> PairingDisplay` |
There was a problem hiding this comment.
Pass listener selections into both migration operations. Unlike the fresh-create and recovery interfaces, these signatures provide no listener input, and Task 13 never says to reserve or persist one before changing roles. An upgraded installation whose legacy listener is loopback-only, a forbidden wildcard, or archived with its legacy security settings can therefore become a hub with no reachable fleet listener, or prepare to join without the restricted private enrollment listener required by Task 4; after the role transition, the normal setup operation cannot simply be retried.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both conversions now take a listener selection: Create Fleet reserves and persists a private hub listener per Task 3's rules, Join Fleet selects Task 4's restricted enrollment listener, both bind before the role transition and fail visibly without changing role. Fixed in HEAD.
|
|
||
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, exactly like Create Fleet: bind and hold the socket first, fail visibly without changing role when binding fails, and never carry the dead hub's listeners or certificate paths forward. Promotion then creates a new owner password, revokes integrations, zeroes every imported `last_acked_sequence`, switches to hub mode, and in the same transaction persists Task 10's durable per-agent announcement obligation for a signed announcement built from the newly bound listener at `max(bundle_generation, locally_accepted_generation) + 1`. Publication runs from that obligation, so a crash or lost response after the role commit retries the exact signed announcement at startup instead of leaving agents pointed at the dead hub. |
There was a problem hiding this comment.
Make persistence of the decrypted fleet signing key crash- and rollback-safe as part of promotion. The database transaction commits the hub role and publication obligation, but the key lives in the filesystem: writing it before the commit can leave a failed promotion's agent holding authoritative key material, while writing it afterward allows a crash to leave a committed hub unable to sign future bundles or transfers, and the recovery key is not retained for startup repair. Use a staged-key journal/finalization flow analogous to fleet creation and cover failures on both sides of the role commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Promotion now stages the decrypted fleet key, journals a reference in the promotion transaction, and finalizes the move afterward. Rollback deletes the staged file, startup repairs a crash between commit and finalization, with tests on both sides of the role commit. Fixed in HEAD.
| await self.samples.insert_upload_if_absent(upload) | ||
| await self.gaps.insert_if_absent(upload.machine_id, upload.gaps) | ||
| highest = await self.samples.highest_contiguous(upload.machine_id) | ||
| await self.machines.mark_reported(upload.machine_id, highest, upload.batches[-1].captured_at) |
There was a problem hiding this comment.
Handle a gap-only upload before indexing the batch list. SignedReportUpload.batches is not constrained to be nonempty, and Step 5 requires an exact persisted GapRange when retention removes unacknowledged history; if transport has only that gap to send, upload.batches[-1] raises instead of committing and acknowledging it, so the agent cannot advance beyond the missing range. Either define and validate that a later batch must accompany every gap or derive mark_reported data without assuming a batch exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Uploads now require at least one batch or gap range, mark_reported uses latest_activity_at() which falls back to the gap's reported time, and a gap-only upload commits and acknowledges end to end with a test. Fixed in HEAD.
Migration conversions take a listener selection with the same bind-before-commit rule as fresh setup, promotion stages the decrypted fleet key through a journaled finalization instead of straddling the role commit, and gap-only uploads validate and acknowledge without assuming a batch exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd3eabbe79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| report_receipts(machine_id, upload_id, payload_digest, sequence_start, sequence_end, highest_contiguous_sequence, accepted_at) | ||
| data_gaps(machine_id, sequence_start, sequence_end, reason, reported_at) | ||
| used_nonces(machine_id, nonce_hash, expires_at) | ||
| enrollment_attempts(enrollment_id, machine_id, agent_claim_digest, hub_claim_json, signed_record_json, claim_nonce_ciphertext, address, state, created_at, finalized_at) |
There was a problem hiding this comment.
The fresh evidence after the earlier thread said this was fixed is that the exhaustive enrollment_attempts schema still has no agent_claim_json column, and Task 4 does not include agent/fleet/schema.py among its modified files. signed_record_json cannot substitute because SignedEnrollmentRecord contains only the claim digest, so after the required crash following store_claimed_attempt(), the hub still cannot reconstruct the agent's signing/encryption keys, hostname, and addresses to finish enrollment. Add the column and migration to the schema task or explicitly modify the schema in Task 4.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd3eabbe79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, exactly like Create Fleet: bind and hold the socket first, fail visibly without changing role when binding fails, and never carry the dead hub's listeners or certificate paths forward. Promotion then creates a new owner password, revokes integrations, zeroes every imported `last_acked_sequence`, switches to hub mode, and in the same transaction persists Task 10's durable per-agent announcement obligation for a signed announcement built from the newly bound listener at `max(bundle_generation, locally_accepted_generation) + 1`. The decrypted fleet signing key follows the same staged-file discipline as fleet creation: write it to a staging path first, have the promotion transaction record a journal reference to that staged key, and move it into the identity location as finalization. A rollback deletes the staged file, so a failed promotion leaves the agent holding no authoritative key material, and a crash between commit and finalization is repaired at startup from the journal reference, so a committed hub can always sign bundles and transfers. Cover failures on both sides of the role commit. Publication runs from that obligation, so a crash or lost response after the role commit retries the exact signed announcement at startup instead of leaving agents pointed at the dead hub. | ||
|
|
||
| Rebuilding history needs an explicit rewind, because the imported registry carries the dead hub's acknowledgement watermarks while the recovered hub imports no central history: agents would treat their retained acknowledged outbox rows as delivered and the history would stay empty. Add `GET /api/internal/v1/reports/watermark` returning the hub's generation and stored watermark for the calling machine. Agent transport re-reads it whenever its accepted generation changes, and when the hub's watermark is lower than the agent's local acknowledgement, the agent resends every still-retained batch from the oldest recoverable sequence. Task 5's upload idempotency makes replayed previously-acknowledged rows harmless. The second term matters after a hub transfer: the recovering machine may have accepted generation `G+1` through the announcement obligation while its newest bundle is still from `G`, and promoting to `G+1` would be rejected by every agent, including itself, as an equal generation. |
There was a problem hiding this comment.
The fresh issue after the earlier rewind fix is that Task 5 deletes acknowledged outbox rows after seven days without recording a gap, while this step only resends from the oldest surviving batch. For a mature agent whose sequences 1–999 were acknowledged and purged, resetting the recovered hub's watermark to zero and replaying from 1000 leaves highest_contiguous_sequence at zero forever, preventing both the replay and future reports from being acknowledged. Generate and persist a recovery-time gap through oldest_recoverable_sequence - 1, or initialize an equivalent disclosed watermark before replay.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4f227b. Before replaying, the agent now persists and uploads a recovery GapRange from the hub watermark plus one through oldest_recoverable_sequence - 1 (skipped when the oldest retained batch is already contiguous), riding Task 5's disclosed-gap path so contiguity advances across the purged prefix. Task 11 Step 1 adds the purged-prefix test.
|
|
||
| - [ ] **Step 5: Implement create-fleet as one transaction** | ||
|
|
||
| Validate that role is `unconfigured`, request source is loopback, username is nonempty, and password is at least 12 characters. The request also names the first hub listener, chosen from the machine's detected non-loopback private addresses and defaulting to the primary private address with the current port. Reject creation when no private address exists rather than silently creating an unreachable hub. Bind and hold the requested socket before the transaction commits: a bind failure (the address disappeared, the port is taken) returns a visible error with the installation still `unconfigured` and freely retryable, instead of committing a hub role that can never come up on its stored listener. Only with the socket held does the transaction create fleet identity, owner record, self machine row, the stored hub listener, and runtime role `hub`, after which the held socket serves enrollment claims and agent reports without a restart. `FleetIdentity.create()` stages its key files in a temporary directory and moves them into place only after the database transaction commits, deleting the staged files on rollback, so a commit failure leaves neither stale identity files nor a vanished recovery key behind a still-`unconfigured` database. Cover the bind-failure path with a test proving the role stays `unconfigured`, and a database commit failure after key creation proving the identity directory stays clean and a retry succeeds. |
There was a problem hiding this comment.
If the process stops after the database commit but before the staged identity files are moved, the installation restarts as hub without its fleet signing key, and Create Fleet cannot be retried because the role is no longer unconfigured. The rollback test only covers commit failure, not this post-commit interruption; journal the staged identity in the transaction and repair it at startup, as the recovery flow intends to do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4f227b. The create-fleet transaction now journals the staged identity path in the new staged_key_journal table (Task 2 schema), and startup repairs any unfinalized row by completing the move before the hub serves anything, the same repair path promotion uses. Step 5 adds a post-commit interruption test proving a restarted process finishes finalization and comes up as a hub with a usable signing key.
|
|
||
| - [ ] **Step 3: Implement local recovery preview and promotion** | ||
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, exactly like Create Fleet: bind and hold the socket first, fail visibly without changing role when binding fails, and never carry the dead hub's listeners or certificate paths forward. Promotion then creates a new owner password, revokes integrations, zeroes every imported `last_acked_sequence`, switches to hub mode, and in the same transaction persists Task 10's durable per-agent announcement obligation for a signed announcement built from the newly bound listener at `max(bundle_generation, locally_accepted_generation) + 1`. The decrypted fleet signing key follows the same staged-file discipline as fleet creation: write it to a staging path first, have the promotion transaction record a journal reference to that staged key, and move it into the identity location as finalization. A rollback deletes the staged file, so a failed promotion leaves the agent holding no authoritative key material, and a crash between commit and finalization is repaired at startup from the journal reference, so a committed hub can always sign bundles and transfers. Cover failures on both sides of the role commit. Publication runs from that obligation, so a crash or lost response after the role commit retries the exact signed announcement at startup instead of leaving agents pointed at the dead hub. |
There was a problem hiding this comment.
Although the earlier thread says promotion journaling was added, the exhaustive Task 2 schema contains no staged-key path or promotion-journal table, and Task 11 does not modify agent/fleet/schema.py. Consequently the promotion transaction has nowhere to record the reference required here, so a crash after committing hub role but before moving the decrypted signing key cannot be repaired on startup. Add the journal storage and migration explicitly to the plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4f227b. Task 2's schema now carries staged_key_journal(journal_id, purpose, staged_path, created_at, finalized_at) with its transaction/repair/finalize semantics spelled out, shared by Create Fleet and promotion. Since schema.py's idempotent SQL is a Task 2 deliverable, Task 11 needs no schema change, and its Step 3 now names the table explicitly instead of a vague journal reference.
| await self.receipts.require_new_or_identical(upload) | ||
| await self.samples.insert_upload_if_absent(upload) | ||
| await self.gaps.insert_if_absent(upload.machine_id, upload.gaps) | ||
| highest = await self.samples.highest_contiguous(upload.machine_id) |
There was a problem hiding this comment.
ReportBatch permits empty metrics and processes lists, which can occur when every collector source fails for a tick, but the schema has no batch-presence table and this flow computes contiguity before recording the receipt. Such a valid batch inserts no sample row, becomes indistinguishable from a missing sequence, and permanently stalls acknowledgements at the preceding sequence. Persist every accepted batch sequence independently, or convert/reject empty batches with an explicit gap before calculating the watermark.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4f227b. Task 2 adds a report_batches presence table (unique on machine_id, sequence) and ingestion inserts one presence row per accepted batch, empty or not, inside the transaction. highest_contiguous_sequence is now computed by extending the persisted watermark through presence rows and disclosed gaps, never by scanning sample rows. Step 1 adds the empty-batch test and Step 5 purges acknowledged presence rows with the other fleet tables.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 316b44c870
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Serialize fleet identity, machine registry, settings, integrations, schema version, recovery epoch, generation, a transactionally incremented positive `bundle_revision`, and timestamp. Bind recovery epoch, generation, and bundle revision into authenticated associated data as well as the encrypted payload. Encrypt with ephemeral X25519, HKDF-SHA256, and AES-GCM to the recovery public key. The encryption alone does not authenticate the sender, because every agent holds the recovery public key and could forge a valid higher-revision ciphertext, so the hub also signs each bundle with the fleet signing key over the recovery epoch, generation, revision, ciphertext digest, and associated data. The epoch rides every layer (payload, associated data, signature, the `recovery_bundles` storage key, and the replica ordering tuple) for the same reason announcements carry it: a promotion forced to rely on its local bundle alone (every peer unreachable) must still mint at one above an epoch the bundle actually proves, and a stale former hub producing fleet-signed bundles at its old epoch must be fenced by ordering, not trusted by revision arithmetic. Agents verify that signature with their retained fleet public key before replacing a replica, and recovery preview verifies it again before decrypting. Copy the ciphertext and signature to enrolled agents through signed internal requests. | ||
|
|
||
| Start `run_replication_loop()` from the hub lifespan. Every tick, including the immediate startup tick, first checks `runtime_state.role` and `runtime_state.transfer_state`: it creates or replicates a bundle only while role is `RuntimeRole.HUB` and `transfer_state` is `accepting`, the same admission rule Task 10's freeze and drain transactions enforce on reports and mutations, so a bundle naming a pre-transfer generation and address set can never be created or handed to an agent mid-transfer. A non-`accepting` `transfer_state` pauses the loop rather than ending it: the tick skips its work and the loop keeps polling, so when a failed transfer's cleanup lifts the freeze back to `accepting` the very next tick resumes periodic bundles and queued delivery rows with no process restart. Only a role change away from `RuntimeRole.HUB` (transfer retirement or, on an agent that never held the role, simply never starting) exits the loop for good, since that database is no longer its to replicate. Create and distribute a bundle immediately on hub startup, after enrollment, revocation, listener or integration changes, and after a completed transfer. Also refresh every six hours. In one transaction, increment `runtime_state.recovery_bundle_revision`, insert the `(recovery_epoch, generation, bundle_revision)` bundle, and create one delivery row per active agent. An unchanged current bundle digest preserves its delivery state. A newer revision creates fresh pending rows, while acknowledgements name recovery epoch, generation, revision, and digest and are accepted only for that exact delivery. Failed deliveries durably update attempt count and next attempt time for retry after 1, 2, 4, 8, 16, then 30 minutes. Startup resumes every pending or due row. Agents persist their accepted `(recovery_epoch, generation, bundle_revision, digest)` and atomically replace a replica only when the incoming authenticated tuple is lexicographically higher on `(recovery_epoch, generation, bundle_revision)`, so a stale former hub's fleet-signed bundles at its old epoch are fenced by the same ordering that fences its announcements. They reject an older or equal tuple with a different digest, retain the newest valid bundle across restart, and return the accepted epoch, generation, revision, and digest so the hub cannot acknowledge stale ciphertext. |
There was a problem hiding this comment.
When two partitioned recoveries create different lineages in the same epoch, both retain the fleet signing key, so the losing lineage can produce a valid bundle with a higher generation or revision that this tuple-only replacement rule accepts. If the winning hub then dies before distributing a newer bundle, recovery resurrects the losing lineage's registry and settings—potentially restoring machines revoked only on the winner—even though announcement handling would have retired that lineage. Bind lineage_id into every authenticated bundle layer and resolve same-epoch cross-lineage bundles with the same deterministic tie-break before comparing generation and revision.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e5d85fd: lineage_id now rides every authenticated bundle layer (serialized payload, associated data, fleet signature input, both storage keys and uniqueness keys, the agent's persisted acceptance tuple), and both replica replacement and recovery selection apply the deterministic tie-break before any tuple comparison, so a same-epoch losing lineage's bundle is rejected or never selected whatever generation or revision it signs, closing the winner-dies-first resurrection path. Retention purging keeps the winning lineage's bundles. Test added for the losing lineage's higher-revision bundle at both replica acceptance and recovery selection. Spec's bundle contents and selection rule updated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5d85fde5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await self.batches.insert_presence_if_absent(upload) | ||
| await self.gaps.insert_if_absent(upload.machine_id, upload.gaps) | ||
| highest = await self.batches.highest_contiguous(upload.machine_id) | ||
| await self.machines.mark_reported(upload.machine_id, highest, upload.latest_activity_at()) |
There was a problem hiding this comment.
When live and oldest-first backfill uploads interleave, a historical upload can arrive after a current one and this call can replace last_seen with the historical batch's older captured_at. Task 6 computes online/stale status from that value, so an actively reporting machine can be shown as stale throughout a long backfill. Update last_seen using the maximum of its existing value and upload.latest_activity_at(), and cover this in the simultaneous live-plus-backfill test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1451e7d: mark_reported now treats last_seen as a monotonic maximum of its existing value and upload.latest_activity_at(), so a historical chunk landing after a live upload can't flip an active machine to stale mid-backfill, and the simultaneous live-plus-backfill test asserts the non-regression.
| sequence_start: int | ||
| sequence_end: int | ||
| batches: list[ReportBatch] | ||
| gaps: list[GapRange] = Field(default_factory=list) |
There was a problem hiding this comment.
When an agent has more than the 100-batch upload limit queued, sequence_end identifies only the end of the current oldest-first chunk; the upload carries neither the newest local sequence nor the total pending count. The hub therefore cannot produce Task 6's promised backfill newest local sequence and pending count from its v2 tables, and the dashboard will under-report or omit progress during an outage recovery. Add signed outbox high-water/pending metadata to the upload and persist it for availability queries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1451e7d: SignedReportUpload gains outbox_high_water, stamped at build time from the agent's persisted sqlite_sequence high-water mark and riding inside the signed payload. The hub persists it monotonically in the new fleet_machines.outbox_high_water column, and Task 6's backfill object computes newest local sequence and pending count from these columns instead of chunk edges.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1451e7d198
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| The target imports into staging, inserts a `pending_hub_authority` row (Task 2 schema) keyed on the announcement digest with the staged snapshot and key paths, persists the signed proposed generation, initializes that generation's `recovery_bundle_revision` to zero, and sets `runtime_state.role` to `RuntimeRole.PENDING_HUB` for unannounced hub mode. An import assertion compares the target's own `sqlite_sequence` row for `report_outbox` before and after the import and fails loudly if it changed, catching a packaging regression on the old hub's side before it can silently rewind the target's allocator. It does not start on the imported listener settings: the snapshot's `fleet_settings` row carries the old PC's addresses and machine-local certificate paths, which the target may be unable to bind at readiness or after its next restart. The owner's preflight-validated `ListenerSelection` from the target's own detected private addresses travels with `stage_pending_hub`, and the target secures that socket before `commit_unannounced_hub`: when the selection names an address the target's `ListenerManager` already serves (the normal case, since Task 4 keeps the enrolled agent's listener bound for announcement and recovery routes), staging reuses that existing bound socket and atomically widens its route set at commit instead of binding the same address a second time, which would fail address-in-use on every ordinary transfer. Only a selection naming a genuinely new address is freshly bound and held, then attached through Task 3's `ListenerManager` when unannounced hub mode starts, so readiness probes reach the staged hub without a restart, exactly like Create Fleet and recovery: a bind failure on such a new address fails staging visibly with the target still an agent, which the old hub's cleanup path catches and aborts with the freeze lifted. The import replaces the machine-local listener and certificate settings with the selection, `prepare_new_generation` builds the announcement from that same selection, and the announcement never advertises the old hub's addresses. The import otherwise applies only hub-authority tables. | ||
|
|
||
| The transferred fleet signing key follows the same staged-file discipline as fleet creation and recovery: the target decrypts it to a staging path during import, never into the identity location, and records that path in `pending_hub_authority.staged_key_path`. Holding staged or unannounced authority never makes the key usable (readiness verifies reports with the fleet public key and the announcement was signed by the old hub, so nothing before activation needs the private key). The target's activation transaction, the one that persists the publication obligation, sets `pending_hub_authority.state` to `activated`, flips `runtime_state.role` to `RuntimeRole.HUB`, installs the transfer announcement as the target's own accepted announcement and floor evidence (`hub_announcement_json`, `floor_announcement_json`, and `generation_floor` advance together, so the target's probe endpoint and any later enrollment serve the new authority rather than the old hub's), and inserts a `staged_key_journal` row referencing the staged key, and finalization moves it into the identity location after commit, stamping the row finalized. That same activation transaction also deletes every `owner_sessions` row on the target: the target may itself have been a hub earlier (a prior transfer moved authority away from it), and an old cookie from that earlier tenure must not authenticate against the new fleet the moment this machine becomes authoritative again. A signed abort must name the announcement digest it is aborting: the target checks it against `pending_hub_authority.announcement_digest` and rejects (with a status the old hub uses to drop the queue row rather than retry) an abort for a digest it never staged or one whose `pending_hub_authority.state` is already `activated`, so a late-delivered abort from a failed earlier transfer can never tear down a different, successfully activated authority for the same machine. An abort that does match a still-staged digest must undo the listener surgery staging performed, not only the staged files: the `pending_hub_authority` row records the pre-transfer `fleet_settings` listener state in `prior_settings_json` when staging first replaces it, and the abort transaction restores that saved state, detaches any socket the transfer newly attached, narrows a reused agent socket back to its post-enrollment route allowlist, and disables the pending-hub readiness routes in the running process, so the target answers announcement and recovery pushes on the address the hub's registry still advertises rather than binding only the abandoned transfer address after its next restart. The same transaction deletes the staged key file, the staged snapshot, and the `pending_hub_authority` row, and returns `runtime_state.role` to `RuntimeRole.AGENT`, so an aborted target resumes agent mode holding no authoritative key material. A crash between the activation commit and the key move is repaired by the Task 3 startup journal repair, so an activated hub can always sign announcements, bundles, and future transfers. None of this replaces the target's machine-local state: the target is still a collecting agent during staging, so its own `report_outbox`, machine identity, and `runtime_state` row survive activation untouched, and the samples it buffered while the old hub was frozen are ingested by its own hub service after the switch through Task 5's role-entry outbox drain, which runs at activation and sweeps every unacknowledged row oldest first before live self-ingest begins (agent transport stops in `hub` role and live self-ingest only handles newly collected batches, so without the drain those staged-period rows would sit in the outbox until retention destroyed them). The snapshot side has the mirror rule: the old hub's local-only `report_outbox` rows are excluded from the transferred payload because they belong to the old machine, which keeps them and delivers them to the new hub as a normal agent after demotion. Readiness must fetch the real static login page, prove an invalid login gets the expected generic `401`, and submit a valid signed report under the committed proposed generation, but that report check is non-committing by design: the old hub constructs a dedicated probe payload rather than handing over a real outbox batch, the target runs the full ingestion path (signature, admission, inserts, contiguity) inside a transaction it rolls back deliberately, and the result leaves the target as a check digest, never as an `IngestAck` any agent outbox could consume. Nothing about the probe persists, so a draining commit failure that aborts the still-staged target discards no acknowledged sample and no machine sequence was ever consumed, with a test asserting the staged database is byte-identical before and after the readiness probe. The target returns a signed readiness receipt containing the generation and each check digest. |
There was a problem hiding this comment.
When the transfer target has used Task 11's manual adoption, its accepted announcement can be below its retained floor while convergence is still pending. This activation replaces floor_announcement_json and generation_floor with the proposed transfer announcement instead of taking the lexicographic maximum; if that announcement is below the target's floor, activation lowers the anti-replay floor—or leaves it inconsistent with floor_recovery_epoch—so later verification can admit stale authority or reject the target's own floor evidence. Preserve the prior floor and evidence unless the transfer pair is strictly higher, and test a transfer immediately after lower-generation manual adoption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e68b17: transfer activation now advances the floor under the same never-lower rule every floor write obeys. The floor columns and floor_announcement_json move to the transfer announcement's pair only when it is lexicographically above the target's retained floor, otherwise the prior floor and evidence stay untouched, keeping floor_recovery_epoch consistent. Added the test: transfer to a target immediately after a lower-pair manual adoption, retained floor survives activation. Also made Task 4's finalize floor write explicitly never-lower for a rejoining machine that retains a higher floor from its earlier tenure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e68b1718d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| class IngestAck(BaseModel): | ||
| highest_contiguous_sequence: int |
There was a problem hiding this comment.
In the default HTTP deployment, this bare acknowledgement is not authenticated by the hub: request signing proves the agent to the hub, but it provides no integrity for the response. An on-path LAN host can suppress an upload and reply with an arbitrarily high highest_contiguous_sequence, causing the agent to mark unsaved samples acknowledged and eventually delete them during retention. Return a hub-signed acknowledgement bound to the upload ID and payload digest, and verify it before advancing the local watermark.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: IngestAck is now hub-signed over the echoed upload_id, payload_digest, and watermark, the ingestion snippet returns sign_ingest_ack(...), and the agent verifies the signature against its persisted runtime_state.hub_signing_public_key plus both echoed identifiers against the upload it sent before advancing anything, treating any mismatch like a failed upload. Added the forgery test (suppressed upload, fabricated high-watermark ack, outbox advances nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d51717193
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stored = await self.receipts.stored_ack_or_reject_conflict(upload) | ||
| if stored is not None: | ||
| return stored |
There was a problem hiding this comment.
Fresh evidence after the acknowledgement-authentication fix is that an identical retry returns here before sign_ingest_ack runs. When the original response is lost after the receipt commits, report_receipts contains the identifiers and watermark but no signature, so returning the persisted value either violates the new IngestAck model or gives the agent an unverifiable acknowledgement and leaves the upload retrying indefinitely; persisting the old signature would also fail after a hub transfer rotates the authorized hub key. Reconstruct and sign the stored tuple with the current hub machine key before returning it, while retaining the receipt's original watermark.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: receipts persist only the tuple, and the identical-retry path now reconstructs and re-signs (upload_id, payload_digest, highest_contiguous_sequence) with the current hub machine key at response time, keeping the receipt's original watermark. The snippet's early return goes through sign_ingest_ack too. Added the cross-transfer retry test: ingest commits, response lost, fleet transfers, retried upload's ack verifies under the new hub's key with the original watermark.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ce750ad2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Serialize fleet identity, machine registry, settings, integrations, schema version, `lineage_id`, recovery epoch, generation, a transactionally incremented positive `bundle_revision`, and timestamp. Bind `lineage_id`, recovery epoch, generation, and bundle revision into authenticated associated data as well as the encrypted payload. Encrypt with ephemeral X25519, HKDF-SHA256, and AES-GCM to the recovery public key. The encryption alone does not authenticate the sender, because every agent holds the recovery public key and could forge a valid higher-revision ciphertext, so the hub also signs each bundle with the fleet signing key over the `lineage_id`, recovery epoch, generation, revision, ciphertext digest, and associated data. The epoch rides every layer (payload, associated data, signature, the `recovery_bundles` storage key, and the replica ordering tuple) for the same reason announcements carry it: a promotion forced to rely on its local bundle alone (every peer unreachable) must still mint at one above an epoch the bundle actually proves, and a stale former hub producing fleet-signed bundles at its old epoch must be fenced by ordering, not trusted by revision arithmetic. Agents verify that signature with their retained fleet public key before replacing a replica, and recovery preview verifies it again before decrypting. Copy the ciphertext and signature to enrolled agents through signed internal requests. | ||
|
|
||
| Start `run_replication_loop()` from the hub lifespan. Every tick, including the immediate startup tick, first checks `runtime_state.role` and `runtime_state.transfer_state`: it creates or replicates a bundle only while role is `RuntimeRole.HUB` and `transfer_state` is `accepting`, the same admission rule Task 10's freeze and drain transactions enforce on reports and mutations, so a bundle naming a pre-transfer generation and address set can never be created or handed to an agent mid-transfer. A non-`accepting` `transfer_state` pauses the loop rather than ending it: the tick skips its work and the loop keeps polling, so when a failed transfer's cleanup lifts the freeze back to `accepting` the very next tick resumes periodic bundles and queued delivery rows with no process restart. Only a role change away from `RuntimeRole.HUB` (transfer retirement or, on an agent that never held the role, simply never starting) exits the loop for good, since that database is no longer its to replicate. Create and distribute a bundle immediately on hub startup, after enrollment, revocation, listener or integration changes, and after a completed transfer. Also refresh every six hours. In one transaction, increment `runtime_state.recovery_bundle_revision`, insert the `(recovery_epoch, generation, bundle_revision)` bundle, and create one delivery row per active agent. An unchanged current bundle digest preserves its delivery state. A newer revision creates fresh pending rows, while acknowledgements name recovery epoch, generation, revision, and digest and are accepted only for that exact delivery. Failed deliveries durably update attempt count and next attempt time for retry after 1, 2, 4, 8, 16, then 30 minutes. Startup resumes every pending or due row. Agents persist their accepted `(lineage_id, recovery_epoch, generation, bundle_revision, digest)` and replace a replica under the same discipline announcements use, tie-break first, then ordering: a same-epoch bundle from a different lineage is admitted to the tuple comparison only when its lineage wins the deterministic tie-break against the retained one (a losing lineage's bundle is rejected whatever its generation or revision, because both partitioned recoveries hold the fleet signing key and the loser can sign arbitrarily high tuples), and an admitted bundle replaces atomically only when lexicographically higher on `(recovery_epoch, generation, bundle_revision)`, so a stale former hub's fleet-signed bundles at its old epoch are fenced by ordering and a same-epoch loser's by lineage, exactly like its announcements. Without the bundle-side tie-break, a winner hub dying before its next replication would let recovery resurrect the losing lineage's registry and settings, including machines revoked only on the winner. They reject an older or equal tuple with a different digest, retain the newest valid bundle across restart, and return the accepted epoch, generation, revision, and digest so the hub cannot acknowledge stale ciphertext. |
There was a problem hiding this comment.
Fresh evidence beyond the earlier bundle tie-break fix is that replacement compares an incoming bundle only with the retained bundle, not with the agent's accepted announcement. If an agent stores lineage B's bundle before convergence, later accepts same-epoch winning lineage A's announcement, and A dies before its next replication, the agent still has only B's bundle; recovery preview has no competing A bundle against which to apply the tie-break and can promote the known loser, restoring B's stale registry and settings, including machines revoked on A. Reconcile retained bundles whenever an announcement changes lineage, or have preview reject a same-epoch bundle whose lineage loses to the locally accepted/floor announcement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: preview now applies the lineage tie-break against announcements, not just competing bundles. A same-epoch bundle whose lineage loses against the machine's accepted announcement, floor evidence, or any verified probe response is refused automatically with the reason stated, covering exactly your holds-only-B's-bundle case. One escape hatch matches the manual-adoption pattern: when no winning-lineage bundle survives anywhere, the owner may explicitly confirm promotion from the losing bundle at the loopback page, safe against split brain since promotion mints the next epoch above both lineages either way, the confirmation only accepts the loser's stale registry as the best data left. Test added for refuse-then-explicit-confirm.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aaef5e7e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - [ ] **Step 3: Implement versioned scrypt and opaque sessions** | ||
|
|
||
| Use a random 32-byte session token and store only `SHA256(token)`. Return the raw token only in the cookie. Generate a separate CSRF token, store only its digest, and return the raw value from `/api/v2/session/me`. |
There was a problem hiding this comment.
After a successful login, the response is 204 and exposes only the session cookie, while the server persists only SHA256(CSRF token). Consequently, /api/v2/session/me cannot reconstruct the raw CSRF token that Task 7 requires the browser to send, especially after a restart or from another worker, so every state-changing dashboard request will fail CSRF validation. Store or derivably regenerate the token, return it during login, or define an atomic /me rotation protocol.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: login now returns the raw CSRF token in a 200 JSON body, and /api/v2/session/me is a rotation protocol, minting a fresh token, atomically replacing csrf_hash, and returning the new raw value, so hash-only storage survives restarts and other workers. A stale tab's distinguishable 403 triggers one /me re-read and retry. Tests cover the rotation chain and the restart-between-login-and-first-write case.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 914269d3b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| record = sign_enrollment_record(hub_claim, claim) | ||
| await repository.store_claimed_attempt(record, address, claim.claim_nonce) | ||
| announcement = current_hub_announcement() | ||
| await hub_client.finalize(address, claim.enrollment_id, claim.claim_nonce, record, announcement) |
There was a problem hiding this comment.
After the finalize interface update, this canonical flow still creates SignedEnrollmentRecord before fetching the announcement and does not pass an announcement to sign_enrollment_record, even though the record requires that announcement's digest. The helper therefore either cannot populate hub_announcement_digest or must independently read mutable current state; if an endpoint or authority change lands before line 628, the record pins one announcement while finalize carries another and enrollment is rejected. Fetch the announcement first and pass that same object into both record signing and finalize.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: enroll_machine fetches the announcement before signing, passes it into sign_enrollment_record(hub_claim, claim, announcement), and sends that same object to finalize, so the record's hub_announcement_digest always pins exactly what finalize carries and a mid-flow authority change just fails cleanly at signing time instead of desynchronizing.
| "username": "static", | ||
| "password": owner_password, | ||
| }) | ||
| assert response.status_code == 204 |
There was a problem hiding this comment.
When Task 3 implements the newly specified login contract, this prescribed test still requires the obsolete bare 204, while Step 3 requires a 200 JSON response containing the raw CSRF token. Following the example either makes the focused suite reject the correct implementation or encourages an implementation that omits the token and leaves subsequent browser mutations unable to pass CSRF validation; assert 200 and the returned token instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: the login test now asserts 200 with a returned csrf_token alongside the cookie attribute assertions, matching Step 3's contract.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30a2d7c642
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Preview reports bundle age, generation, known PCs, and the oldest recoverable local sample, and states the bundle's creation timestamp with an explicit line that any PC enrolled after that time is not in this registry and must be re-enrolled after promotion rather than expected to reconnect on its own: the bundle can only carry the machines the hub knew about when it last replicated. The recovered hub's own rejection of a report from an unregistered machine returns a status distinguishable from an ordinary signature or revocation failure (`unknown_machine`, not `unauthorized`), and that unregistered agent's local recovery page surfaces it plainly with the Task 4 Rejoin Fleet action as the stated remedy, since silently retrying forever would look identical to a network problem to the owner watching it. Preview also probes the generation actually accepted across the fleet, because announcement delivery can be partial before a hub dies: after a transfer, machine A may have accepted `G+1` while the recovering machine and its newest bundle are both still at `G`, and promoting at `G+1` would be rejected by A as an equal generation, leaving it pointed at the dead hub forever. Add `GET /api/internal/v1/announcement` on every fleet member, returning the machine's currently accepted signed hub announcement verbatim from `runtime_state.hub_announcement_json` (Task 2 schema), which acceptance writes atomically and which therefore survives restart: a machine that accepted a partially delivered `G+1` and rebooted still answers the probe with the exact `G+1` object. The response needs no additional authentication: the announcement is fleet-signed, so the prober verifies each returned announcement with its retained fleet public key and an attacker without the fleet key can neither forge a higher generation nor gain anything by replaying a lower one. The probe response carries two fleet-signed objects rather than any bare number: the machine's accepted announcement (`hub_announcement_json`) and its floor evidence announcement (`floor_announcement_json`), which differ only on a machine that manually adopted a lower generation. Both authenticate themselves under the fleet key, so the prober needs no trust in the responder at all: a LAN host or compromised machine can replay only announcements the fleet key actually signed, and no fabricated scalar, however large, ever reaches the generation formula. The storage-ceiling guard now lives where increments actually happen: promotion increments only the epoch (generation restarts at 1), so preview rejects an epoch whose increment would leave the storage-safe integer range, and the within-epoch generation increments (endpoint-change mints, transfer mints, convergence bumps) each reject a generation increment that would leave it, failing loudly rather than minting an unpersistable value. Preview applies the lineage tie-break against announcements, not merely against competing bundles, because an agent can be holding only the losing lineage's bundle: it stores lineage B's replica before convergence, then accepts winning lineage A's announcement, and A dies before its first replication ever reaches it, leaving no A bundle for the bundle-versus-bundle tie-break to prefer. A same-epoch bundle whose lineage loses the tie-break against the lineage of this machine's own accepted announcement, its floor evidence announcement, or any verified probe response is therefore refused automatically, whatever tuple it carries, with preview stating plainly that this bundle belongs to a retired lineage and that promoting it would resurrect a superseded registry (including machines revoked on the winner). One escape hatch matches the manual-adoption pattern: when no winning-lineage bundle exists anywhere, the owner may explicitly confirm promotion from the losing bundle at the machine's loopback recovery page, which is safe against split brain because promotion mints the next epoch above both lineages either way, and the confirmation is only accepting the loser's stale registry as the best data that still exists. Test it: retain B's bundle, accept A's announcement, kill A, and prove preview refuses automatically, states the reason, and proceeds only through the explicit confirmation. Preview probes every address in the imported registry, keeps the highest generation across every verified announcement in every response, and lists the machines that did not respond with the warning that an unreachable machine which accepted a newer generation must be re-pointed through the manual hub address entry on its local recovery page after promotion. The bundle's listener settings describe the dead hub's addresses and possibly its machine-local certificate paths, none of which belong to the recovering PC, so promotion requires a listener selection from this machine's detected private addresses, with the bind-before-commit discipline of Create Fleet adjusted for the machine promotion actually runs on: the promoting PC is normally an enrolled agent whose own Task 4 private listener already owns the selected address and port, so a naive fresh bind would fail address-in-use against the machine's own socket and leave recovery unusable. The selection therefore goes through Task 3's `ListenerManager` the way Task 10's transfer target does: an already-attached compatible socket counts as held (Uvicorn owning it is the proof it binds, the same rule Task 9's save-time validation applies), only a genuinely new endpoint is bound and held fresh, and a bind failure on a new endpoint fails visibly without changing role. The promotion commit then attaches any newly held socket so the recovered hub serves without a restart (a crash before the attach is repaired by hub startup binding the stored listener), and the dead hub's listeners and certificate paths are never carried forward. Promotion then creates a new owner password, deletes every `owner_sessions` row already present on this machine (it may have been a hub before, or briefly held transfer authority, and an old cookie from either must not authenticate against the recovered fleet), revokes integrations, zeroes every imported `last_acked_sequence`, switches to hub mode, and in the same transaction persists Task 10's durable per-agent announcement obligation for a signed announcement built from the newly bound listener, installing that same announcement as this machine's own accepted announcement and floor evidence (`hub_announcement_json`, `floor_announcement_json`, `recovery_epoch`, `hub_generation`, and the complete floor pair `floor_recovery_epoch` and `generation_floor` all advance atomically with the role change, the floor columns always matching the pair the evidence announcement proves, so the probe and manual adoption serve the recovered hub's announcement, never the dead hub's, the watermark verifier never sees a floor claim its evidence contradicts, and the above-floor rule cannot admit a high-generation announcement from the fenced old epoch through a stale `floor_recovery_epoch`), at recovery epoch one above the highest epoch carried by the bundle or by any verified probe response (the single epoch-incrementing path in this design, fencing the dead lineage's key-retaining hub from ever minting acceptable authority again) and generation 1, because the fresh epoch outranks every pair any machine holds lexicographically, so no generation arithmetic against the dead lineage is needed or wanted: the old formula's max over bundle, floor, and probed generations mixed epochs, could inflate the new lineage from obsolete evidence, and could refuse recovery outright when a dead lineage's generation sat at the storage ceiling, while a per-epoch restart at 1 is strictly higher than all of them under the pair rule. The bundle, the promoting machine's own durable floor pair, and every verified probe response (re-run at promotion time) still participate, but only through the epoch term: the minted epoch is one above the highest epoch any of those sources proves. The decrypted fleet signing key follows the same staged-file discipline as fleet creation: write it to a staging path first, have the promotion transaction insert a `staged_key_journal` row (Task 2 schema, so no schema change is needed here) referencing that staged key, and move it into the identity location as finalization, stamping the row finalized. A rollback deletes the staged file and the journal row rolls back, so a failed promotion leaves the agent holding no authoritative key material, and a crash between commit and finalization is repaired by the Task 3 startup journal repair, so a committed hub can always sign bundles and transfers. Cover failures on both sides of the role commit. Publication runs from that obligation, so a crash or lost response after the role commit retries the exact signed announcement at startup instead of leaving agents pointed at the dead hub. | ||
|
|
||
| Rebuilding history needs an explicit rewind, because the imported registry carries the dead hub's acknowledgement watermarks while the recovered hub imports no central history: agents would treat their retained acknowledged outbox rows as delivered and the history would stay empty. Add `POST /api/internal/v1/reports/watermark`, a Task 4 signed internal request whose body carries the calling machine's accepted `(recovery_epoch, generation)` pair, its floor pair `(floor_recovery_epoch, generation_floor)`, and the fleet-signed floor evidence announcement from `floor_announcement_json` (the envelope's body digest authenticates the transport, and a tampering test proves a modified body fails verification), returning the hub's generation and stored watermark for that machine. The convergence rule consumes the evidence, not the scalar, and it bumps only for unresolved authority: the hub verifies the evidence announcement with the fleet key, requires its `(recovery_epoch, generation)` pair to equal the claimed floor pair, and resolves same-epoch conflicts by lineage rather than generation racing, because two machines recovering the same dead fleet while partitioned from each other can both legitimately mint the same fresh epoch. Evidence at the hub's own epoch naming a different hub lineage, at any pair equal to or above its own, invokes a deterministic tie-break: the lineage whose `lineage_id` is lexicographically smaller wins, a property minted at promotion time and carried verbatim through every same-lineage mint and transfer, so neither minting further generations nor transferring the role to another machine can change it. Different lineage means a differing `lineage_id`. A hub that loses the tie-break stops publishing and retires through exactly the fenced stale-epoch path (surfaced on its local recovery page with Rejoin as the remedy), and a hub that wins mints its fresh self-announcement at its own epoch and the evidence generation plus one, publishing through the normal obligations so every machine converges, including those that had accepted the losing lineage. The loser retiring instead of answering back is what makes leapfrog oscillation impossible. Same-lineage evidence strictly above the hub's own pair at its own epoch also mints at evidence generation plus one. Evidence carrying a strictly higher epoch than the hub's own is the fenced-hub case: the hub does not bump past it, it stops publishing and retires as Task 9's rejection handling describes. Evidence at a lower epoch, whatever its generation, is settled history and mints nothing, which is exactly the case the scalar floor got wrong: an old lineage's generation 100 reported to a recovered hub at `(2, 2)` must not drive it to 101. Evidence that is simply the hub's own current announcement is the healthy steady state after every ordinary enrollment, transfer, or recovery, and mints nothing, because bumping on equality would loop forever (each accepted bump re-triggers the watermark re-read at a floor equal to the new generation, climbing straight to the storage ceiling). The rule still rejects any floor whose increment would leave the storage-safe integer range. A compromised machine signing an enormous or maximal floor without fleet-signed evidence changes nothing, and a floor claim above its evidence is discarded, with tests for both. Agent transport re-reads it whenever its accepted generation changes and after every enrollment finalize, including a Rejoin: a machine missing from the recovery bundle re-enrolls against a hub whose announcement it may already have adopted, so the generation need not change while its fresh registry row still starts at watermark zero, and without the finalize trigger its retained acknowledged batches would never replay and contiguity would sit at zero forever. When the hub's watermark is lower than the agent's local acknowledgement, the agent resends every still-retained batch from the oldest recoverable sequence. `oldest_recoverable_sequence` is defined as the oldest surviving `report_outbox` row's sequence when the outbox retains at least one row, and as `last_allocated_sequence + 1` (the persisted `sqlite_sequence` high-water mark for `report_outbox`) when the outbox is completely empty, which Task 5's retention path makes possible for any agent that has been fully caught up and idle past the purge window. Without that empty-outbox case, an agent with nothing retained has no batch to anchor a gap disclosure on and its acknowledgement would never resume after promotion. The replay must first disclose what retention already destroyed: Task 5 deletes acknowledged outbox rows after seven days, so a mature agent's oldest surviving batch usually starts well above the recovered hub's zeroed watermark, and replaying from there alone would leave `highest_contiguous_sequence` stuck at zero with nothing ever acknowledged again. Before the replay, the agent persists and uploads a recovery `GapRange` from the hub watermark plus one through `oldest_recoverable_sequence - 1` (skipped entirely when the oldest retained batch is already contiguous with the hub watermark, and covering the whole prior range up to the high-water mark in the empty-outbox case), riding the same disclosed-gap path Task 5 built for retention, so contiguity advances across the purged prefix and the replayed batches acknowledge normally. Task 5's upload idempotency makes replayed previously-acknowledged rows harmless. |
There was a problem hiding this comment.
In the default plain-HTTP deployment, request signing authenticates only the agent-to-hub request; this response is an unsigned bare generation and watermark. An on-path host can suppress the request and return a watermark at or above the agent's local acknowledgement, causing the agent to skip the one-time rewind that is triggered only after generation changes or finalization. The recovered hub then remains at watermark zero, so retained history is not restored and subsequent batches cannot become contiguous. Return and verify a hub-signed response bound to the request and current authority, as the plan already requires for IngestAck.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the new head: the watermark response is now hub-signed over the requesting machine ID, the request nonce, the hub's current authority pair, and the watermark, verified against the persisted hub_signing_public_key before the agent acts, with mismatches retried as failed requests, the same discipline as IngestAck. Test added proving a forged high-watermark response cannot suppress the rewind.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d21e4c4fe5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - [ ] **Step 3: Implement versioned scrypt and opaque sessions** | ||
|
|
||
| Use a random 32-byte session token and store only `SHA256(token)`. Return the raw token only in the cookie. Generate a separate CSRF token and store only its digest, which means the server can never hand back the original raw value later, so retrieval is rotation: login returns the raw CSRF token in its response body (a `200` with a JSON body, not a bare `204`), and every `/api/v2/session/me` call mints a fresh CSRF token, atomically replaces `csrf_hash` on the session row, and returns the new raw value, so a fresh page load or a post-restart worker always ends up holding a token whose digest the database currently has. A second tab that rotated the token out from under an older tab costs one round trip, not a login: a state-changing request failing CSRF validation returns a distinguishable `403`, and the client re-reads `/me` once and retries with the fresh token. Test the rotation chain (login token works, `/me` rotates it, the old token's `403` triggers exactly one re-read and the retry succeeds) and test that a hub restart between login and the first state-changing request still validates after a `/me` read. |
There was a problem hiding this comment.
When two tabs sharing the same session call /me concurrently, the later transaction immediately invalidates the token returned to the first tab; that tab's mutation then gets 403, and its one recovery read rotates the token again, invalidating the second tab in turn. Active tabs can therefore repeatedly make each other's state-changing requests fail despite both having valid sessions. Keep multiple bounded CSRF hashes, issue per-tab tokens, or derive a stable token instead of replacing the sole session-wide hash on every /me call.
Useful? React with 👍 / 👎.
1 participant
What changed
Why
HumWatch 2.0 regressed day-to-day access by making each PC a separate token and certificate problem. The 2.1 design restores one authenticated web hub reachable by IP and port over ordinary LAN or optional Tailscale, while keeping every enrolled machine capable of taking over as hub.
Validation
.venv/bin/python -m pytest -q: 241 passed, 5 skippedgit diff --check: cleanImplementation handoff
After this PR merges, start Task 1 from fresh
origin/mainin a new worktree. Task 1 closes #14 by preserving and proving the real 2.0 SQLite database before any fleet migration.