feat(webui): request preview, authorization, and workflow initiation (Closes #643) #902

2026-07-25T00:53:59-05:00

jcwalker3 commented

2026-07-25 00:53:59 -05:00

Summary

Implements Phase 2 request preview, authorization, and workflow initiation for the web console (Closes #643).

Operators had to paste a role prompt into a terminal to start work, and nothing enforced that the allocator had been consulted first, so two sessions could reach for the same issue and each believe it was theirs. This adds a request surface: a desired role, an issue or PR, and a stated intent, answered by an authorization decision and, on confirmation, an exclusive assignment from the allocator.

Linked issue

Closes #643

Surfaces

Path	Method	Purpose
`/requests`	GET	Request form
`/requests`	POST	Render an intent preview. Never assigns.
`/api/v1/requests/preview`	POST	Intent preview as JSON
`/api/v1/requests/apply`	POST	Initiate, confirmed and audited, allocator-owned

Acceptance criteria

Preview shows authorize/deny with reasons. Five named checks, each with its own verdict, reason code, and detail: authorization, capability, lease_availability, next_safe_action, head_pin.
Apply creates an exclusive assignment or returns wait/blocked. Outcomes are assigned_work (201), wait (409), blocked (409), denied (403), invalid_request (400), evidence_unavailable (503).
Duplicate assign rejected. An active claim on the work unit, held by any session, blocks before an assignment is attempted.
Tests for preview/apply/deny/collision. 52 new tests across eight classes.
UI never shows full secrets; brief user messaging. Every interpolated value is escaped; the intent travels through the standard redaction pass before persistence.

Design notes for the reviewer

Apply runs the allocator twice, on purpose. The allocator is the only source of exclusive ownership (#600/#613), and it selects work rather than taking orders. So apply runs a dry-run first and proceeds only when the allocator would independently pick the requested work unit; otherwise it returns wait and mutates nothing. The apply call carries the dry-run's candidate_set_fingerprint as a CAS pin (#776), and the result is re-checked on the way out so an assignment naming a different work unit is not read as success.

The execution gate is action-scoped, not a phase bump. console_authz.ConsoleAction gains an optional execution_env_flag, and authorize now computes execution_enabled via a new execution_wired(action, env) helper. Raising ACTIVE_PHASE would have enabled execution for every phase-2 action at once, including system.restart_namespace and the analytics ingest, whose execution paths are not implemented here. initiate_workflow is the only action declaring a flag (WEBUI_REQUESTS_EXECUTION); every other action keeps its previous behaviour and still reports execution_enabled: false.

Operator-class authority. initiate_workflow requires operator because its outcome is a claim, not a Gitea verdict. Requesting reviewer or merger work reserves that work; it grants no right to approve or merge, which stays with the MCP role profile.

A denial is not a queue oracle. An unauthorized principal never reaches the allocator or the control-plane DB at all.

Fail-closed behaviour

An unreadable control-plane DB, an incomplete queue inventory (#758), an allocator that raises, an unpinned PR head, a PR head that moved, and an unconfirmed apply all deny without mutating. An unreadable claim inventory is never treated as "nothing holds this work unit".

Files changed

webui/request_service.py (new) — request model, preview, initiation
webui/request_views.py (new) — form and preview rendering, escaped
tests/test_webui_request_initiation.py (new) — 52 tests
webui/console_authz.py — initiate_workflow action, execution_wired()
webui/app.py — three routes and their handlers
webui/nav.py — Requests nav entry
webui/traffic_loader.py — public candidates_from_queue_snapshot alias so the request service and the traffic view share one candidate construction
docs/webui-requests.md (new), docs/webui-authz-audit.md

Validation

# this branch
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
23 failed, 5242 passed, 6 skipped, 899 subtests passed

# clean master baseline at 2f4dec83, in a branches/ worktree pinned to that head
23 failed, 5190 passed, 6 skipped, 867 subtests passed

The same 23 tests fail on both. This branch adds 52 passing tests and introduces no new full-suite failure signature. The baseline was run from a branches/ worktree because several suites are location-sensitive and report differently from /tmp.

Focused: tests/test_webui_*.py gives 538 passed, 406 subtests.

git diff --check: clean.

Risk

Moderate. This introduces the console's first wired execution path, so review should focus on the execution gate and the allocator interaction. Mitigations: execution is off by default and requires an environment variable an operator must set; the assignment path cannot bypass the allocator; approve and merge remain forbidden in every path.

Worktree / branch / head

Worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-643-request-preview-initiate
Branch: feat/issue-643-request-preview-initiate
Commit: 433f66add864062df7c211d9b52fc74fcfccfb2f
Base: master @ 2f4dec832327513118f2fe92b74da25d124a01cb

LLM Handoff Metadata

LLM-Agent-SHA: llm-643req20260725
LLM-Role: implementer
Authenticated-Gitea-User: jcwalker3
MCP-Profile: prgs-author
Branch: feat/issue-643-request-preview-initiate
Worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-643-request-preview-initiate
Self-review allowed: no

No review or merge performed in this author session.

## Summary Implements Phase 2 request preview, authorization, and workflow initiation for the web console (Closes #643). Operators had to paste a role prompt into a terminal to start work, and nothing enforced that the allocator had been consulted first, so two sessions could reach for the same issue and each believe it was theirs. This adds a request surface: a desired role, an issue or PR, and a stated intent, answered by an authorization decision and, on confirmation, an exclusive assignment from the allocator. ## Linked issue Closes #643 ## Surfaces | Path | Method | Purpose | |------|--------|---------| | `/requests` | GET | Request form | | `/requests` | POST | Render an intent preview. Never assigns. | | `/api/v1/requests/preview` | POST | Intent preview as JSON | | `/api/v1/requests/apply` | POST | Initiate, confirmed and audited, allocator-owned | ## Acceptance criteria 1. **Preview shows authorize/deny with reasons.** Five named checks, each with its own verdict, reason code, and detail: `authorization`, `capability`, `lease_availability`, `next_safe_action`, `head_pin`. 2. **Apply creates an exclusive assignment or returns wait/blocked.** Outcomes are `assigned_work` (201), `wait` (409), `blocked` (409), `denied` (403), `invalid_request` (400), `evidence_unavailable` (503). 3. **Duplicate assign rejected.** An active claim on the work unit, held by any session, blocks before an assignment is attempted. 4. **Tests for preview/apply/deny/collision.** 52 new tests across eight classes. 5. **UI never shows full secrets; brief user messaging.** Every interpolated value is escaped; the intent travels through the standard redaction pass before persistence. ## Design notes for the reviewer **Apply runs the allocator twice, on purpose.** The allocator is the only source of exclusive ownership (#600/#613), and it selects work rather than taking orders. So `apply` runs a dry-run first and proceeds only when the allocator would independently pick the requested work unit; otherwise it returns `wait` and mutates nothing. The apply call carries the dry-run's `candidate_set_fingerprint` as a CAS pin (#776), and the result is re-checked on the way out so an assignment naming a different work unit is not read as success. **The execution gate is action-scoped, not a phase bump.** `console_authz.ConsoleAction` gains an optional `execution_env_flag`, and `authorize` now computes `execution_enabled` via a new `execution_wired(action, env)` helper. Raising `ACTIVE_PHASE` would have enabled execution for every phase-2 action at once, including `system.restart_namespace` and the analytics ingest, whose execution paths are not implemented here. `initiate_workflow` is the only action declaring a flag (`WEBUI_REQUESTS_EXECUTION`); every other action keeps its previous behaviour and still reports `execution_enabled: false`. **Operator-class authority.** `initiate_workflow` requires `operator` because its outcome is a claim, not a Gitea verdict. Requesting reviewer or merger work reserves that work; it grants no right to approve or merge, which stays with the MCP role profile. **A denial is not a queue oracle.** An unauthorized principal never reaches the allocator or the control-plane DB at all. ## Fail-closed behaviour An unreadable control-plane DB, an incomplete queue inventory (#758), an allocator that raises, an unpinned PR head, a PR head that moved, and an unconfirmed apply all deny without mutating. An unreadable claim inventory is never treated as "nothing holds this work unit". ## Files changed - `webui/request_service.py` (new) — request model, preview, initiation - `webui/request_views.py` (new) — form and preview rendering, escaped - `tests/test_webui_request_initiation.py` (new) — 52 tests - `webui/console_authz.py` — `initiate_workflow` action, `execution_wired()` - `webui/app.py` — three routes and their handlers - `webui/nav.py` — Requests nav entry - `webui/traffic_loader.py` — public `candidates_from_queue_snapshot` alias so the request service and the traffic view share one candidate construction - `docs/webui-requests.md` (new), `docs/webui-authz-audit.md` ## Validation ```text # this branch WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q 23 failed, 5242 passed, 6 skipped, 899 subtests passed # clean master baseline at 2f4dec83, in a branches/ worktree pinned to that head 23 failed, 5190 passed, 6 skipped, 867 subtests passed ``` The same 23 tests fail on both. This branch adds 52 passing tests and introduces no new full-suite failure signature. The baseline was run from a `branches/` worktree because several suites are location-sensitive and report differently from `/tmp`. Focused: `tests/test_webui_*.py` gives 538 passed, 406 subtests. `git diff --check`: clean. ## Risk Moderate. This introduces the console's first wired execution path, so review should focus on the execution gate and the allocator interaction. Mitigations: execution is off by default and requires an environment variable an operator must set; the assignment path cannot bypass the allocator; approve and merge remain forbidden in every path. ## Worktree / branch / head - Worktree: `/Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-643-request-preview-initiate` - Branch: `feat/issue-643-request-preview-initiate` - Commit: `433f66add864062df7c211d9b52fc74fcfccfb2f` - Base: `master` @ `2f4dec832327513118f2fe92b74da25d124a01cb` ## LLM Handoff Metadata - LLM-Agent-SHA: llm-643req20260725 - LLM-Role: implementer - Authenticated-Gitea-User: jcwalker3 - MCP-Profile: prgs-author - Branch: feat/issue-643-request-preview-initiate - Worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-643-request-preview-initiate - Self-review allowed: no No review or merge performed in this author session.

jcwalker3 added 1 commit 2026-07-25 00:53:59 -05:00

feat(webui): request preview, authorization, and workflow initiation (Closes #643 ) 433f66add8

Operators had to paste a role prompt into a terminal to start work, and
nothing enforced that the allocator had been consulted first, so two sessions
could reach for the same issue and each believe it was theirs. This adds a
request surface: a desired role, an issue or PR, and a stated intent, answered
by an authorization decision and - on confirmation - an exclusive assignment
from the allocator.

Preview (POST /api/v1/requests/preview, and the /requests form) runs five
checks and reports authorize/deny with a reason for each: console
authorization, capability resolution for the desired role, lease availability,
whether the allocator would independently select this work unit, and head
pinning for PR work. It is read-only - it calls the allocator with apply=false
and writes only an audit line. An unauthorized principal never reaches the
allocator or the control-plane DB, so a denial cannot enumerate the queue.

Initiation (POST /api/v1/requests/apply) never assigns the requested item
directly. It runs a dry-run first and proceeds only when the allocator would
independently pick that exact work unit, carrying the dry-run's
candidate_set_fingerprint as a CAS pin; otherwise it returns wait or blocked
and mutates nothing. An active claim on the work unit rejects a duplicate
assign before one is attempted. A returned assignment carries a handoff block
naming the required profile, namespace, and the actions that stay forbidden.

Authorization reuses the #633 model rather than adding a second one. The new
initiate_workflow action is operator-class because its outcome is a claim, not
a Gitea verdict: requesting reviewer or merger work reserves that work but
grants no right to approve or merge. Execution is gated by a new per-action
execution_env_flag (WEBUI_REQUESTS_EXECUTION), deliberately in place of raising
ACTIVE_PHASE - a phase bump would enable execution for every phase-2 action at
once, including ones whose execution path is not implemented. Actions that
declare no flag are unchanged and still report execution_enabled false.

Every preview and apply emits a console audit record correlated to the
resulting assignment by correlation.request_id.

Fail-closed throughout: an unreadable control-plane DB, an incomplete queue
inventory (#758), an allocator that raises, an unpinned PR head, a moved PR
head, and an unconfirmed apply all deny without mutating.

Files:
- webui/request_service.py (new) - request model, preview, initiation
- webui/request_views.py (new) - form and preview rendering, escaped
- tests/test_webui_request_initiation.py (new) - 52 tests
- webui/console_authz.py - initiate_workflow action, execution_wired()
- webui/app.py - /requests, /api/v1/requests/preview, /api/v1/requests/apply
- webui/nav.py - Requests nav entry
- webui/traffic_loader.py - public candidates_from_queue_snapshot alias
- docs/webui-requests.md (new), docs/webui-authz-audit.md

Validation: full suite on this branch 5242 passed, 6 skipped, 899 subtests, 23
failed. Clean master baseline at 2f4dec83 in an equivalent branches/ worktree:
5190 passed, 6 skipped, 867 subtests, the same 23 tests failed. The branch adds
52 passing tests and introduces no new full-suite failure signature.

Closes #643

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

sysadmin commented

2026-07-25 01:20:19 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #902
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: review-902-433f66ad
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-902-head
phase: claimed
candidate_head: 433f66add8
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T06:20:18Z
expires_at: 2026-07-25T06:30:18Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #902 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: review-902-433f66ad worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-902-head phase: claimed candidate_head: 433f66add864062df7c211d9b52fc74fcfccfb2f target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T06:20:18Z expires_at: 2026-07-25T06:30:18Z blocker: none

sysadmin requested changes 2026-07-25 01:23:55 -05:00

sysadmin left a comment

Canonical PR State

STATE: PR-open
WHO_IS_NEXT: author
NEXT_ACTION: Fix B1 by releasing or explicitly surfacing an assignment the allocator created when the egress selection does not match the request, and by handling the post-commit exception route; fix B2 by keeping the preview path free of session-row writes and reusing one session id across the dry-run and the apply. Add coverage for both, including a test that fails if an orphaned assignment is left behind.
NEXT_PROMPT:

Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #643, PR #902, remote prgs. Read the reviewer REQUEST_CHANGES on PR #902 at head 433f66add864062df7c211d9b52fc74fcfccfb2f. Two code blockers must be resolved.

B1: in webui/request_service.py apply_request, when the allocator ran with apply=True and returned outcome assigned_work but _selection_matches fails against the requested work unit, the allocator has committed an assignment and lease via db.assign_and_lease; the current code returns mutation_performed false and leaves that lease orphaned. Release it or surface it with an explicit reclaim action, and report the mutation truthfully. The same state is reachable when _run_allocator's bare except Exception swallows an exception raised after the commit, since allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError, and ControlPlaneError. Note that candidate_set_fingerprint hashes only kind and number, so the CAS pin cannot detect the lease-state change that causes the divergence.

B2: allocate_next_work calls db.upsert_session and db.expire_stale_leases unconditionally before the apply branch, so every preview writes a new webui-request-<hex> session row and mutates lease state while the payload reports dry_run true and mutation_performed false. Keep the preview path free of session-row writes and reuse a single session id across the dry-run and the apply so the created lease has a resolvable owner.

Tests: tests/test_webui_request_initiation.py test_allocator_drift_on_apply_is_not_read_as_an_assignment currently asserts the defective behaviour and must be changed to require the compensating release. Add coverage for default_allocator past its two fail-closed early returns and for default_claims_source, both of which have none.

Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree and compare against the clean-master baseline at 2f4dec83, which is 23 failed / 5190 passed / 6 skipped / 867 subtests. Then push to feat/issue-643-request-preview-initiate and request a fresh review.

WHAT_HAPPENED: Independent reviewer review of PR #902 at head 433f66ad. Full suite re-run on both the PR head and a clean-master baseline worktree under branches/; three single-guard mutation runs; authorization gate and rendering surface verified independently. Two code blockers found in the initiation path.
WHY: The apply path can leave an allocator-created assignment orphaned while reporting mutation_performed false, and the preview path writes control-plane session rows and expires leases while declaring itself read-only.
ISSUE: 643
HEAD_SHA: 433f66add8
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: false
BLOCKERS: B1 — apply can orphan an allocator-created assignment on egress mismatch or post-commit exception and reports mutation_performed false (webui/request_service.py:745-781). B2 — the preview path writes a control-plane session row per call and calls expire_stale_leases while reporting mutation_performed false (webui/request_service.py:892-934, allocator_service.py:888-912).
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-902-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-902-head @433f66ad gave 23 failed / 5242 passed / 6 skipped / 899 subtests; sorted FAILED lines diff empty, 23 identical signatures. Mutation runs gave 1, 1, and 30 failures respectively.
LAST_UPDATED_BY: sysadmin (prgs-reviewer)

NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 433f66add8. No offline, import, or helper path was used.

[THREAD STATE LEDGER] PR #902 — REQUEST_CHANGES posted to Gitea

What is true now:

PR state: open
Server-side decision state: REQUEST_CHANGES posted to Gitea
Local verdict/state: REQUEST_CHANGES prepared locally
Latest known validation: branch 23 failed / 5242 passed / 6 skipped / 899 subtests, clean-master baseline 23 failed / 5190 passed / 6 skipped / 867 subtests, identical 23 failure signatures

What changed:

REQUEST_CHANGES posted to Gitea

What is blocked:

Blocker classification: code blocker

Who/what acts next:

Next actor: author
Required action: address B1 and B2 below and push fixes
Do not do: merge
Resume from: PR review comments

Server-side mutation ledger:

gitea_submit_pr_review → REQUEST_CHANGES posted to Gitea

Review — PR #902 (#643), head `433f66ad`

Two code blockers, both in the initiation path. The authorization gate and the
rendering surface were checked independently and are clean; details below so the
re-review can skip them.

B1 — an allocator-created assignment can be orphaned and reported as no mutation

webui/request_service.py:745-781, with allocator_service.py:741-757,
1101-1142, 1339, 1460-1513.

Failure scenario, concrete:

WEBUI_REQUESTS_EXECUTION=1. An operator POSTs /api/v1/requests/apply
with {desired_role: author, work_kind: issue, work_number: 664, confirm: true}.
apply_request re-previews at line 720. The dry-run calls
default_allocator(apply=False), which does a live load_queue_snapshot()
and gets candidates [issue#664, issue#665]; list_active_claims shows both
free; the allocator selects #664. Fingerprint fp = sha256([{issue,664},{issue,665}]).
In the window between that dry-run and the apply call at line 745, a
concurrent session takes a lease on #664. That window contains a second
full load_queue_snapshot() — a live Gitea fetch — so it is hundreds of
milliseconds to seconds wide, not microseconds. Foreign sessions acting on
this repo concurrently are an observed condition, not a hypothetical.
_run_allocator(apply=True) re-runs default_allocator. The queue is
unchanged, so cas_fp == fp and the #776 CAS pin passes.
candidate_set_fingerprint (allocator_service.py:741-757) hashes only
{kind, number} plus exclusions. Lease state is not an input to it, and
lease state is exactly what changed.
The selection loop (allocator_service.py:1101-1142) now skips #664 as
SKIP_CLAIMED_BY_OTHER_SESSION and continues to select #665, then
calls db.assign_and_lease at line 1339. A durable assignment row and lease
on #665 are committed. It returns outcome=assigned_work, selected=#665.
Back in apply_request, _selection_matches(#665, request #664) is False,
so assigned is False and control reaches _refuse(...) at line 772, which
returns "mutation_performed": False and "assignment": None.

Wrong outcome: a real lease on #665 exists and is owned by a synthetic
webui-request-<hex> session that no process heartbeats, resumes, or releases.
Nothing in this path releases it. #665 is unavailable to every other role
until the TTL expires, and the API response states that nothing changed, so
neither the operator nor any automation has a signal to reclaim it.

This is the direct answer to whether the CAS pin is sufficient: it is not, and
it cannot be made sufficient as written, because it is blind by construction to
the state transition that causes the divergence.

Second trigger, same class: the bare except Exception in _run_allocator
(line 864) returns None for any exception raised after db.assign_and_lease
has committed. allocate_next_work's own catch at allocator_service.py:1340
covers only InvalidWorkKindError, LeaseRequiredError, and
ControlPlaneError, so an unexpected exception in the post-commit result
construction (for example a KeyError on selection[...] at lines 1425/1469,
or inside _next_command) is swallowed and surfaces as
evidence_unavailable with mutation_performed: False — while the lease is
committed. So that bare except can indeed swallow a successful-but-partial result.

Coverage is inverted rather than absent.
tests/test_webui_request_initiation.py:589-613
(test_allocator_drift_on_apply_is_not_read_as_an_assignment) constructs
exactly this state — the fake allocator returns
assignment: {"assignment_id": "asn-wrong"} when apply=True — and then
asserts mutation_performed is False and assignment is None. The test
encodes the orphaned assignment as intended behaviour. No test asserts a
compensating release or any leak signal.

Fix direction: on an egress mismatch, release the assignment the allocator
created (or surface it explicitly with a reclaim action) and report
mutation_performed truthfully. A lease-state-aware CAS token would narrow the
window but does not remove the need for the compensating path, since the
post-commit exception route reaches the same state.

B2 — the preview path writes to the control-plane DB, contradicting its stated contract

webui/request_service.py:556-582 and 892-934, with
allocator_service.py:888-912.

Failure scenario, concrete:

An operator opens /requests and submits the preview form, or POSTs
/api/v1/requests/preview. No confirmation, no execution flag needed —
preview only requires an authenticated operator-role principal.
preview_request calls _run_allocator(apply=False) →
default_allocator → allocate_next_work(..., apply=False).
allocate_next_work calls db.upsert_session(session_id=..., role=..., profile=..., pid=os.getpid()) at allocator_service.py:889-896
unconditionally, before the apply branch is ever consulted, then
db.expire_stale_leases() at line 912.
default_allocator mints f"webui-request-{uuid.uuid4().hex[:12]}" fresh on
every call (line 920), so N previews write N session rows that are never
reused, never heartbeated, and never released.

Wrong outcome: the preview payload reports "dry_run": True and
"mutation_performed": False (request_service.py:296-297), the module
docstring says "Read-only, always. It creates no assignment and never mutates",
and the form footer says "Preview is read-only and creates no assignment" —
while each preview appends a control-plane session row and mutates global lease
state through expire_stale_leases(). Row growth is unbounded and driven by an
ordinary operator refreshing a form. This feeds the known post-restart
reconcile degradation (accumulated dead-pid session rows), which currently has
no reaper, so the rows persist.

The same defect distorts ownership on the apply path: a single apply writes
two session rows — one for the dry-run inside preview_request, one for the
apply call — and binds the resulting lease to the second. The assignment is
therefore owned by a session id that no process holds, which is also why B1's
orphan cannot be reclaimed by its owner.

Coverage is zero. default_allocator is exercised only at
tests/test_webui_request_initiation.py:640-684, and both cases
(inventory_complete=False, fetch_error) return before reaching
ControlPlaneDB() or allocate_next_work. default_claims_source has no test
at all. So upsert_session, the synthetic session id, and the CAS round-trip
are entirely untested.

Fix direction: do not write a session row on the preview path, and reuse one
session id across the dry-run and the apply so the created lease has a
resolvable owner.

Verified clean — no change requested

The execution gate did not loosen. ACTIVE_PHASE is still 1
(webui/console_authz.py:458). Every action in _ACTION_SPECS is phase 2 or 3;
there is no phase-1 action, so the action.phase <= ACTIVE_PHASE branch of
execution_wired never fires for any registered action today. initiate_workflow
is the only action declaring execution_env_flag, so every other action takes
the if not flag: return False path, reports execution_enabled: false, and
denies under for_execution. Checked specifically:
system.restart_namespace (phase 2, no flag) denies, and
webui/sanctioned_restart.py additionally hardcodes execution_enabled: False
in its own payloads at lines 275/315/399 and calls authorize() without
for_execution; /api/v1/analytics/usage gates on
decision.allowed and decision.execution_enabled at webui/app.py:672 for
record_analytics_usage (phase 2, no flag), so it stays dead.

One latent note, not a blocker: execution_enabled changed from a hardcoded
False to wired, and webui/app.py:672 gates a real write on that field. No
phase-1 action exists today, but the moment one is added it would report
execution_enabled: true without anyone touching the ingest route. Worth a
comment or an assertion that the registry holds no phase-1 action.

Rendering. Every interpolation in webui/request_views.py passes through
_escape (html.escape(..., quote=True)), including the allocator evidence
JSON at lines 105-126 and both error fields at lines 131-141. Attributes are
single-quoted and quote=True escapes ' as well as ", so the attribute
contexts are safe. No injection path found. Minor doc inaccuracy only: the
module docstring says it "renders only values the service already produced
rather than echoing a raw request body back", but _form(submitted) does
re-echo submitted values — escaped, so the behaviour is correct and only the
comment overstates it.

The self-check on list_active_claims. It is redundant for safety and
load-bearing only for reporting. The allocator independently re-reads claims and
skips claimed candidates, so a duplicate would be refused either way; the
console's own _lease_check changes the reason code and the blocked-vs-wait
classification, not the outcome. It is not misleading, but it must not be read
as the duplicate-assignment guarantee — the atomic db.assign_and_lease is that
guarantee.

The fail-closed claims that hold. Unreadable DB → _load_claims returns
None → _lease_check fails with evidence_unavailable, correct. Incomplete
inventory and fetch error → default_allocator returns None before touching
the DB, correct. Unpinned and moved head → _head_pin_check fails, correct.
Unconfirmed apply → refused at line 709 before the allocator runs, correct. The
allocator-exception claim is where it breaks down; see B1.

Mutation results

Each guard neutered one at a time in a throwaway worktree at the same head,
reverted after each run.

Guard	Mutation	Result
Duplicate-assignment check	`_lease_check`: `if claim:` → `if False:`	1 failed, 51 passed — `TestApplyOutcomes::test_duplicate_assignment_rejected`
`_selection_matches` egress guard	dropped the `and _selection_matches(...)` term from `assigned`	1 failed, 51 passed — `TestApplyOutcomes::test_allocator_drift_on_apply_is_not_read_as_an_assignment`
`execution_wired` flag	unflagged actions return `True` instead of `False`	30 failed across 4 files, including `test_execution_is_refused_while_phase_is_not_active`, `test_allowed_decision_never_reports_execution_enabled` (7 subtests), `test_analytics_ingest_unauthorized_denied`, and `test_preview_never_enables_execution`

All three guards have real coverage. Guards with zero coverage:
default_claims_source (no test), and everything in default_allocator past
the two fail-closed early returns — upsert_session, the synthetic session id,
and the CAS fingerprint round-trip. The _selection_matches guard has coverage
that fires, but as noted in B1 the assertion encodes the orphaned assignment
rather than catching it.

Validation, re-run by the reviewer

Both runs from branches/ worktrees, not /tmp, since several suites resolve
the repo root from the checkout path.

# clean-master baseline, branches/review-902-baseline @ 2f4dec83
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
23 failed, 5190 passed, 6 skipped, 867 subtests passed in 154.35s

# PR head, branches/review-902-head @ 433f66ad
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
23 failed, 5242 passed, 6 skipped, 899 subtests passed in 154.43s

diff of the sorted FAILED lines from both runs is empty — the 23 signatures
are identical. The author's reported numbers reproduce exactly. The branch adds
52 passing tests and introduces no new failure signature. Scope matches the
stated file list; no unrelated files in the diff.

Canonical Handoff

REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: 643
PR: 902
WORKFLOW_STATE: needs-author
HEAD_SHA: 433f66add864062df7c211d9b52fc74fcfccfb2f
BASE_BRANCH: master
BASE_OR_MERGE_SHA: 2f4dec832327513118f2fe92b74da25d124a01cb
ACTING_ROLE: reviewer
ACTING_IDENTITY: sysadmin (prgs-reviewer)
COMPLETED_ACTIONS: Independent review of PR #902 at head 433f66ad; full-suite re-run on both the PR head and a clean-master baseline worktree; three single-guard mutation runs; REQUEST_CHANGES posted to Gitea with two code blockers.
VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-902-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-902-head @433f66ad gave 23 failed / 5242 passed / 6 skipped / 899 subtests; sorted FAILED lines diff empty, 23 identical signatures. Mutation runs on tests/test_webui_request_initiation.py and the authz/analytics/restart suites gave 1, 1, and 30 failures respectively.
MUTATION_LEDGER: gitea_submit_pr_review to REQUEST_CHANGES posted to Gitea. No other server-side state changed. Local: three review worktrees created under branches/ and removed at the end of the session; no file edits to tracked branch content.
BLOCKERS: B1 — apply can orphan an allocator-created assignment on egress mismatch or post-commit exception and reports mutation_performed false (webui/request_service.py:745-781). B2 — the preview path writes a control-plane session row per call and calls expire_stale_leases while reporting mutation_performed false (webui/request_service.py:892-934, allocator_service.py:888-912).
NEXT_ACTOR: author
NEXT_ACTION: Fix B1 by releasing or explicitly surfacing an assignment the allocator created when the egress selection does not match the request, and by handling the post-commit exception route; fix B2 by keeping the preview path free of session-row writes and by reusing one session id across the dry-run and the apply. Add coverage for both, including a test that fails if an orphaned assignment is left behind.
PROHIBITED_ACTIONS: Do not merge. Do not self-approve. Do not treat the existing drift test as proof the egress guard is correct — it asserts the defective behaviour.
NEXT_PROMPT: Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #643, PR #902, remote prgs. Read the reviewer REQUEST_CHANGES on PR #902 at head 433f66add864062df7c211d9b52fc74fcfccfb2f. Two code blockers must be resolved. B1: in webui/request_service.py apply_request, when the allocator ran with apply=True and returned outcome assigned_work but _selection_matches fails against the requested work unit, the allocator has committed an assignment and lease via db.assign_and_lease; the current code returns mutation_performed false and leaves that lease orphaned. Release it or surface it with an explicit reclaim action, and report the mutation truthfully. The same state is reachable when _run_allocator's bare except Exception swallows an exception raised after the commit, since allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError, and ControlPlaneError. Note that candidate_set_fingerprint hashes only kind and number, so the CAS pin cannot detect the lease-state change that causes the divergence. B2: allocate_next_work calls db.upsert_session and db.expire_stale_leases unconditionally before the apply branch, so every preview writes a new webui-request-<hex> session row and mutates lease state while the payload reports dry_run true and mutation_performed false; keep the preview path free of session-row writes and reuse a single session id across the dry-run and the apply so the created lease has a resolvable owner. Update tests/test_webui_request_initiation.py: test_allocator_drift_on_apply_is_not_read_as_an_assignment currently asserts the defective behaviour and must be changed to require the compensating release. Add coverage for default_allocator past its two fail-closed early returns and for default_claims_source, both of which have none. Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree and compare against the clean-master baseline at 2f4dec83, which is 23 failed / 5190 passed / 6 skipped / 867 subtests. Then push to feat/issue-643-request-preview-initiate and request a fresh review.
WORKFLOW_FAILURE_ISSUES: none
LAST_UPDATED: 2026-07-25T06:22:00Z

## Canonical PR State STATE: PR-open WHO_IS_NEXT: author NEXT_ACTION: Fix B1 by releasing or explicitly surfacing an assignment the allocator created when the egress selection does not match the request, and by handling the post-commit exception route; fix B2 by keeping the preview path free of session-row writes and reusing one session id across the dry-run and the apply. Add coverage for both, including a test that fails if an orphaned assignment is left behind. NEXT_PROMPT: ```text Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #643, PR #902, remote prgs. Read the reviewer REQUEST_CHANGES on PR #902 at head 433f66add864062df7c211d9b52fc74fcfccfb2f. Two code blockers must be resolved. B1: in webui/request_service.py apply_request, when the allocator ran with apply=True and returned outcome assigned_work but _selection_matches fails against the requested work unit, the allocator has committed an assignment and lease via db.assign_and_lease; the current code returns mutation_performed false and leaves that lease orphaned. Release it or surface it with an explicit reclaim action, and report the mutation truthfully. The same state is reachable when _run_allocator's bare except Exception swallows an exception raised after the commit, since allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError, and ControlPlaneError. Note that candidate_set_fingerprint hashes only kind and number, so the CAS pin cannot detect the lease-state change that causes the divergence. B2: allocate_next_work calls db.upsert_session and db.expire_stale_leases unconditionally before the apply branch, so every preview writes a new webui-request-<hex> session row and mutates lease state while the payload reports dry_run true and mutation_performed false. Keep the preview path free of session-row writes and reuse a single session id across the dry-run and the apply so the created lease has a resolvable owner. Tests: tests/test_webui_request_initiation.py test_allocator_drift_on_apply_is_not_read_as_an_assignment currently asserts the defective behaviour and must be changed to require the compensating release. Add coverage for default_allocator past its two fail-closed early returns and for default_claims_source, both of which have none. Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree and compare against the clean-master baseline at 2f4dec83, which is 23 failed / 5190 passed / 6 skipped / 867 subtests. Then push to feat/issue-643-request-preview-initiate and request a fresh review. ``` WHAT_HAPPENED: Independent reviewer review of PR #902 at head 433f66ad. Full suite re-run on both the PR head and a clean-master baseline worktree under branches/; three single-guard mutation runs; authorization gate and rendering surface verified independently. Two code blockers found in the initiation path. WHY: The apply path can leave an allocator-created assignment orphaned while reporting mutation_performed false, and the preview path writes control-plane session rows and expires leases while declaring itself read-only. ISSUE: 643 HEAD_SHA: 433f66add864062df7c211d9b52fc74fcfccfb2f REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: false BLOCKERS: B1 — apply can orphan an allocator-created assignment on egress mismatch or post-commit exception and reports mutation_performed false (webui/request_service.py:745-781). B2 — the preview path writes a control-plane session row per call and calls expire_stale_leases while reporting mutation_performed false (webui/request_service.py:892-934, allocator_service.py:888-912). VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-902-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-902-head @433f66ad gave 23 failed / 5242 passed / 6 skipped / 899 subtests; sorted FAILED lines diff empty, 23 identical signatures. Mutation runs gave 1, 1, and 30 failures respectively. LAST_UPDATED_BY: sysadmin (prgs-reviewer) NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 433f66add864062df7c211d9b52fc74fcfccfb2f. No offline, import, or helper path was used. [THREAD STATE LEDGER] PR #902 — REQUEST_CHANGES posted to Gitea What is true now: - PR state: open - Server-side decision state: REQUEST_CHANGES posted to Gitea - Local verdict/state: REQUEST_CHANGES prepared locally - Latest known validation: branch 23 failed / 5242 passed / 6 skipped / 899 subtests, clean-master baseline 23 failed / 5190 passed / 6 skipped / 867 subtests, identical 23 failure signatures What changed: - REQUEST_CHANGES posted to Gitea What is blocked: - Blocker classification: code blocker Who/what acts next: - Next actor: author - Required action: address B1 and B2 below and push fixes - Do not do: merge - Resume from: PR review comments Server-side mutation ledger: - gitea_submit_pr_review → REQUEST_CHANGES posted to Gitea --- # Review — PR #902 (#643), head 433f66ad Two code blockers, both in the initiation path. The authorization gate and the rendering surface were checked independently and are clean; details below so the re-review can skip them. ## B1 — an allocator-created assignment can be orphaned and reported as no mutation `webui/request_service.py:745-781`, with `allocator_service.py:741-757`, `1101-1142`, `1339`, `1460-1513`. Failure scenario, concrete: 1. `WEBUI_REQUESTS_EXECUTION=1`. An operator POSTs `/api/v1/requests/apply` with `{desired_role: author, work_kind: issue, work_number: 664, confirm: true}`. 2. `apply_request` re-previews at line 720. The dry-run calls `default_allocator(apply=False)`, which does a live `load_queue_snapshot()` and gets candidates `[issue#664, issue#665]`; `list_active_claims` shows both free; the allocator selects `#664`. Fingerprint `fp = sha256([{issue,664},{issue,665}])`. 3. In the window between that dry-run and the apply call at line 745, a concurrent session takes a lease on `#664`. That window contains a second full `load_queue_snapshot()` — a live Gitea fetch — so it is hundreds of milliseconds to seconds wide, not microseconds. Foreign sessions acting on this repo concurrently are an observed condition, not a hypothetical. 4. `_run_allocator(apply=True)` re-runs `default_allocator`. The queue is unchanged, so `cas_fp == fp` and the #776 CAS pin **passes**. `candidate_set_fingerprint` (`allocator_service.py:741-757`) hashes only `{kind, number}` plus exclusions. Lease state is not an input to it, and lease state is exactly what changed. 5. The selection loop (`allocator_service.py:1101-1142`) now skips `#664` as `SKIP_CLAIMED_BY_OTHER_SESSION` and `continue`s to select **`#665`**, then calls `db.assign_and_lease` at line 1339. A durable assignment row and lease on `#665` are committed. It returns `outcome=assigned_work`, `selected=#665`. 6. Back in `apply_request`, `_selection_matches(#665, request #664)` is False, so `assigned` is False and control reaches `_refuse(...)` at line 772, which returns `"mutation_performed": False` and `"assignment": None`. Wrong outcome: a real lease on `#665` exists and is owned by a synthetic `webui-request-<hex>` session that no process heartbeats, resumes, or releases. Nothing in this path releases it. `#665` is unavailable to every other role until the TTL expires, and the API response states that nothing changed, so neither the operator nor any automation has a signal to reclaim it. This is the direct answer to whether the CAS pin is sufficient: it is not, and it cannot be made sufficient as written, because it is blind by construction to the state transition that causes the divergence. Second trigger, same class: the bare `except Exception` in `_run_allocator` (line 864) returns `None` for any exception raised after `db.assign_and_lease` has committed. `allocate_next_work`'s own catch at `allocator_service.py:1340` covers only `InvalidWorkKindError`, `LeaseRequiredError`, and `ControlPlaneError`, so an unexpected exception in the post-commit result construction (for example a `KeyError` on `selection[...]` at lines 1425/1469, or inside `_next_command`) is swallowed and surfaces as `evidence_unavailable` with `mutation_performed: False` — while the lease is committed. So that bare except can indeed swallow a successful-but-partial result. Coverage is inverted rather than absent. `tests/test_webui_request_initiation.py:589-613` (`test_allocator_drift_on_apply_is_not_read_as_an_assignment`) constructs exactly this state — the fake allocator returns `assignment: {"assignment_id": "asn-wrong"}` when `apply=True` — and then asserts `mutation_performed` is False and `assignment` is None. The test encodes the orphaned assignment as intended behaviour. No test asserts a compensating release or any leak signal. Fix direction: on an egress mismatch, release the assignment the allocator created (or surface it explicitly with a reclaim action) and report `mutation_performed` truthfully. A lease-state-aware CAS token would narrow the window but does not remove the need for the compensating path, since the post-commit exception route reaches the same state. ## B2 — the preview path writes to the control-plane DB, contradicting its stated contract `webui/request_service.py:556-582` and `892-934`, with `allocator_service.py:888-912`. Failure scenario, concrete: 1. An operator opens `/requests` and submits the preview form, or POSTs `/api/v1/requests/preview`. No confirmation, no execution flag needed — preview only requires an authenticated operator-role principal. 2. `preview_request` calls `_run_allocator(apply=False)` → `default_allocator` → `allocate_next_work(..., apply=False)`. 3. `allocate_next_work` calls `db.upsert_session(session_id=..., role=..., profile=..., pid=os.getpid())` at `allocator_service.py:889-896` **unconditionally, before the apply branch is ever consulted**, then `db.expire_stale_leases()` at line 912. 4. `default_allocator` mints `f"webui-request-{uuid.uuid4().hex[:12]}"` fresh on every call (line 920), so N previews write N session rows that are never reused, never heartbeated, and never released. Wrong outcome: the preview payload reports `"dry_run": True` and `"mutation_performed": False` (`request_service.py:296-297`), the module docstring says "Read-only, always. It creates no assignment and never mutates", and the form footer says "Preview is read-only and creates no assignment" — while each preview appends a control-plane session row and mutates global lease state through `expire_stale_leases()`. Row growth is unbounded and driven by an ordinary operator refreshing a form. This feeds the known post-restart reconcile degradation (accumulated dead-pid session rows), which currently has no reaper, so the rows persist. The same defect distorts ownership on the apply path: a single apply writes **two** session rows — one for the dry-run inside `preview_request`, one for the apply call — and binds the resulting lease to the second. The assignment is therefore owned by a session id that no process holds, which is also why B1's orphan cannot be reclaimed by its owner. Coverage is zero. `default_allocator` is exercised only at `tests/test_webui_request_initiation.py:640-684`, and both cases (`inventory_complete=False`, `fetch_error`) return before reaching `ControlPlaneDB()` or `allocate_next_work`. `default_claims_source` has no test at all. So `upsert_session`, the synthetic session id, and the CAS round-trip are entirely untested. Fix direction: do not write a session row on the preview path, and reuse one session id across the dry-run and the apply so the created lease has a resolvable owner. ## Verified clean — no change requested **The execution gate did not loosen.** `ACTIVE_PHASE` is still `1` (`webui/console_authz.py:458`). Every action in `_ACTION_SPECS` is phase 2 or 3; there is no phase-1 action, so the `action.phase <= ACTIVE_PHASE` branch of `execution_wired` never fires for any registered action today. `initiate_workflow` is the only action declaring `execution_env_flag`, so every other action takes the `if not flag: return False` path, reports `execution_enabled: false`, and denies under `for_execution`. Checked specifically: `system.restart_namespace` (phase 2, no flag) denies, and `webui/sanctioned_restart.py` additionally hardcodes `execution_enabled: False` in its own payloads at lines 275/315/399 and calls `authorize()` without `for_execution`; `/api/v1/analytics/usage` gates on `decision.allowed and decision.execution_enabled` at `webui/app.py:672` for `record_analytics_usage` (phase 2, no flag), so it stays dead. One latent note, not a blocker: `execution_enabled` changed from a hardcoded `False` to `wired`, and `webui/app.py:672` gates a real write on that field. No phase-1 action exists today, but the moment one is added it would report `execution_enabled: true` without anyone touching the ingest route. Worth a comment or an assertion that the registry holds no phase-1 action. **Rendering.** Every interpolation in `webui/request_views.py` passes through `_escape` (`html.escape(..., quote=True)`), including the allocator evidence JSON at lines 105-126 and both error fields at lines 131-141. Attributes are single-quoted and `quote=True` escapes `'` as well as `"`, so the attribute contexts are safe. No injection path found. Minor doc inaccuracy only: the module docstring says it "renders only values the service already produced rather than echoing a raw request body back", but `_form(submitted)` does re-echo submitted values — escaped, so the behaviour is correct and only the comment overstates it. **The self-check on `list_active_claims`.** It is redundant for safety and load-bearing only for reporting. The allocator independently re-reads claims and skips claimed candidates, so a duplicate would be refused either way; the console's own `_lease_check` changes the reason code and the `blocked`-vs-`wait` classification, not the outcome. It is not misleading, but it must not be read as the duplicate-assignment guarantee — the atomic `db.assign_and_lease` is that guarantee. **The fail-closed claims that hold.** Unreadable DB → `_load_claims` returns `None` → `_lease_check` fails with `evidence_unavailable`, correct. Incomplete inventory and fetch error → `default_allocator` returns `None` before touching the DB, correct. Unpinned and moved head → `_head_pin_check` fails, correct. Unconfirmed apply → refused at line 709 before the allocator runs, correct. The allocator-exception claim is where it breaks down; see B1. ## Mutation results Each guard neutered one at a time in a throwaway worktree at the same head, reverted after each run. | Guard | Mutation | Result | |---|---|---| | Duplicate-assignment check | `_lease_check`: `if claim:` → `if False:` | **1 failed**, 51 passed — `TestApplyOutcomes::test_duplicate_assignment_rejected` | | `_selection_matches` egress guard | dropped the `and _selection_matches(...)` term from `assigned` | **1 failed**, 51 passed — `TestApplyOutcomes::test_allocator_drift_on_apply_is_not_read_as_an_assignment` | | `execution_wired` flag | unflagged actions return `True` instead of `False` | **30 failed** across 4 files, including `test_execution_is_refused_while_phase_is_not_active`, `test_allowed_decision_never_reports_execution_enabled` (7 subtests), `test_analytics_ingest_unauthorized_denied`, and `test_preview_never_enables_execution` | All three guards have real coverage. Guards with **zero** coverage: `default_claims_source` (no test), and everything in `default_allocator` past the two fail-closed early returns — `upsert_session`, the synthetic session id, and the CAS fingerprint round-trip. The `_selection_matches` guard has coverage that fires, but as noted in B1 the assertion encodes the orphaned assignment rather than catching it. ## Validation, re-run by the reviewer Both runs from `branches/` worktrees, not `/tmp`, since several suites resolve the repo root from the checkout path. ```text # clean-master baseline, branches/review-902-baseline @ 2f4dec83 WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q 23 failed, 5190 passed, 6 skipped, 867 subtests passed in 154.35s # PR head, branches/review-902-head @ 433f66ad WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q 23 failed, 5242 passed, 6 skipped, 899 subtests passed in 154.43s ``` `diff` of the sorted `FAILED` lines from both runs is empty — the 23 signatures are identical. The author's reported numbers reproduce exactly. The branch adds 52 passing tests and introduces no new failure signature. Scope matches the stated file list; no unrelated files in the diff.  ## Canonical Handoff ```text REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools ISSUE: 643 PR: 902 WORKFLOW_STATE: needs-author HEAD_SHA: 433f66add864062df7c211d9b52fc74fcfccfb2f BASE_BRANCH: master BASE_OR_MERGE_SHA: 2f4dec832327513118f2fe92b74da25d124a01cb ACTING_ROLE: reviewer ACTING_IDENTITY: sysadmin (prgs-reviewer) COMPLETED_ACTIONS: Independent review of PR #902 at head 433f66ad; full-suite re-run on both the PR head and a clean-master baseline worktree; three single-guard mutation runs; REQUEST_CHANGES posted to Gitea with two code blockers. VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-902-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-902-head @433f66ad gave 23 failed / 5242 passed / 6 skipped / 899 subtests; sorted FAILED lines diff empty, 23 identical signatures. Mutation runs on tests/test_webui_request_initiation.py and the authz/analytics/restart suites gave 1, 1, and 30 failures respectively. MUTATION_LEDGER: gitea_submit_pr_review to REQUEST_CHANGES posted to Gitea. No other server-side state changed. Local: three review worktrees created under branches/ and removed at the end of the session; no file edits to tracked branch content. BLOCKERS: B1 — apply can orphan an allocator-created assignment on egress mismatch or post-commit exception and reports mutation_performed false (webui/request_service.py:745-781). B2 — the preview path writes a control-plane session row per call and calls expire_stale_leases while reporting mutation_performed false (webui/request_service.py:892-934, allocator_service.py:888-912). NEXT_ACTOR: author NEXT_ACTION: Fix B1 by releasing or explicitly surfacing an assignment the allocator created when the egress selection does not match the request, and by handling the post-commit exception route; fix B2 by keeping the preview path free of session-row writes and by reusing one session id across the dry-run and the apply. Add coverage for both, including a test that fails if an orphaned assignment is left behind. PROHIBITED_ACTIONS: Do not merge. Do not self-approve. Do not treat the existing drift test as proof the egress guard is correct — it asserts the defective behaviour. NEXT_PROMPT: Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #643, PR #902, remote prgs. Read the reviewer REQUEST_CHANGES on PR #902 at head 433f66add864062df7c211d9b52fc74fcfccfb2f. Two code blockers must be resolved. B1: in webui/request_service.py apply_request, when the allocator ran with apply=True and returned outcome assigned_work but _selection_matches fails against the requested work unit, the allocator has committed an assignment and lease via db.assign_and_lease; the current code returns mutation_performed false and leaves that lease orphaned. Release it or surface it with an explicit reclaim action, and report the mutation truthfully. The same state is reachable when _run_allocator's bare except Exception swallows an exception raised after the commit, since allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError, and ControlPlaneError. Note that candidate_set_fingerprint hashes only kind and number, so the CAS pin cannot detect the lease-state change that causes the divergence. B2: allocate_next_work calls db.upsert_session and db.expire_stale_leases unconditionally before the apply branch, so every preview writes a new webui-request-<hex> session row and mutates lease state while the payload reports dry_run true and mutation_performed false; keep the preview path free of session-row writes and reuse a single session id across the dry-run and the apply so the created lease has a resolvable owner. Update tests/test_webui_request_initiation.py: test_allocator_drift_on_apply_is_not_read_as_an_assignment currently asserts the defective behaviour and must be changed to require the compensating release. Add coverage for default_allocator past its two fail-closed early returns and for default_claims_source, both of which have none. Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree and compare against the clean-master baseline at 2f4dec83, which is 23 failed / 5190 passed / 6 skipped / 867 subtests. Then push to feat/issue-643-request-preview-initiate and request a fresh review. WORKFLOW_FAILURE_ISSUES: none LAST_UPDATED: 2026-07-25T06:22:00Z ```

sysadmin referenced this pull request

2026-07-25 01:25:05 -05:00

Web Console: Requests, intent preview, authorization, and workflow initiation (Phase 2) #643

sysadmin commented

2026-07-25 01:25:17 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #902
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: review-902-433f66ad
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-902-head
phase: released
candidate_head: 433f66add8
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T06:25:17Z
expires_at: 2026-07-25T06:35:17Z
blocker: manual-release

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #902 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: review-902-433f66ad worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-902-head phase: released candidate_head: 433f66add864062df7c211d9b52fc74fcfccfb2f target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T06:25:17Z expires_at: 2026-07-25T06:35:17Z blocker: manual-release

jcwalker3 referenced this issue from a commit

2026-07-25 02:43:28 -05:00

fix(webui): compensate stray allocations and keep preview side-effect free

jcwalker3 added 1 commit 2026-07-25 02:43:28 -05:00

fix(webui): compensate stray allocations and keep preview side-effect free 53ce1b1a5e

Addresses both blockers from the PR #902 review (review 589) for issue #643.

B1 — an allocator-created assignment could be orphaned and reported as no
mutation.

apply_request re-previews, then re-runs the allocator with apply=True. The CAS
fingerprint hashes only {kind, number} plus exclusions, so a competing lease
taken on the requested unit inside the window leaves the fingerprint identical:
the pin passes, the selection loop skips the now-claimed unit and commits an
assignment on the *next* one, and _selection_matches then fails on egress. The
old code returned mutation_performed False with that lease still committed and
owned by a synthetic session nothing heartbeats. The window contains a second
full load_queue_snapshot(), so it is seconds wide, and foreign sessions acting
on this repo concurrently are an observed condition.

The egress mismatch now releases the assignment the allocator created before
refusing. When the release succeeds the refusal reports mutation_performed
False and a compensation record; when it fails the response carries
mutation_performed True, an explicit orphaned_assignment, and a
gitea_release_workflow_lease reclaim action, because claiming nothing changed
while a lease is live is the defect rather than a report of it.

The same state was reachable through _run_allocator's bare except Exception:
allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError and
ControlPlaneError, so anything raised after assign_and_lease committed arrived
as "no result" with a durable lease. A None result on the apply path now sweeps
and releases whatever this flow's session owns. That sweep is only possible
because of the B2 fix below — the session id is now stable across the flow, so
the lease is findable.

B2 — the "read-only" preview wrote to the control-plane DB.

allocate_next_work called db.upsert_session and db.expire_stale_leases
unconditionally, before the apply branch was consulted, and default_allocator
minted a fresh webui-request-<hex> per call. Every preview therefore appended a
never-reused session row and mutated global lease state while the payload said
dry_run True / mutation_performed False, driven by an operator refreshing a
form. One apply wrote two rows and bound the lease to the second, which is why
B1's orphan had no reclaimable owner.

allocate_next_work gains a keyword-only side_effect_free flag, default False so
every existing caller is byte-for-byte unchanged. Under the flag both writes are
suppressed and expired leases are instead filtered out of the claim map in
memory, which reaches the same selection the sweep would have produced without
persisting anything; a claim whose expiry cannot be parsed is kept, since an
unreadable expiry is not evidence that work is free. side_effect_free with
apply=True fails closed rather than silently reserving. request_service mints
one session id per request flow and threads it through both the dry-run and the
apply, and the dry-run now routes through the side-effect-free path.

Coverage.

test_allocator_drift_on_apply_is_not_read_as_an_assignment asserted the defect —
it built the orphan state and then required mutation_performed to be False, which
a leak satisfies. It now requires the compensating release. Added: release-failure
surfacing a reclaim action, an assignment with no lease id, the post-commit
exception route, and session-id identity across the flow. The two areas the review
named as having zero coverage now have it: default_allocator past its two
fail-closed early returns (side_effect_free routing, session-id pass-through and
minting, scope and fingerprint propagation) and default_claims_source (scoped
read, and an unreadable substrate denying rather than reading as "nothing
claimed"). allocator_service gains side-effect-free tests against a real
temp-file DB plus the expiry-filter unit tests.

Every new guard was mutation-tested by reverting it one at a time: B1
compensation removed → 3 failures; post-commit sweep removed → 1; session id
re-minted per call → 2; side_effect_free ignored → 3; in-memory expiry filter
removed → 1.

Verification: WEBUI_TEST_OFFLINE=1 python -m pytest tests/ -q from this
branches/ worktree gives 23 failed, 5263 passed, 6 skipped, 899 subtests. The
sorted FAILED set is identical to the reviewer's clean-master baseline at
2f4dec83 (23 failed, 5190 passed) — no new, changed, or disappeared failure —
and 21 tests were added over the reviewed head's 5242. Zero conflict markers;
py_compile passes; git diff --check clean.

Refs #643, PR #902

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01V6xFqovhbArPv61j9KCGkL

jcwalker3 added 1 commit 2026-07-25 02:44:59 -05:00

Merge branch 'master' into feat/issue-643-request-preview-initiate 6da68fffb8

jcwalker3 commented

2026-07-25 02:52:06 -05:00

Author remediation of review 589 — both blockers addressed

Reviewed head 433f66add864062df7c211d9b52fc74fcfccfb2f → new head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77. The fix landed as 53ce1b1 (fast-forward, non-force, no rebase); master 8598537a was then integrated by update-by-merge, since PR #886 landed while this work was in progress and branch protection requires a current base.

B1 — the apply path no longer leaves an assignment behind

The egress mismatch now releases what the allocator committed:

committed = bool(outcome == allocator_service.OUTCOME_ASSIGNED and assignment)
assigned = bool(committed and _selection_matches(allocation.get("selected"), request))
if not assigned:
    if committed:
        compensation = _release_stray_assignment(request, allocation,
                                                 session_id=session_id)

When the release succeeds the refusal reports mutation_performed: false plus a compensation record; when it fails the response carries mutation_performed: true, an explicit orphaned_assignment, and a gitea_release_workflow_lease reclaim action naming the lease and its owning session. Reporting "nothing changed" while a lease is live was the defect, so the truthful signal is now part of the contract rather than an afterthought.

The post-commit exception route reaches the same state and is handled too. allocate_next_work catches only InvalidWorkKindError, LeaseRequiredError and ControlPlaneError, so anything else raised after assign_and_lease committed arrives at _run_allocator as None with a durable lease. A None result on the apply path now sweeps and releases whatever this flow's session owns. That sweep is only possible because of B2: the session id is stable across the flow, so the lease is findable.

The CAS fingerprint itself is unchanged. As the review noted, it is blind by construction to the lease-state transition that causes the divergence, so narrowing the window would not remove the need for the compensating path.

B2 — the preview no longer writes to the control-plane DB

allocate_next_work gains a keyword-only side_effect_free flag, defaulting to False so every existing caller — the MCP tool and all 13 test modules — is unchanged. Under the flag upsert_session and expire_stale_leases are both suppressed, and expired leases are instead filtered out of the claim map in memory by _drop_expired_claims, which reaches the same selection the sweep would have produced without persisting anything. A claim whose expires_at cannot be parsed is kept: an unreadable expiry is not evidence that work is free. side_effect_free together with apply=True fails closed rather than silently reserving.

request_service now mints one session id per request flow via _new_session_id() and threads it through both the dry-run and the apply, and default_allocator routes a dry run through the side-effect-free path. An apply therefore writes one session row instead of two, and the lease it creates is owned by an id the request flow still holds.

Coverage

test_allocator_drift_on_apply_is_not_read_as_an_assignment asserted the defect — it built the orphan state and then required mutation_performed to be False, which a leak satisfies just as well. It now requires the compensating release and asserts the released lease id.

Added for B1: release-failure surfacing the reclaim action, an assignment carrying no lease id, the post-commit exception route, and session-id identity across the flow. Added for the two areas the review identified as having zero coverage: default_allocator past both fail-closed early returns (side-effect-free routing, session-id pass-through and minting, scope and fingerprint propagation) and default_claims_source (scoped read, plus an unreadable substrate denying rather than reading as "nothing claimed"). tests/test_allocator_service.py gains side-effect-free tests against a real temp-file control-plane DB, plus unit tests for the expiry filter.

Every new guard was mutation-tested by reverting it one at a time in the worktree and restoring after each run:

Guard neutered	Failures
B1 compensation branch removed	3
post-commit sweep removed	1
session id re-minted per call	2
`side_effect_free` ignored	3
in-memory expiry filter removed	1

Validation

Both runs from branches/ worktrees, never /tmp. The baseline was re-measured at the new master, since the review's 2f4dec83 baseline predates PR #886 landing.

# clean-master baseline, branches/baseline-master-8598537a @ 8598537a
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
23 failed, 5230 passed, 6 skipped, 912 subtests passed

# PR head, branches/feat-issue-643-request-preview-initiate @ 6da68fff
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
23 failed, 5303 passed, 6 skipped, 944 subtests passed

The sorted FAILED diff between the two is empty — 23 identical signatures, no new failure, none changed, none disappeared. The 23 are pre-existing on clean master and are disclosed rather than concealed; none is touched by this change. The branch adds 73 passing tests over that baseline: 52 from the original #643 work plus 21 from this remediation. Targeted suites tests/test_webui_request_initiation.py and tests/test_allocator_service.py: 88 passed, 26 subtests.

Also verified: zero conflict markers; py_compile passes on all four changed files; git diff --check clean; Closes #643 remains in the PR body; scope is four files staged explicitly with no unrelated change; and mergeable_after: false from update-by-merge was stale cache — git merge-base --is-ancestor 8598537a HEAD exits 0, and live state now reports mergeable: true, has_conflicts: false, commits_behind: 0.

Review status

Review 589 was recorded at 433f66ad and does not carry to this head. approval_at_current_head: false, stale_approval: true, recommended_next_action: fresh_review_required. A fresh independent review at 6da68fffb889a0e99e6fd648046f2a2e3c54fc77 is required.

[THREAD STATE LEDGER]

what is true now:

Server-side decision state: PR #902 is in open state at head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77, base master @ 8598537a3547b646bfeedf6f96482959489746dd. Gitea reports mergeable: true, has_conflicts: false, commits_behind: 0, approval_at_current_head: false. The only review verdict on this PR is the REQUEST_CHANGES recorded at the former head 433f66ad.
Local verdict/state: both blockers from review 589 are implemented and verified in the bound worktree; full suite 23 failed / 5303 passed against a clean-master baseline of 23 failed / 5230 passed, with an identical failure set.

what changed:

B1: an allocator-committed assignment that fails the egress selection check is released, with compensation, or surfaced as orphaned_assignment with a reclaim action when the release fails; the post-commit exception route sweeps the flow's session leases.
B2: allocate_next_work gained side_effect_free (default False); the preview path writes no session row and sweeps no leases; one session id spans the dry-run and the apply.
Tests: the inverted drift test now requires the release; coverage added for default_allocator past its early returns and for default_claims_source; five guards mutation-tested.
Branch advanced 433f66ad → 53ce1b1 (fast-forward publish) → 6da68fff (update-by-merge bringing in master 8598537a).

what is blocked:

Blocker classification: no blocker
Nothing is waiting on the author role.

who/what acts next:

Next actor: prgs-reviewer
Required action: perform a fresh independent review of PR #902 at exact head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77 and record a verdict at that exact head.
Do not do: do not reuse review 589 recorded at 433f66ad; do not treat the former drift test as evidence about the egress guard, since it previously asserted the defect; do not rebase or force-push this branch; do not merge from the author role.

Canonical Issue State

STATE: awaiting-review

WHO_IS_NEXT: reviewer

NEXT_ACTION: Perform a fresh independent review of PR #902 at exact head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77 and record a formal verdict at that exact head.

BLOCKERS: none

VALIDATION: clean-master baseline @8598537a = 23 failed / 5230 passed / 6 skipped / 912 subtests; head @6da68fff = 23 failed / 5303 passed / 6 skipped / 944 subtests; sorted FAILED diff empty, 23 identical signatures; targeted suites 88 passed / 26 subtests; five guards mutation-tested at 3 / 1 / 2 / 3 / 1 failures; zero conflict markers; py_compile passes; git diff --check clean.

LAST_UPDATED_BY: jcwalker3 / prgs-author / 2026-07-25T07:55:00Z

WHAT_HAPPENED: An author session implemented both blockers from reviewer review 589 — the compensating release for an allocator-created assignment that fails the egress check, and a genuinely side-effect-free preview path — added regression coverage for each including the two zero-coverage areas the review named, corrected the drift test that asserted the defect, published the fix, and integrated master after PR #886 landed.

WHY: The apply path could commit an assignment for a different work unit than the one requested and then report that nothing was mutated, and the preview path wrote a control-plane session row and swept leases on every call while declaring itself read-only.

RELATED_PRS: #902 (this PR), #886 (landed on master as 8598537a during this remediation)

NEXT_PROMPT:

As prgs-reviewer on PR #902 at exact head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77: this is a fresh independent review — review 589 at 433f66ad is stale, so do not reuse it. Verify B1, that an allocator-committed assignment failing _selection_matches is released and that a failed release surfaces mutation_performed true with an explicit orphaned_assignment and reclaim action, and that the post-commit exception route sweeps the flow's session leases. Verify B2, that allocate_next_work under side_effect_free writes no session row and calls no expire_stale_leases, that expired claims are filtered in memory so free work stays selectable while an unparseable expiry is kept, that side_effect_free with apply=True fails closed, and that one session id spans the dry-run and the apply. Confirm the drift test now requires the compensating release rather than asserting the leak. Measure your own baseline at master 8598537a in a branches/ worktree rather than accepting the author's numbers. Record APPROVE or REQUEST_CHANGES at that exact head; stop.

## Author remediation of review 589 — both blockers addressed Reviewed head `433f66add864062df7c211d9b52fc74fcfccfb2f` → new head `6da68fffb889a0e99e6fd648046f2a2e3c54fc77`. The fix landed as `53ce1b1` (fast-forward, non-force, no rebase); master `8598537a` was then integrated by update-by-merge, since PR #886 landed while this work was in progress and branch protection requires a current base. ### B1 — the apply path no longer leaves an assignment behind The egress mismatch now releases what the allocator committed: ```python committed = bool(outcome == allocator_service.OUTCOME_ASSIGNED and assignment) assigned = bool(committed and _selection_matches(allocation.get("selected"), request)) if not assigned: if committed: compensation = _release_stray_assignment(request, allocation, session_id=session_id) ``` When the release succeeds the refusal reports `mutation_performed: false` plus a `compensation` record; when it fails the response carries `mutation_performed: true`, an explicit `orphaned_assignment`, and a `gitea_release_workflow_lease` reclaim action naming the lease and its owning session. Reporting "nothing changed" while a lease is live was the defect, so the truthful signal is now part of the contract rather than an afterthought. The post-commit exception route reaches the same state and is handled too. `allocate_next_work` catches only `InvalidWorkKindError`, `LeaseRequiredError` and `ControlPlaneError`, so anything else raised after `assign_and_lease` committed arrives at `_run_allocator` as `None` with a durable lease. A `None` result on the apply path now sweeps and releases whatever this flow's session owns. That sweep is only possible because of B2: the session id is stable across the flow, so the lease is findable. The CAS fingerprint itself is unchanged. As the review noted, it is blind by construction to the lease-state transition that causes the divergence, so narrowing the window would not remove the need for the compensating path. ### B2 — the preview no longer writes to the control-plane DB `allocate_next_work` gains a keyword-only `side_effect_free` flag, defaulting to `False` so every existing caller — the MCP tool and all 13 test modules — is unchanged. Under the flag `upsert_session` and `expire_stale_leases` are both suppressed, and expired leases are instead filtered out of the claim map in memory by `_drop_expired_claims`, which reaches the same selection the sweep would have produced without persisting anything. A claim whose `expires_at` cannot be parsed is **kept**: an unreadable expiry is not evidence that work is free. `side_effect_free` together with `apply=True` fails closed rather than silently reserving. `request_service` now mints one session id per request flow via `_new_session_id()` and threads it through both the dry-run and the apply, and `default_allocator` routes a dry run through the side-effect-free path. An apply therefore writes one session row instead of two, and the lease it creates is owned by an id the request flow still holds. ### Coverage `test_allocator_drift_on_apply_is_not_read_as_an_assignment` asserted the defect — it built the orphan state and then required `mutation_performed` to be `False`, which a leak satisfies just as well. It now requires the compensating release and asserts the released lease id. Added for B1: release-failure surfacing the reclaim action, an assignment carrying no lease id, the post-commit exception route, and session-id identity across the flow. Added for the two areas the review identified as having zero coverage: `default_allocator` past both fail-closed early returns (side-effect-free routing, session-id pass-through and minting, scope and fingerprint propagation) and `default_claims_source` (scoped read, plus an unreadable substrate denying rather than reading as "nothing claimed"). `tests/test_allocator_service.py` gains side-effect-free tests against a real temp-file control-plane DB, plus unit tests for the expiry filter. Every new guard was mutation-tested by reverting it one at a time in the worktree and restoring after each run: | Guard neutered | Failures | |---|---| | B1 compensation branch removed | 3 | | post-commit sweep removed | 1 | | session id re-minted per call | 2 | | `side_effect_free` ignored | 3 | | in-memory expiry filter removed | 1 | ### Validation Both runs from `branches/` worktrees, never `/tmp`. The baseline was re-measured at the **new** master, since the review's `2f4dec83` baseline predates PR #886 landing. ```text # clean-master baseline, branches/baseline-master-8598537a @ 8598537a WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q 23 failed, 5230 passed, 6 skipped, 912 subtests passed # PR head, branches/feat-issue-643-request-preview-initiate @ 6da68fff WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q 23 failed, 5303 passed, 6 skipped, 944 subtests passed ``` The sorted `FAILED` diff between the two is empty — 23 identical signatures, no new failure, none changed, none disappeared. The 23 are pre-existing on clean master and are disclosed rather than concealed; none is touched by this change. The branch adds 73 passing tests over that baseline: 52 from the original #643 work plus 21 from this remediation. Targeted suites `tests/test_webui_request_initiation.py` and `tests/test_allocator_service.py`: 88 passed, 26 subtests. Also verified: zero conflict markers; `py_compile` passes on all four changed files; `git diff --check` clean; `Closes #643` remains in the PR body; scope is four files staged explicitly with no unrelated change; and `mergeable_after: false` from update-by-merge was stale cache — `git merge-base --is-ancestor 8598537a HEAD` exits 0, and live state now reports `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`. ### Review status Review 589 was recorded at `433f66ad` and does not carry to this head. `approval_at_current_head: false`, `stale_approval: true`, `recommended_next_action: fresh_review_required`. A fresh independent review at `6da68fffb889a0e99e6fd648046f2a2e3c54fc77` is required. [THREAD STATE LEDGER] what is true now: - Server-side decision state: PR #902 is in open state at head `6da68fffb889a0e99e6fd648046f2a2e3c54fc77`, base `master` @ `8598537a3547b646bfeedf6f96482959489746dd`. Gitea reports `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`, `approval_at_current_head: false`. The only review verdict on this PR is the REQUEST_CHANGES recorded at the former head `433f66ad`. - Local verdict/state: both blockers from review 589 are implemented and verified in the bound worktree; full suite 23 failed / 5303 passed against a clean-master baseline of 23 failed / 5230 passed, with an identical failure set. what changed: - B1: an allocator-committed assignment that fails the egress selection check is released, with `compensation`, or surfaced as `orphaned_assignment` with a reclaim action when the release fails; the post-commit exception route sweeps the flow's session leases. - B2: `allocate_next_work` gained `side_effect_free` (default False); the preview path writes no session row and sweeps no leases; one session id spans the dry-run and the apply. - Tests: the inverted drift test now requires the release; coverage added for `default_allocator` past its early returns and for `default_claims_source`; five guards mutation-tested. - Branch advanced `433f66ad` → `53ce1b1` (fast-forward publish) → `6da68fff` (update-by-merge bringing in master `8598537a`). what is blocked: - Blocker classification: no blocker - Nothing is waiting on the author role. who/what acts next: - Next actor: prgs-reviewer - Required action: perform a fresh independent review of PR #902 at exact head `6da68fffb889a0e99e6fd648046f2a2e3c54fc77` and record a verdict at that exact head. - Do not do: do not reuse review 589 recorded at `433f66ad`; do not treat the former drift test as evidence about the egress guard, since it previously asserted the defect; do not rebase or force-push this branch; do not merge from the author role. ## Canonical Issue State STATE: awaiting-review WHO_IS_NEXT: reviewer NEXT_ACTION: Perform a fresh independent review of PR #902 at exact head `6da68fffb889a0e99e6fd648046f2a2e3c54fc77` and record a formal verdict at that exact head. BLOCKERS: none VALIDATION: clean-master baseline @8598537a = 23 failed / 5230 passed / 6 skipped / 912 subtests; head @6da68fff = 23 failed / 5303 passed / 6 skipped / 944 subtests; sorted FAILED diff empty, 23 identical signatures; targeted suites 88 passed / 26 subtests; five guards mutation-tested at 3 / 1 / 2 / 3 / 1 failures; zero conflict markers; py_compile passes; git diff --check clean. LAST_UPDATED_BY: jcwalker3 / prgs-author / 2026-07-25T07:55:00Z WHAT_HAPPENED: An author session implemented both blockers from reviewer review 589 — the compensating release for an allocator-created assignment that fails the egress check, and a genuinely side-effect-free preview path — added regression coverage for each including the two zero-coverage areas the review named, corrected the drift test that asserted the defect, published the fix, and integrated master after PR #886 landed. WHY: The apply path could commit an assignment for a different work unit than the one requested and then report that nothing was mutated, and the preview path wrote a control-plane session row and swept leases on every call while declaring itself read-only. RELATED_PRS: #902 (this PR), #886 (landed on master as 8598537a during this remediation) NEXT_PROMPT: ```text As prgs-reviewer on PR #902 at exact head 6da68fffb889a0e99e6fd648046f2a2e3c54fc77: this is a fresh independent review — review 589 at 433f66ad is stale, so do not reuse it. Verify B1, that an allocator-committed assignment failing _selection_matches is released and that a failed release surfaces mutation_performed true with an explicit orphaned_assignment and reclaim action, and that the post-commit exception route sweeps the flow's session leases. Verify B2, that allocate_next_work under side_effect_free writes no session row and calls no expire_stale_leases, that expired claims are filtered in memory so free work stays selectable while an unparseable expiry is kept, that side_effect_free with apply=True fails closed, and that one session id spans the dry-run and the apply. Confirm the drift test now requires the compensating release rather than asserting the leak. Measure your own baseline at master 8598537a in a branches/ worktree rather than accepting the author's numbers. Record APPROVE or REQUEST_CHANGES at that exact head; stop. ```

You are not authorized to merge this pull request.

This pull request can be merged automatically.

View command line instructions

Checkout

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

git fetch -u origin feat/issue-643-request-preview-initiate:feat/issue-643-request-preview-initiate

git checkout feat/issue-643-request-preview-initiate

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#902