feat(fleet): add CAS-protected stale worker retirement capability (Closes #980) #982

2026-07-30T14:25:50-05:00

jcwalker3 commented

2026-07-30 14:25:50 -05:00

Summary

Adds a sanctioned controller/reconciler capability that retires conclusively
stale MCP worker-registry rows through a dry-run-first, compare-and-swap
protected workflow (#980).

#978 / merged PR #979 made the fleet observable but read-only. There was no
native mutation that could retire registry rows whose owning process is gone,
and the only available token — mcp_fleet_snapshot's registry_revision — is
seeded with snapshot_at at second precision, so a CAS gated on it can never
pass. This implements the corrected R3-C contract from the #980 body.

Stable registry CAS

New mcp_fleet_retirement.registry_fingerprint(rows) derives the token
exclusively from canonical retirement-relevant registry content
(FINGERPRINT_FIELDS): identity, ownership, liveness, registration state,
repository binding. It contains no snapshot_at, wall-clock, request,
report-generation, or serialization time, and the serialized rows are sorted so
iteration order cannot perturb the digest. Numeric values are canonicalised, so
a TTL that round-trips through SQLite as 900.0 hashes identically to 900.

A second candidate_fingerprint pins the exact approved candidate set.

The existing time-seeded fleet_revision / registry_revision is left
exactly as it is — a separate token was the smaller and safer change, and the
#978 snapshot's shape and semantics are unchanged.

Transactional revalidation and fail-closed behaviour

WorkerRegistry.retire_stale_workers performs the whole decision inside one
BEGIN IMMEDIATE transaction: re-read the authoritative rows → recompute the
registry fingerprint from those rows → recompute the eligibility plan from
those rows (re-reading active workflow leases, so a lease acquired after
planning still protects its worker) → compare the candidate fingerprint →
revalidate every requested identity → retire each survivor with a guarded
UPDATE that additionally asserts status, last_heartbeat_at,
generation_id, session_id, fencing_epoch, and pid are unchanged.

There is no window between a safety check and its matching write, so the
check/delete race cannot occur. Drift returns registry_revision_moved or
candidate_set_moved with retired_count: 0 and no write. Any exception —
including a commit failure — rolls back and returns transaction_failed with
success: false, retired_count: 0, mutation_performed: false; a partial
write can never be reported as success.

A registration is retired only when every conjunct holds: status active,
all required registry fields present, parsable heartbeat, not live,
pid_alive is False (an unprobeable None fails closed), expired heartbeat,
ownership_state == "stale", canonical repository binding, no identity
evidence shared with a live or unprobeable worker, and no active workflow-lease
ownership. Everything else is preserved with a structured reason code.

Retired rows keep their history (status -> retired, plus retired_at,
retired_by, retirement_reason); nothing is deleted. Because retired is not
active, the #978 snapshot counts them as historical rather than stale.

Retirement does not repair untrusted live identity. Live workers on legacy
pid-/proc- instance identities are preserved untouched and keep their
legacy_incomplete_identity blockers, so stale_worker_count: 0 alongside
live_fleet_safe: false is a truthful result, and the post_apply block
reports the remaining blockers rather than claiming safety.

Authorization

Exposed as gitea_plan_stale_worker_retirement (read-only) and
gitea_apply_stale_worker_retirement, both restricted to controller /
reconciler role kinds — matching the gitea_snapshot_instance_fleet
restriction. The Gitea operation gate stays gitea.read because the mutation
lands in the local control-plane worker registry, not in Gitea, exactly as
the #601 lease lifecycle does. No new Gitea write permission is introduced for
any profile and no author permission is broadened. Apply additionally
re-asserts master parity, stable-runtime mode, fresh capability resolution,
workflow-lease enumeration, and #948 daemon-cohort uniqueness before touching
anything.

No authorization bypass, recovery grant, or BAA activation is added, and the
observational fleet snapshot is not turned into a gate on author work.

Changes

mcp_fleet_retirement.py (new) — pure decision layer: registry_fingerprint,
candidate_fingerprint, plan_stale_worker_retirement, summarize_plan,
reason codes, FINGERPRINT_FIELDS, REQUIRED_IDENTITY_FIELDS.
mcp_worker_identity.py — STATUS_RETIRED; additive retired_at,
retired_by, retirement_reason columns; WorkerRegistry.retire_stale_workers.
gitea_mcp_server.py — the two tools plus _retirement_role_block,
_retirement_runtime_block, _retirement_protected_owners,
_retirement_plan, _retirement_revalidation_plan, _retirement_post_apply.
task_capability_map.py — four additive task entries, gitea.read /
controller.
docs/stale-worker-retirement.md (new), cross-linked from
docs/instance-fleet-identity.md; both tools registered in
docs/mcp-tool-inventory.md.
tests/test_issue_980_stale_worker_retirement.py (new).

Tests

tests/test_issue_980_stale_worker_retirement.py — 40 passed, 23 subtests
passed. Covers token stability across observation times, canonical ordering,
numeric-typing stability, retirement-relevant change detection (8 subtests),
heartbeat and TTL changes, plan-performs-no-mutation, moved registry token
retires zero, moved candidate set retires zero, per-target revalidation, a
worker becoming live between plan and apply, a worker becoming ambiguous,
incomplete legacy identity fail-closed, mixed eligible/ineligible fleets,
transaction failure cannot report partial success, structured
mutation_performed / reason-code accuracy, idempotent replay, post-apply fleet
compatibility, and capability exposure. Includes the explicit regression test
proving the old snapshot_at derivation moved the token one second apart while
the new registry CAS token does not.

Adjacent suites (#980, #978, #975, #948, task-capability role invariants) —
192 passed, 124 subtests passed.

Full suite, both runs from branches/ worktrees (baseline detached at master
108cbfa173de34470d7abc022b929e6b3adc418a):

Run	Result
master baseline	28 failed, 6166 passed, 6 skipped, 1106 subtests passed
this branch	28 failed, 6206 passed, 6 skipped, 1129 subtests passed

The failing test set is byte-identical between the two runs (comm over the
sorted FAILED lines reports no line on either side): zero regressions
introduced, zero pre-existing failures masked. The +40 passing delta is this
PR's new tests.

The 28 baseline failures reproduce identically on unmodified master and are
pre-existing (branch-cleanup guard, commit payloads, dirty-orphan worktree
recovery, #702 review findings, #784 dependency-edge schema, #956 threat-model
anchors, mcp_server preflight/whoami, post-merge moot lease, PR-ownership
mismatch, preflight workspace forwarding, reconciler cleanup/close/supersession,
workspace-guard alignment). They fall outside the scope of this change.

Scope

BAA, issue #981, PR #906, and issue #650 are not dependencies of this work
and were not touched. Review 624, branch feat/issue-650-providers-insights,
worktree branches/conflict-fix-pr-906-983e8ac, profiles, configuration,
credentials, sessions, and running processes were not modified. No real worker
registry row was retired — every test uses a throwaway SQLite database.

Reviewer focus

Token stability (FINGERPRINT_FIELDS completeness), the transaction boundary in
WorkerRegistry.retire_stale_workers, the per-row guarded UPDATE, and the
fail-closed preservation of active or ambiguously identified workers. One
deliberate interpretation is called out in the module docstring and the doc:
trusted inst-… launcher provenance is not required for retirement, because
#980 places trusted client_instance_id propagation out of scope and requiring
it would preserve every legacy row forever; what is required is that the registry
fields the eligibility conjunction reads are present.

Closes #980

## Summary Adds a sanctioned controller/reconciler capability that retires conclusively stale MCP worker-registry rows through a dry-run-first, compare-and-swap protected workflow (#980). #978 / merged PR #979 made the fleet observable but read-only. There was no native mutation that could retire registry rows whose owning process is gone, and the only available token — `mcp_fleet_snapshot`'s `registry_revision` — is seeded with `snapshot_at` at second precision, so a CAS gated on it can never pass. This implements the corrected R3-C contract from the #980 body. ## Stable registry CAS New `mcp_fleet_retirement.registry_fingerprint(rows)` derives the token **exclusively** from canonical retirement-relevant registry content (`FINGERPRINT_FIELDS`): identity, ownership, liveness, registration state, repository binding. It contains no `snapshot_at`, wall-clock, request, report-generation, or serialization time, and the serialized rows are sorted so iteration order cannot perturb the digest. Numeric values are canonicalised, so a TTL that round-trips through SQLite as `900.0` hashes identically to `900`. A second `candidate_fingerprint` pins the exact approved candidate set. The existing time-seeded `fleet_revision` / `registry_revision` is **left exactly as it is** — a separate token was the smaller and safer change, and the #978 snapshot's shape and semantics are unchanged. ## Transactional revalidation and fail-closed behaviour `WorkerRegistry.retire_stale_workers` performs the whole decision inside one `BEGIN IMMEDIATE` transaction: re-read the authoritative rows → recompute the registry fingerprint from *those* rows → recompute the eligibility plan from *those* rows (re-reading active workflow leases, so a lease acquired after planning still protects its worker) → compare the candidate fingerprint → revalidate every requested identity → retire each survivor with a **guarded `UPDATE`** that additionally asserts `status`, `last_heartbeat_at`, `generation_id`, `session_id`, `fencing_epoch`, and `pid` are unchanged. There is no window between a safety check and its matching write, so the check/delete race cannot occur. Drift returns `registry_revision_moved` or `candidate_set_moved` with `retired_count: 0` and no write. Any exception — including a commit failure — rolls back and returns `transaction_failed` with `success: false`, `retired_count: 0`, `mutation_performed: false`; a partial write can never be reported as success. A registration is retired only when **every** conjunct holds: `status` active, all required registry fields present, parsable heartbeat, not live, `pid_alive is False` (an unprobeable `None` fails closed), expired heartbeat, `ownership_state == "stale"`, canonical repository binding, no identity evidence shared with a live or unprobeable worker, and no active workflow-lease ownership. Everything else is preserved with a structured reason code. Retired rows keep their history (`status -> retired`, plus `retired_at`, `retired_by`, `retirement_reason`); nothing is deleted. Because `retired` is not `active`, the #978 snapshot counts them as historical rather than stale. **Retirement does not repair untrusted live identity.** Live workers on legacy `pid-`/`proc-` instance identities are preserved untouched and keep their `legacy_incomplete_identity` blockers, so `stale_worker_count: 0` alongside `live_fleet_safe: false` is a truthful result, and the `post_apply` block reports the remaining blockers rather than claiming safety. ## Authorization Exposed as `gitea_plan_stale_worker_retirement` (read-only) and `gitea_apply_stale_worker_retirement`, both restricted to `controller` / `reconciler` role kinds — matching the `gitea_snapshot_instance_fleet` restriction. The Gitea operation gate stays `gitea.read` because the mutation lands in the **local control-plane worker registry**, not in Gitea, exactly as the #601 lease lifecycle does. **No new Gitea write permission is introduced for any profile and no author permission is broadened.** Apply additionally re-asserts master parity, stable-runtime mode, fresh capability resolution, workflow-lease enumeration, and #948 daemon-cohort uniqueness before touching anything. No authorization bypass, recovery grant, or BAA activation is added, and the observational fleet snapshot is not turned into a gate on author work. ## Changes - `mcp_fleet_retirement.py` (new) — pure decision layer: `registry_fingerprint`, `candidate_fingerprint`, `plan_stale_worker_retirement`, `summarize_plan`, reason codes, `FINGERPRINT_FIELDS`, `REQUIRED_IDENTITY_FIELDS`. - `mcp_worker_identity.py` — `STATUS_RETIRED`; additive `retired_at`, `retired_by`, `retirement_reason` columns; `WorkerRegistry.retire_stale_workers`. - `gitea_mcp_server.py` — the two tools plus `_retirement_role_block`, `_retirement_runtime_block`, `_retirement_protected_owners`, `_retirement_plan`, `_retirement_revalidation_plan`, `_retirement_post_apply`. - `task_capability_map.py` — four additive task entries, `gitea.read` / `controller`. - `docs/stale-worker-retirement.md` (new), cross-linked from `docs/instance-fleet-identity.md`; both tools registered in `docs/mcp-tool-inventory.md`. - `tests/test_issue_980_stale_worker_retirement.py` (new). ## Tests `tests/test_issue_980_stale_worker_retirement.py` — **40 passed, 23 subtests passed**. Covers token stability across observation times, canonical ordering, numeric-typing stability, retirement-relevant change detection (8 subtests), heartbeat and TTL changes, plan-performs-no-mutation, moved registry token retires zero, moved candidate set retires zero, per-target revalidation, a worker becoming live between plan and apply, a worker becoming ambiguous, incomplete legacy identity fail-closed, mixed eligible/ineligible fleets, transaction failure cannot report partial success, structured `mutation_performed` / reason-code accuracy, idempotent replay, post-apply fleet compatibility, and capability exposure. Includes the explicit regression test proving the old `snapshot_at` derivation moved the token one second apart while the new registry CAS token does not. Adjacent suites (#980, #978, #975, #948, task-capability role invariants) — **192 passed, 124 subtests passed**. Full suite, both runs from `branches/` worktrees (baseline detached at master `108cbfa173de34470d7abc022b929e6b3adc418a`): | Run | Result | | --- | --- | | master baseline | 28 failed, 6166 passed, 6 skipped, 1106 subtests passed | | this branch | 28 failed, 6206 passed, 6 skipped, 1129 subtests passed | The failing test set is **byte-identical** between the two runs (`comm` over the sorted `FAILED` lines reports no line on either side): zero regressions introduced, zero pre-existing failures masked. The +40 passing delta is this PR's new tests. The 28 baseline failures reproduce identically on unmodified master and are pre-existing (branch-cleanup guard, commit payloads, dirty-orphan worktree recovery, #702 review findings, #784 dependency-edge schema, #956 threat-model anchors, mcp_server preflight/whoami, post-merge moot lease, PR-ownership mismatch, preflight workspace forwarding, reconciler cleanup/close/supersession, workspace-guard alignment). They fall outside the scope of this change. ## Scope BAA, issue #981, PR #906, and issue #650 are **not dependencies** of this work and were not touched. Review `624`, branch `feat/issue-650-providers-insights`, worktree `branches/conflict-fix-pr-906-983e8ac`, profiles, configuration, credentials, sessions, and running processes were not modified. No real worker registry row was retired — every test uses a throwaway SQLite database. ## Reviewer focus Token stability (`FINGERPRINT_FIELDS` completeness), the transaction boundary in `WorkerRegistry.retire_stale_workers`, the per-row guarded `UPDATE`, and the fail-closed preservation of active or ambiguously identified workers. One deliberate interpretation is called out in the module docstring and the doc: trusted `inst-…` launcher provenance is **not** required for retirement, because #980 places trusted `client_instance_id` propagation out of scope and requiring it would preserve every legacy row forever; what is required is that the registry fields the eligibility conjunction reads are present. Closes #980

jcwalker3 added 1 commit 2026-07-30 14:25:51 -05:00

feat(fleet): add CAS-protected stale worker retirement capability c0c6d14b73

Adds a sanctioned controller/reconciler capability that retires conclusively
stale MCP worker-registry rows through a dry-run-first, compare-and-swap
protected workflow (#980).

The #978 fleet snapshot is read-only, and its registry_revision is seeded with
snapshot_at at second precision, so a CAS gated on it can never pass. That
token is left unchanged; retirement gets its own stable registry_fingerprint
derived exclusively from canonical retirement-relevant registry content, plus a
candidate_fingerprint pinning the approved set. Identical contents observed at
different times produce the same token; row order never affects it; any
retirement-relevant change moves it.

WorkerRegistry.retire_stale_workers performs the whole decision inside one
BEGIN IMMEDIATE transaction: re-read rows, recompute the fingerprint from those
rows, recompute the eligibility plan from those rows (re-reading active workflow
leases), compare the candidate fingerprint, revalidate every target, then retire
each survivor with a guarded UPDATE asserting its status, heartbeat, generation,
session, fencing epoch, and pid are unchanged. Drift retires zero workers and
reports registry_revision_moved or candidate_set_moved; any exception rolls back
and reports transaction_failed, so a partial write is never reported as success.

Retirement requires a full conjunction: active status, complete registry fields,
parsable heartbeat, not live, pid_alive false, expired heartbeat, stale
ownership, canonical repository binding, no identity evidence shared with a live
or unprobeable worker, and no active workflow-lease ownership. Everything else
is preserved with a structured reason code. Retired rows become historical
rather than stale and keep their history; nothing is deleted.

Retirement does not repair untrusted live identity: live workers on legacy
instance identities are preserved and keep their legacy_incomplete_identity
blockers, so the result never claims the fleet became safe.

Exposed as gitea_plan_stale_worker_retirement and
gitea_apply_stale_worker_retirement, restricted to controller/reconciler role
kinds. The Gitea operation gate stays gitea.read because the mutation lands in
the local control-plane registry, matching the #601 lease lifecycle; no new
Gitea write permission is introduced and no author permission is broadened.

Tests: tests/test_issue_980_stale_worker_retirement.py (40 passed, 23 subtests),
including the regression test proving the old snapshot_at derivation moved the
token one second apart while the new registry CAS token does not. Full suite
from the branch worktree matches the master baseline exactly: 28 failed / 6206
passed vs 28 failed / 6166 passed, identical failure set.

Closes #980

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

jcwalker3 referenced this pull request

2026-07-30 14:29:55 -05:00

feat(fleet): add CAS-protected stale worker retirement capability #980

sysadmin commented

2026-07-30 15:03:54 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #982
issue: #980
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 65116-044a9d36aaed
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr982-c0c6d14
phase: claimed
candidate_head: c0c6d14b73
target_branch: master
target_branch_sha: 108cbfa173
last_activity: 2026-07-30T20:03:53Z
expires_at: 2026-07-30T20:13:53Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #982 issue: #980 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 65116-044a9d36aaed worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr982-c0c6d14 phase: claimed candidate_head: c0c6d14b73add36ac157d501c603fd9ea51de1da target_branch: master target_branch_sha: 108cbfa173de34470d7abc022b929e6b3adc418a last_activity: 2026-07-30T20:03:53Z expires_at: 2026-07-30T20:13:53Z blocker: none

sysadmin requested changes 2026-07-30 15:09:07 -05:00

sysadmin left a comment

Canonical PR State

STATE: request_changes — independent reviewer found three hard-rule blockers (authorization, identity proof, external safety inputs outside TX)
WHO_IS_NEXT: author
NEXT_ACTION: prgs-author addresses only B1–B3 on PR #982, adds required regression tests, pushes a new head, and publishes a new exact-head author handoff.
NEXT_PROMPT:

As prgs-author on prgs for Scaled-Tech-Consulting/Gitea-Tools: remediate PR #982 (issue #980) at prior head c0c6d14b73add36ac157d501c603fd9ea51de1da after REQUEST_CHANGES. Fix only: (B1) map gitea_apply_stale_worker_retirement to a dedicated mutation capability (not gitea.read alone) with an immediate mutation gate; (B2) fail closed on incomplete/untrusted identity so complete legacy-pid rows are not retired without affirmative identity/ownership/liveness proof; (B3) re-probe PID liveness and active workflow-lease ownership immediately before each guarded UPDATE (or equivalent fencing) because lease DB and OS liveness sit outside the worker-registry BEGIN IMMEDIATE transaction. Add the required regression tests; push a new head; publish exact-head I5-A handoff. Do not merge.

WHAT_HAPPENED: Independent prgs-reviewer review of PR #982 at pinned head c0c6d14b73. Scope reconstructed (1 commit, 8 files, Closes #980). Focused #980 tests 40/40 pass; adjacent 195 pass. Formal REQUEST_CHANGES for B1/B2/B3.
WHY: Gate A hard rule fails (apply authorized solely by gitea.read). Gate B hard rule fails (complete untrusted legacy rows are retireable). Gate D hard rule fails (lease DB + PID probe outside registry TX without equivalent fencing at write time).
ISSUE: #980
HEAD_SHA: c0c6d14b73
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: no
BLOCKERS: B1 dedicated mutation capability missing; B2 incomplete identity = non-null columns only; B3 external lease/PID safety inputs not fenced at UPDATE.
VALIDATION: whoami sysadmin/prgs-reviewer; parity in_parity mutation_safe at 108cbfa173de; head pin confirmed; lease 18835 own_active; worktree branches/review-pr982-c0c6d14 @ c0c6d14 clean; focused tests 40 passed; adjacent 195 passed; no live registry mutation; no source edits.
LAST_UPDATED_BY: sysadmin / prgs-reviewer (gitea-reviewer)

Independent review — REQUEST_CHANGES (V6-B)

Reviewed head: c0c6d14b73add36ac157d501c603fd9ea51de1da
Reviewer: sysadmin / prgs-reviewer (namespace gitea-reviewer)
Base: master @ 108cbfa173de34470d7abc022b929e6b3adc418a
Author: jcwalker3 (identity separation verified)

Author handoff (I5-A, comment 18823) and all author test claims were treated as untrusted until independently verified.

Scope reconstruction (verified)

Exactly one commit above master; eight files as claimed: mcp_fleet_retirement.py, mcp_worker_identity.py, gitea_mcp_server.py, task_capability_map.py, three docs, tests/test_issue_980_stale_worker_retirement.py. Closes #980 present. No unrelated behavior observed.

Non-blocking design strengths: stable fingerprints without wall-clock; plan mutation-free; drift retires zero; TX rollback reports transaction_failed; per-row guarded UPDATE; live/unprobeable/shared-identity/foreign/protected rows preserved in plan layer; retired rows historical not deleted; post-apply does not claim fleet safety with residual live legacy blockers.

Blocking findings

B1 — CRITICAL (Gate A): Apply authorized by `gitea.read` alone

Severity: Critical / hard-rule violation
Files/symbols: task_capability_map.py apply_stale_worker_retirement / gitea_apply_stale_worker_retirement; gitea_mcp_server.py gitea_apply_stale_worker_retirement, _retirement_role_block
Path: _profile_operation_gate("gitea.read") then capability re-check still requires only gitea.read. Role controller/reconciler is secondary, not a mutation capability.
Expected: Dedicated enforceable mutation capability + immediate mutation gate (in-repo precedent: review 434 F1 / #709 — "gitea.read is insufficient").
Actual: Sole mapped permission is gitea.read. Plan and apply share the observational permission class.
Why tests miss it: CapabilityExposureTests asserts gitea.read; never proves only-read profiles cannot apply.
Smallest fix: New permission e.g. gitea.worker_registry.retire granted only to controller/reconciler; map apply tasks only; gate apply with that permission before any write; keep plan on gitea.read.
Required regression: only-gitea.read denied; author/reviewer/merger denied; controller/reconciler with mutation perm allowed on isolated temp DB; plan still read-only.

B2 — CRITICAL (Gate B): Incomplete identity = missing columns; complete untrusted legacy rows retire

Severity: Critical / hard-rule violation
Files/symbols: mcp_fleet_retirement.py REQUIRED_IDENTITY_FIELDS, plan_stale_worker_retirement
Path: _missing_identity_fields only null/blank checks. Full columns + legacy-pid-… + dead PID + expired HB → eligible. Independent probe confirmed candidate_count=1 for such a row.
Expected: Affirmative unique identity/ownership/liveness proof; fail closed otherwise.
Actual: Trusted inst-* provenance deliberately excluded; liveness is local os.kill(pid,0) only.
Why tests miss it: incomplete tests cover only missing pid/session; fixtures default to trusted inst- IDs.
Smallest fix: Preserve non-trusted provenance unless stronger fencing (start-time/host/generation) exists; or amend #980 with explicit reviewed authorization. Do not rely on "legacy rows would remain forever."
Required regression: complete legacy-pid dead row preserved (or only retired under documented stronger proof) with reason code.

B3 — HIGH (Gate D): Lease DB + PID liveness outside worker-registry TX

Severity: High / hard-rule violation
Files/symbols: WorkerRegistry.retire_stale_workers; _retirement_protected_owners; is_process_alive
Path: BEGIN IMMEDIATE locks worker registry only. Leases re-read from separate control-plane DB. Guarded UPDATE does not re-assert pid_alive=false or current lease ownership after revalidation snapshot.
Expected: Critical safety inputs in same TX or equally strong fencing at write.
Actual: Window for new lease attach or PID reuse after plan_fn before UPDATE.
Why tests miss it: static probes; no dual-DB concurrent lease race; no post-revalidation PID reuse.
Smallest fix: Re-probe pid_alive and lease ownership immediately before each UPDATE (fail closed); consider start-time checks per #971.
Required regression: lease-after-plan_fn and PID-alive-after-revalidation → zero retirement; mutation_performed matches commit.

Non-blocking

Subset of planned identities may retire while candidate fingerprint still matches full set — document or require exact multiset equality.
Gate C fingerprints sound for stored columns.
Gate E retirement semantics (mark/history/no silent heartbeat revive) sound.
Full-suite author parity (28 fail both sides) not independently re-run; not used as proof. Independent: 40/40 focused; 195 adjacent.

Verdict

REQUEST_CHANGES at c0c6d14b73add36ac157d501c603fd9ea51de1da. Do not merge.

WHO_IS_NEXT: prgs-author to address only the exact requested changes on PR #982, add the required regression tests, push a new head, and publish a new exact-head author handoff.

## Canonical PR State STATE: request_changes — independent reviewer found three hard-rule blockers (authorization, identity proof, external safety inputs outside TX) WHO_IS_NEXT: author NEXT_ACTION: prgs-author addresses only B1–B3 on PR #982, adds required regression tests, pushes a new head, and publishes a new exact-head author handoff. NEXT_PROMPT: ```text As prgs-author on prgs for Scaled-Tech-Consulting/Gitea-Tools: remediate PR #982 (issue #980) at prior head c0c6d14b73add36ac157d501c603fd9ea51de1da after REQUEST_CHANGES. Fix only: (B1) map gitea_apply_stale_worker_retirement to a dedicated mutation capability (not gitea.read alone) with an immediate mutation gate; (B2) fail closed on incomplete/untrusted identity so complete legacy-pid rows are not retired without affirmative identity/ownership/liveness proof; (B3) re-probe PID liveness and active workflow-lease ownership immediately before each guarded UPDATE (or equivalent fencing) because lease DB and OS liveness sit outside the worker-registry BEGIN IMMEDIATE transaction. Add the required regression tests; push a new head; publish exact-head I5-A handoff. Do not merge. ``` WHAT_HAPPENED: Independent prgs-reviewer review of PR #982 at pinned head c0c6d14b73add36ac157d501c603fd9ea51de1da. Scope reconstructed (1 commit, 8 files, Closes #980). Focused #980 tests 40/40 pass; adjacent 195 pass. Formal REQUEST_CHANGES for B1/B2/B3. WHY: Gate A hard rule fails (apply authorized solely by gitea.read). Gate B hard rule fails (complete untrusted legacy rows are retireable). Gate D hard rule fails (lease DB + PID probe outside registry TX without equivalent fencing at write time). ISSUE: #980 HEAD_SHA: c0c6d14b73add36ac157d501c603fd9ea51de1da REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: no BLOCKERS: B1 dedicated mutation capability missing; B2 incomplete identity = non-null columns only; B3 external lease/PID safety inputs not fenced at UPDATE. VALIDATION: whoami sysadmin/prgs-reviewer; parity in_parity mutation_safe at 108cbfa173de; head pin confirmed; lease 18835 own_active; worktree branches/review-pr982-c0c6d14 @ c0c6d14 clean; focused tests 40 passed; adjacent 195 passed; no live registry mutation; no source edits. LAST_UPDATED_BY: sysadmin / prgs-reviewer (gitea-reviewer) --- ## Independent review — REQUEST_CHANGES (V6-B) **Reviewed head:** `c0c6d14b73add36ac157d501c603fd9ea51de1da` **Reviewer:** `sysadmin` / `prgs-reviewer` (namespace `gitea-reviewer`) **Base:** `master` @ `108cbfa173de34470d7abc022b929e6b3adc418a` **Author:** `jcwalker3` (identity separation verified) Author handoff (I5-A, comment 18823) and all author test claims were treated as untrusted until independently verified. ### Scope reconstruction (verified) Exactly **one** commit above master; **eight** files as claimed: `mcp_fleet_retirement.py`, `mcp_worker_identity.py`, `gitea_mcp_server.py`, `task_capability_map.py`, three docs, `tests/test_issue_980_stale_worker_retirement.py`. `Closes #980` present. No unrelated behavior observed. Non-blocking design strengths: stable fingerprints without wall-clock; plan mutation-free; drift retires zero; TX rollback reports `transaction_failed`; per-row guarded UPDATE; live/unprobeable/shared-identity/foreign/protected rows preserved in plan layer; retired rows historical not deleted; post-apply does not claim fleet safety with residual live legacy blockers. ## Blocking findings ### B1 — CRITICAL (Gate A): Apply authorized by `gitea.read` alone - **Severity:** Critical / hard-rule violation - **Files/symbols:** `task_capability_map.py` `apply_stale_worker_retirement` / `gitea_apply_stale_worker_retirement`; `gitea_mcp_server.py` `gitea_apply_stale_worker_retirement`, `_retirement_role_block` - **Path:** `_profile_operation_gate("gitea.read")` then capability re-check still requires only `gitea.read`. Role controller/reconciler is secondary, not a mutation capability. - **Expected:** Dedicated enforceable mutation capability + immediate mutation gate (in-repo precedent: review 434 F1 / #709 — "gitea.read is insufficient"). - **Actual:** Sole mapped permission is `gitea.read`. Plan and apply share the observational permission class. - **Why tests miss it:** `CapabilityExposureTests` asserts `gitea.read`; never proves only-read profiles cannot apply. - **Smallest fix:** New permission e.g. `gitea.worker_registry.retire` granted only to controller/reconciler; map apply tasks only; gate apply with that permission before any write; keep plan on `gitea.read`. - **Required regression:** only-`gitea.read` denied; author/reviewer/merger denied; controller/reconciler with mutation perm allowed on isolated temp DB; plan still read-only. ### B2 — CRITICAL (Gate B): Incomplete identity = missing columns; complete untrusted legacy rows retire - **Severity:** Critical / hard-rule violation - **Files/symbols:** `mcp_fleet_retirement.py` `REQUIRED_IDENTITY_FIELDS`, `plan_stale_worker_retirement` - **Path:** `_missing_identity_fields` only null/blank checks. Full columns + `legacy-pid-…` + dead PID + expired HB → eligible. Independent probe confirmed candidate_count=1 for such a row. - **Expected:** Affirmative unique identity/ownership/liveness proof; fail closed otherwise. - **Actual:** Trusted `inst-*` provenance deliberately excluded; liveness is local `os.kill(pid,0)` only. - **Why tests miss it:** incomplete tests cover only missing pid/session; fixtures default to trusted inst- IDs. - **Smallest fix:** Preserve non-trusted provenance unless stronger fencing (start-time/host/generation) exists; or amend #980 with explicit reviewed authorization. Do not rely on "legacy rows would remain forever." - **Required regression:** complete legacy-pid dead row preserved (or only retired under documented stronger proof) with reason code. ### B3 — HIGH (Gate D): Lease DB + PID liveness outside worker-registry TX - **Severity:** High / hard-rule violation - **Files/symbols:** `WorkerRegistry.retire_stale_workers`; `_retirement_protected_owners`; `is_process_alive` - **Path:** BEGIN IMMEDIATE locks worker registry only. Leases re-read from separate control-plane DB. Guarded UPDATE does not re-assert pid_alive=false or current lease ownership after revalidation snapshot. - **Expected:** Critical safety inputs in same TX or equally strong fencing at write. - **Actual:** Window for new lease attach or PID reuse after plan_fn before UPDATE. - **Why tests miss it:** static probes; no dual-DB concurrent lease race; no post-revalidation PID reuse. - **Smallest fix:** Re-probe pid_alive and lease ownership immediately before each UPDATE (fail closed); consider start-time checks per #971. - **Required regression:** lease-after-plan_fn and PID-alive-after-revalidation → zero retirement; mutation_performed matches commit. ## Non-blocking 1. Subset of planned identities may retire while candidate fingerprint still matches full set — document or require exact multiset equality. 2. Gate C fingerprints sound for stored columns. 3. Gate E retirement semantics (mark/history/no silent heartbeat revive) sound. 4. Full-suite author parity (28 fail both sides) **not** independently re-run; not used as proof. Independent: 40/40 focused; 195 adjacent. ## Verdict **REQUEST_CHANGES** at `c0c6d14b73add36ac157d501c603fd9ea51de1da`. Do not merge. `WHO_IS_NEXT: prgs-author to address only the exact requested changes on PR #982, add the required regression tests, push a new head, and publish a new exact-head author handoff.`

sysadmin commented

2026-07-30 15:09:19 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #982
issue: #980
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 65116-044a9d36aaed
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr982-c0c6d14
phase: released
candidate_head: c0c6d14b73
target_branch: master
target_branch_sha: 108cbfa173
last_activity: 2026-07-30T20:09:19Z
expires_at: 2026-07-30T20:19:19Z
blocker: manual-release

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #982 issue: #980 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 65116-044a9d36aaed worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr982-c0c6d14 phase: released candidate_head: c0c6d14b73add36ac157d501c603fd9ea51de1da target_branch: master target_branch_sha: 108cbfa173de34470d7abc022b929e6b3adc418a last_activity: 2026-07-30T20:09:19Z expires_at: 2026-07-30T20:19:19Z blocker: manual-release

jcwalker3 referenced this issue from a commit

2026-07-30 17:02:06 -05:00

fix(fleet): dedicated retirement capability, identity proof, external fencing

jcwalker3 added 1 commit 2026-07-30 17:02:06 -05:00

fix(fleet): dedicated retirement capability, identity proof, external fencing e344e68a13

Addresses the three blocking findings in review 657 on PR #982 and nothing
else.

B1 — apply was authorized by gitea.read
---------------------------------------
Plan and apply shared the observational permission class, so any profile that
could look could also destroy; the mutation landing in the local control-plane
registry rather than in Gitea makes it no less a mutation.

Apply now requires its own capability, gitea.worker_registry.retire, checked at
entry and re-resolved immediately before the registry mutation. Plan stays on
gitea.read. No profile holds the new permission by default, so author,
reviewer, merger, and read-only profiles fail closed on the permission itself
rather than on the role check alone; the controller/reconciler role restriction
remains as defence in depth. Granting it is a deliberate operator edit to
profiles.json, and removing it revokes apply completely. No new Gitea write
permission is introduced and no author permission is broadened.

B2 — complete legacy rows could be retired without identity proof
-----------------------------------------------------------------
"Incomplete identity" previously meant only that pre-existing columns were
null, which a legacy-pid row satisfies trivially. Retirement now requires an
affirmative two-part proof: launcher-minted inst- attribution, plus fencing
evidence (host_id, boot_id, process_start_time) that turns a bare pid into a
statement about one process incarnation.

New mcp_process_fencing supplies those probes; every one returns None rather
than guessing, and None always preserves. Rows written before these columns
existed, and rows on legacy instance identities, are preserved permanently —
they are retired only after re-registering under a trusted identity. That
legacy rows would otherwise remain outstanding is explicitly not treated as
grounds for a weaker proof. A live pid stays an absolute block even across a
boot boundary, and pid reuse is reported distinctly from a live worker.

B3 — lease and OS liveness sat outside the registry transaction
---------------------------------------------------------------
BEGIN IMMEDIATE locks the worker registry only, and the per-target loop runs
after revalidation, so a lease acquired or a pid revived in between would go
unnoticed — the registry-column guard cannot catch it because no registry
column changed.

Two mechanisms now close that window, both applied per target immediately
before its own write: an external_fence_fn version token over active leases,
captured inside the transaction before the authoritative read and re-compared
before every guarded UPDATE (movement aborts the whole transaction; an
unreadable lease store raises rather than comparing equal), and a liveness_fn
re-probe that must affirmatively re-establish that this exact process is gone,
comparing process_start_time so a reused pid is refused. Omitting the re-probe
retires nothing rather than proceeding unfenced. The guarded UPDATE also
asserts the fencing triple is unchanged, and all three columns participate in
the CAS token.

Preserved from the accepted work: registry_fingerprint still excludes
observation time and is order- and numeric-typing stable, plan still mutates
nothing, drift still retires zero, retired rows stay historical, and ordinary
author operations remain ungated by fleet state.

Tests: tests/test_issue_980_stale_worker_retirement.py 87 passed, 46 subtests
(was 40 passed, 23 subtests), covering all three blockers' required
regressions. Adjacent suites (#980, #978, #975, #948, task-capability role
invariants) 242 passed, 156 subtests. Full suite from the branch worktree
28 failed, 6253 passed, 6 skipped, 1152 subtests; master baseline at
108cbfa173 from branches/baseline-980-108cbfa 28 failed, 6166 passed, 6
skipped, 1106 subtests. The failing sets are identical under comm, so zero
regressions and zero masked pre-existing failures; the +87 passing delta is
this branch's tests.

No live worker-registry row was retired — every test uses a throwaway SQLite
database. BAA, issue #981, PR #906, issue #650, review 624, unrelated branches
and worktrees, profiles, configuration, credentials, sessions, and running
processes were not touched.

Refs #980
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

jcwalker3 referenced this pull request

2026-07-30 17:05:32 -05:00

feat(fleet): add CAS-protected stale worker retirement capability #980

You are not authorized to merge this pull request.

This pull request can be merged automatically.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/issue-980-stale-worker-retirement:feat/issue-980-stale-worker-retirement

git checkout feat/issue-980-stale-worker-retirement

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#982