Repository navigation
feat(lifecycle): turn the ownership writer on, and make the bootstrap's apply reachable - #1250
Conversation
ae6e237 to
2337233
Compare
Step 1 of the observe rollout, and the only one that can land before the store is bootstrapped. This worker runs DevLoopWorkflow, the only writer. Below `observe` its ownership activity short-circuits before any HTTP call, so nothing is recorded -- which is also why the bootstrap refuses to --apply below observe: a row it wrote for a devloop-workflow owner would never be heartbeated, progressed or released by the workflow it names, and would sit `active` until its liveness bound expired. So the writer goes live first, then the bootstrap imports the pull requests already in flight, then the shepherd flips and the comparison starts against a populated store. Nothing reads or compares yet: the shepherd is untouched, and the comparison is what produces the soak's evidence. The gate is in activity code rather than workflow code, so this changes no command the workflow issues and a history recorded under one mode replays under another. Refs: mctlhq/.github#57 Claude-Session: https://claude.ai/code/session_01RUQTbQvUQjwbDFd7Nq9bvC
The template described two runs and no ordering constraint. The bootstrap now refuses --apply below observe -- a row written while the writer is off is never heartbeated by the owner it names -- so the rollout is three steps and this workflow is the middle one. It also gained --service and --slug, which are worth using for a first apply: an entity the store already holds is skipped, so a row written wrongly cannot be corrected by re-running. Claude-Session: https://claude.ai/code/session_01RUQTbQvUQjwbDFd7Nq9bvC
…plicit bootstrap.py refuses --apply below LIFECYCLE_ROLLOUT_MODE=observe, and nothing in this template set that variable -- so --apply would have exited 2 in production, always. The gate was correct and unreachable. It is now a workflow parameter, and being a parameter is the point. The mode bootstrap.py reads is THIS pod's, while the hazard it guards is the Temporal worker's, which this pod cannot see: the gate is an operator ATTESTATION, not a verification. Naming it in the submission -- dry_run=false AND rollout_mode=observe -- makes that a deliberate two-part statement rather than something a default carried in unnoticed, and the value is echoed into the report so what was claimed sits beside what it licensed. Defaults to "off" so a submission that forgets it refuses rather than writes. Refs: mctlhq/.github#57 Claude-Session: https://claude.ai/code/session_01RUQTbQvUQjwbDFd7Nq9bvC
c6304ae to
7209355
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @mashkovd's task in 2m 24s —— View job Review complete
Scope: Checked and found consistent:
No P1/P2/P3 findings. This is a small, well-scoped, safe-by-default wiring change consistent with the repo's conventions and the PR's stated rollout plan.
|
There was a problem hiding this comment.
🔵 Needs a closer look
Restrict bootstrap apply to the exact observe rollout mode.
Pull request overview
Enables lifecycle ownership recording in observe mode and wires rollout attestation into the bootstrap workflow.
Changes:
- Sets the mctl-agents worker to
LIFECYCLE_ROLLOUT_MODE=observe. - Adds and forwards the bootstrap
rollout_modeparameter.
File summaries
| File | Summary |
|---|---|
platform-gitops/services/admins/mctl-agents-worker/values.yaml |
Enables observe-mode ownership writes. |
platform-gitops/argo-workflows/cluster-templates/wft-lifecycle-bootstrap.yaml |
Adds rollout-mode configuration; apply is not restricted to exactly observe (moderate issue). |
Review details
Suppressed comments (1)
platform-gitops/argo-workflows/cluster-templates/wft-lifecycle-bootstrap.yaml:109
- The template documents
rollout_mode=observeas the required apply attestation, but bootstrap.py only checksrollout.records_writes(), which is also true forenforceandonly. Thusdry_run=false -p rollout_mode=enforcecan write rows after the shepherd has begun consuming the store, bypassing the staged order this template describes and potentially racing the reader. Reject any apply mode other than exactlyobserve(preferably in bootstrap.py, with a matching template guard).
- name: rollout_mode
value: "off"
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit No significant issues found. |
|
Copilot's point is right and is fixed in
Two hazards, one on each side, so the gate has to be narrow to close both. No matching guard in the template: Worth recording that the same review round found the deeper version of this: the parameter is an operator attestation, not a verification — it sets the mode in the bootstrap pod, while the hazard belongs to the Temporal worker, which that pod cannot see. That is stated in the template comment and in the module docstring, and the attested mode is echoed into the report. |
Part of mctlhq/.github#57 — step 1 of 2 of the observe rollout, and the only one that
can land before the store is bootstrapped.
What
LIFECYCLE_ROLLOUT_MODE=observeon the mctl-agents worker. That worker runsDevLoopWorkflow, the only writer.Why it goes first
Below
observethe ownership activity short-circuits before any HTTP call. That is alsowhy the bootstrap refuses to
--applybelow observe: a row it wrote for adevloop-workflowowner would never be heartbeated, progressed or released by theworkflow it names — it would sit
activeuntil its liveness bound expired, held by anowner that does not know it holds anything.
So the order is:
dry_run=true, report reviewed row by row;dry_run=false;The previous ordering had the comparison starting against an empty store, which meant the
first week measured the import rather than the contract.
What this does not do
Nothing reads and nothing compares. The shepherd is untouched, and the comparison is what
produces the soak's evidence. Merging this changes what is recorded, not what is
decided.
The gate is in activity code, not workflow code, so this changes no command the
workflow issues: a history recorded under one mode replays byte-for-byte under another. A
test in mctl-agents asserts
LIFECYCLE_ROLLOUT_MODEnever appears indev_loop.py.Depends on
mctl-agents#378 merged and released, with this worker on that image. The rollout mode
means nothing on an image whose activity does not read it.
Verification
Then: ownership rows start appearing for PRs the DevLoop drives —
mctl_get_lifecycle_ownershipreturns a non-zero count where it returned 0.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RUQTbQvUQjwbDFd7Nq9bvC