Defect
phlo operations maintenance apply and POST /api/continuity/apply journaled every maintenance run under the fixed id <operation>:<table>:<ref> with the hard-coded subject operator. Consequences:
- A second apply of the same operation on the same table and ref failed with
conflicting_claim, even under a fresh, still-current plan. In practice a table could be compacted once per journal directory. - Journal entries were not attributed to the principal that authorised the mutation, which the support manifest listed as an open gap for
maintenance_operations.
Change
phlo.operations.maintenance_operation_id(operation, table, ref, plan_token)is the single canonical journal id,<operation>:<table>:<ref>:<plan token>. The CLI and HTTP plan/apply paths all use it, so the verification handle returned by/api/continuity/planmatches the entry written by apply. This mirrors the id shape the Dagster maintenance assets already use (…:<run_id>) and the plan-digest ids used by restore and upgrade.- The CLI records
CliPrincipalResolver.resolve().subjectinstead of"operator"; the HTTP path already used the authenticated subject. - Support manifest: the
maintenance_operationsreason no longer lists this gap and describes the new behaviour; the packaged copy undersrc/phlo/support_data/is kept byte-identical.
Behaviour: a fresh plan for the same table applies again; re-applying the same plan token still conflicts with its own entry, and an UNKNOWN outcome still blocks replay under a new idempotency key.
Tests
tests/cli/test_operations_maintenance.py: two plans on the same table both succeed, entries carry the resolved principal and plan-bound ids, and a repeat of the same plan is refused withconflicting_claim.packages/phlo-api/tests/test_continuity_api.py: the apply response id equals the plan id; a second plan on the same table applies; the UNKNOWN verification handle uses the plan-bound id.- Local:
pytest tests/cli packages/phlo-api/tests tests/tooling→ 2542 passed, 1 skipped; ruff check/format clean;scripts/validate_support_manifest.pyvalidated.
Activity
311
phlo-agent[bot]6h agoReview at a13c153
The CLI half is sound: maintenance_operation_id binds the journal entry to the plan, the resolved principal replaces the hard-coded "operator", and both journal stores still reject any active claim for the same (action, target), so an UNKNOWN outcome keeps blocking a new plan. The two support-manifest copies are byte-identical and PR Validation is green at this SHA.
1. HTTP compaction plans still collapse to one journal id — packages/phlo-api/src/phlo_api/api/continuity.py:345-349, :434-437
The API passes the plan token through verbatim (plan.get("plan_token", "")). A compaction plan has no plan_token, only before_revision (packages/phlo-iceberg/src/phlo_iceberg/resource.py:1221-1243); the CLI promotes that revision to the token (src/phlo/cli/commands/operations.py:536-539), the API does not.
/api/continuity/plantherefore returnsoperation_id: "compact:<table>:<ref>:None"andplan_token: null. Apply recomputes the same...:None, so two compaction plans at different revisions still share one id and the second is refused withconflicting_claim— the defect fixed for the CLI persists over HTTP.- The same plan now gets a different id from the CLI (
...:<revision>) and the API (...:None), so the "single canonical journal id" is not canonical across surfaces.
Normalise the token once and use it for both the response and the id (for example str(plan.get("plan_token") or plan.get("before_revision") or "")), or reject a plan with no token. The new API test misses this because FakeExecutor.plan() always returns maintenance-token-1 and the "fresh" apply uses a hand-written token-2 instead of a second /plan response.
2. The plan-handle/apply-entry equality only holds when the apply request repeats ref — continuity.py:170, :434-437
ContinuityApplyRequest.ref defaults to "main", so ref = request.ref or plan.get("ref", "main") never reaches the plan. A plan issued with ref: "feature" returns compact:t:feature:<token>; applying that plan without repeating ref journals compact:t:main:<token> and calls the provider with ref="main", so the plan's verification handle resolves 404 and the wrong ref is mutated. The execution half predates this PR, but the new equality guarantee and its test only cover the default ref. Consider ref: str | None = None (and deriving or validating table the same way), or rejecting an apply whose table/ref disagree with the plan.
Checked: CLI subject resolution, both journal stores' conflict scans, the packaged manifest copy, and the existing compaction/expiry tests. Not covered by any test: a tokenless plan and a non-default ref; the nightly lane only runs snapshot_expiry.
Architecture
Data flow
Review
✨ Nothing flagged · reviewed
a13c153🪧 More tips
npx skills add coldteadotai/pr-lens, then tell your coding agent: "Diagram the change you just made with PR Lens and attach it to the pull request."npx @coldtea/pr-lens-cli analyze --base origin/mainon a branch, thennpx @coldtea/pr-lens-cli render .pr-lens/graph.json. Same lenses, your own model key, before the pull request existsrenderreads.github/pr-lens.ymland applies your renames, exclusions and lane pins at draw timegithub.comment.collapsed: truein.github/pr-lens.ymlto fold the comment behind one View architecture… row. Drawing still runs as beforegithub.draw: on-demandin.github/pr-lens.ymland PR Lens stops drawing pull requests on its own. Comment@pr-lens drawon a pull request when you want that one drawn.github/workflows/pr-lens.ymlwithcoldteadotai/pr-lens/packages/action@v0and your model provider's key as itsapi-keyto run PR Lens from your own CI. Any/chat/completionsendpoint worksgithub.description: falsein.github/pr-lens.ymlto stop PR Lens adding buttons to your PR descriptionsThanks for using PR Lens! It's free for OSS. If it helped, a star helps other teams find it.