Capability map, resolver, and router can disagree on review-task roles; role-stamp mismatch escapes as unhandled internal_error in submit path #723

Open
opened 2026-07-16 17:32:11 -05:00 by jcwalker3 · 2 comments
Owner

Product defects exposed by incident #722

Two distinct code defects allowed commit 970e68b to strand formal reviews and to fail with an unactionable error. The break-glass repair for #722 restores the mappings and adds first-line invariant tests; the defects below need their own reviewed fixes.

Defect A — no invariant ties the capability map to configured profiles or the router

task_capability_map.py is documented as "the single source of truth; regression tests assert tool gates cannot drift from it", but nothing asserts:

  1. Profile coverage: for every role-exclusive formal-review task, at least one enabled configured profile satisfies both the required permission and the required role. After 970e68b, review_pr/approve_pr/request_changes_pr required role=merger while every merger profile forbids the review permissions → matching_configured_profile: [] and a repository-wide review lockout that no test named directly.
  2. Map ↔ router agreement: role_session_router.REVIEWER_TASKS still said reviewer while the map said merger. Two authorities, no consistency test.
  3. Runtime-context consistency: _build_runtime_task_capabilities reports permission-only allowed_in_current_session/matching_configured_profiles (no role-exclusive filter), so gitea_get_runtime_context reported review_pr as allowed while gitea_resolve_task_capability fail-closed the same task in the same session.

Defect B — capability/preflight disagreement surfaces as generic internal_error

Observed on PR #721 (reviewer session, prgs-reviewer):

  1. gitea_resolve_task_capability("review_pr") stamps _preflight_resolved_role via record_preflight_check(...) before computing/denying allowed_in_current_session, so a denied resolve still poisons the session role stamp (here: merger).
  2. gitea_mark_final_review_decision has no role-kind gate in its chain (daemon guard → decision lock → session → workflow load → head → eligibility action "review" is permission-only) → it succeeded and persisted a durable ready-APPROVE decision lock.
  3. gitea_submit_pr_review_evaluate_pr_review_submission_verify_role_mutation_workspace_effective_workspace_role() returns the stamped mergernwb.assess_namespace_mutation_workspace(role_kind="merger", ...) blocks against the reviewer session → raise RuntimeError(...) escapes the tool uncaught → the client sees a generic internal_error instead of a structured fail-closed result with reasons.

Net effect: mark/submit half-completed states with no actionable error. Compare #718 (same generic-internal_error pattern in the adopt-lease tool).

Suggested acceptance criteria

  • AC1: A test fails if any role-exclusive task's (permission, role) pair has no enabled profile in the canonical role-profile shape satisfying both.
  • AC2: A test fails if any task in REVIEWER_TASKS has required_role(task) != "reviewer" (and equivalents for MERGER_TASKS).
  • AC3: record_preflight_check must not stamp a resolved role for a task the resolve denied (or the stamp must carry the denial), so a later mutation preflight fails closed with structured reasons instead of raising.
  • AC4: _evaluate_pr_review_submission converts workspace/role-binding failures into result["reasons"] (fail closed) rather than propagating exceptions as internal_error.
  • AC5: gitea_get_runtime_context task capabilities apply the same role-exclusive filter as the resolver, or explicitly label their view as permission-only.

References

## Product defects exposed by incident #722 Two distinct code defects allowed commit `970e68b` to strand formal reviews and to fail with an unactionable error. The break-glass repair for #722 restores the mappings and adds first-line invariant tests; the defects below need their own reviewed fixes. ### Defect A — no invariant ties the capability map to configured profiles or the router `task_capability_map.py` is documented as "the single source of truth; regression tests assert tool gates cannot drift from it", but nothing asserts: 1. **Profile coverage:** for every role-exclusive formal-review task, at least one enabled configured profile satisfies both the required permission and the required role. After `970e68b`, `review_pr`/`approve_pr`/`request_changes_pr` required `role=merger` while every merger profile forbids the review permissions → `matching_configured_profile: []` and a repository-wide review lockout that no test named directly. 2. **Map ↔ router agreement:** `role_session_router.REVIEWER_TASKS` still said `reviewer` while the map said `merger`. Two authorities, no consistency test. 3. **Runtime-context consistency:** `_build_runtime_task_capabilities` reports permission-only `allowed_in_current_session`/`matching_configured_profiles` (no role-exclusive filter), so `gitea_get_runtime_context` reported `review_pr` as allowed while `gitea_resolve_task_capability` fail-closed the same task in the same session. ### Defect B — capability/preflight disagreement surfaces as generic `internal_error` Observed on PR #721 (reviewer session, prgs-reviewer): 1. `gitea_resolve_task_capability("review_pr")` stamps `_preflight_resolved_role` via `record_preflight_check(...)` **before** computing/denying `allowed_in_current_session`, so a denied resolve still poisons the session role stamp (here: `merger`). 2. `gitea_mark_final_review_decision` has no role-kind gate in its chain (daemon guard → decision lock → session → workflow load → head → eligibility action "review" is permission-only) → it **succeeded** and persisted a durable ready-APPROVE decision lock. 3. `gitea_submit_pr_review` → `_evaluate_pr_review_submission` → `_verify_role_mutation_workspace` → `_effective_workspace_role()` returns the stamped `merger` → `nwb.assess_namespace_mutation_workspace(role_kind="merger", ...)` blocks against the reviewer session → `raise RuntimeError(...)` escapes the tool uncaught → the client sees a generic `internal_error` instead of a structured fail-closed result with reasons. Net effect: mark/submit half-completed states with no actionable error. Compare #718 (same generic-`internal_error` pattern in the adopt-lease tool). ### Suggested acceptance criteria - AC1: A test fails if any role-exclusive task's (permission, role) pair has no enabled profile in the canonical role-profile shape satisfying both. - AC2: A test fails if any task in `REVIEWER_TASKS` has `required_role(task) != "reviewer"` (and equivalents for `MERGER_TASKS`). - AC3: `record_preflight_check` must not stamp a resolved role for a task the resolve denied (or the stamp must carry the denial), so a later mutation preflight fails closed with structured reasons instead of raising. - AC4: `_evaluate_pr_review_submission` converts workspace/role-binding failures into `result["reasons"]` (fail closed) rather than propagating exceptions as `internal_error`. - AC5: `gitea_get_runtime_context` task capabilities apply the same role-exclusive filter as the resolver, or explicitly label their view as permission-only. ### References - Incident: #722 (root cause, timeline, break-glass authorization) - Regressive commit: `970e68bddb510aef1b2c4de58981f03ed570e98e` - Stranded review: PR #721 @ `80f59b334e6671b08006725292c08a8e8b6c823f`, comments 11891/11893/11895 - Related: #718, #685, #664, #671
jcwalker3 added the mcpstatus:readyworkflow-hardeningsafetytype:bug labels 2026-07-16 17:32:11 -05:00
Author
Owner

Author claim — issue #723 (comment-based; label tools degraded)

Claiming this issue for implementation. gitea_mark_issue / gitea_set_issue_labels currently return internal_error (daemon subprocess spawn failure per gitea_diagnose_terminal), so this comment is the durable claim signal; the status:in-progress label will be reconciled when the label path recovers.

Branch: fix/issue-723-capability-role-invariants
Worktree: branches/fix-issue-723-capability-role-invariants (fresh from master 67e4a2b5e96b680a1dcb90a20c2ebf87357a3b1f, clean)

Live corroboration of Defect B this session (2026-07-17 ~04:30Z): gitea_acquire_reviewer_pr_lease, gitea_mark_issue, and gitea_set_issue_labels all surfaced generic internal_error while an underlying subprocess spawn failure was the real cause — the same unactionable-error pattern this issue names in AC4.

Canonical Issue State

STATE: claimed for implementation by prgs-author at master 67e4a2b5e9
WHO_IS_NEXT: author
NEXT_ACTION: implement AC1-AC5 with tests in branches/fix-issue-723-capability-role-invariants, run full suite, open the implementing PR
NEXT_PROMPT:

Role: AUTHOR (continuation). Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools. Issue #723.
Continue in branches/fix-issue-723-capability-role-invariants: AC1 profile-coverage invariant, AC2 map/router agreement tests, AC3 no role stamp on denied resolve, AC4 structured fail-closed reasons in _evaluate_pr_review_submission instead of escaping RuntimeError, AC5 runtime-context role-exclusive filter or explicit permission-only labeling. Full suite green, open PR, stop (no self-review or merge).

WHAT_HAPPENED: author picked up the issue after reproducing the Defect B internal_error pattern live; existing first-line invariant tests from the #722 break-glass repair (tests/test_task_capability_role_invariants.py at ba79154) will be extended rather than duplicated
WHY: two authorities (capability map, router) plus permission-only runtime reporting can silently disagree, and role-stamp mismatches escape as unactionable internal_error; fail-closed structured reasons are required
RELATED_PRS: implementing PR to follow from branch fix/issue-723-capability-role-invariants
BLOCKERS: none for implementation; label-claim tooling degraded (internal_error) documented above
VALIDATION: pending — focused invariant/resolver/submission suites plus full suite will be recorded in the PR
LAST_UPDATED_BY: prgs-author (jcwalker3), 2026-07-17

[THREAD STATE LEDGER]
what is true now: issue #723 remains open and is claimed by prgs-author via this comment; worktree branches/fix-issue-723-capability-role-invariants exists clean at master 67e4a2b5.
what changed: author claim recorded; live Defect B corroboration captured from this session's tool failures.
what is blocked: only the label-based claim signal; classification below. Implementation proceeds now.
who/what acts next: author implements AC1-AC5 in the named worktree.
Server-side decision state: no review or merge decision exists for this issue; implementation not yet delivered
Local verdict/state: fresh clean worktree at master head; no local edits yet
Next actor: author
Required action: implement AC1-AC5 with tests, validate, open PR
Blocker classification: environment/tooling blocker
Do not do: do not modify the #722 break-glass invariant tests' intent; do not self-review or merge the eventual PR; do not touch capability behavior beyond the five acceptance criteria

## Author claim — issue #723 (comment-based; label tools degraded) Claiming this issue for implementation. `gitea_mark_issue` / `gitea_set_issue_labels` currently return `internal_error` (daemon subprocess spawn failure per `gitea_diagnose_terminal`), so this comment is the durable claim signal; the `status:in-progress` label will be reconciled when the label path recovers. Branch: `fix/issue-723-capability-role-invariants` Worktree: `branches/fix-issue-723-capability-role-invariants` (fresh from master `67e4a2b5e96b680a1dcb90a20c2ebf87357a3b1f`, clean) Live corroboration of Defect B this session (2026-07-17 ~04:30Z): `gitea_acquire_reviewer_pr_lease`, `gitea_mark_issue`, and `gitea_set_issue_labels` all surfaced generic `internal_error` while an underlying subprocess spawn failure was the real cause — the same unactionable-error pattern this issue names in AC4. ## Canonical Issue State STATE: claimed for implementation by prgs-author at master 67e4a2b5e96b680a1dcb90a20c2ebf87357a3b1f WHO_IS_NEXT: author NEXT_ACTION: implement AC1-AC5 with tests in branches/fix-issue-723-capability-role-invariants, run full suite, open the implementing PR NEXT_PROMPT: ```text Role: AUTHOR (continuation). Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools. Issue #723. Continue in branches/fix-issue-723-capability-role-invariants: AC1 profile-coverage invariant, AC2 map/router agreement tests, AC3 no role stamp on denied resolve, AC4 structured fail-closed reasons in _evaluate_pr_review_submission instead of escaping RuntimeError, AC5 runtime-context role-exclusive filter or explicit permission-only labeling. Full suite green, open PR, stop (no self-review or merge). ``` WHAT_HAPPENED: author picked up the issue after reproducing the Defect B internal_error pattern live; existing first-line invariant tests from the #722 break-glass repair (tests/test_task_capability_role_invariants.py at ba79154) will be extended rather than duplicated WHY: two authorities (capability map, router) plus permission-only runtime reporting can silently disagree, and role-stamp mismatches escape as unactionable internal_error; fail-closed structured reasons are required RELATED_PRS: implementing PR to follow from branch fix/issue-723-capability-role-invariants BLOCKERS: none for implementation; label-claim tooling degraded (internal_error) documented above VALIDATION: pending — focused invariant/resolver/submission suites plus full suite will be recorded in the PR LAST_UPDATED_BY: prgs-author (jcwalker3), 2026-07-17 [THREAD STATE LEDGER] what is true now: issue #723 remains open and is claimed by prgs-author via this comment; worktree branches/fix-issue-723-capability-role-invariants exists clean at master 67e4a2b5. what changed: author claim recorded; live Defect B corroboration captured from this session's tool failures. what is blocked: only the label-based claim signal; classification below. Implementation proceeds now. who/what acts next: author implements AC1-AC5 in the named worktree. Server-side decision state: no review or merge decision exists for this issue; implementation not yet delivered Local verdict/state: fresh clean worktree at master head; no local edits yet Next actor: author Required action: implement AC1-AC5 with tests, validate, open PR Blocker classification: environment/tooling blocker Do not do: do not modify the #722 break-glass invariant tests' intent; do not self-review or merge the eventual PR; do not touch capability behavior beyond the five acceptance criteria
Author
Owner

Implementation delivered — PR creation pending tooling recovery

AC1–AC5 are implemented, tested, committed, and pushed. gitea_lock_issue and gitea_create_pr currently return internal_error (same daemon subprocess spawn failure noted in the claim comment), so the pull request itself is not yet open. No raw-API bypass was used.

Branch: fix/issue-723-capability-role-invariants
Head: bc9366c (pushed to prgs)
Worktree: branches/fix-issue-723-capability-role-invariants

Changes:

  • task_capability_map.ROLE_EXCLUSIVE_TASKS shared constant; resolver inline copy removed (AC1/AC5 drift surface gone)
  • AC3: denied resolve no longer stamps _preflight_resolved_role; stale stamps cleared via _clear_resolved_capability_stamp
  • AC4: _evaluate_pr_review_submission returns structured reasons + blocker_kind=workspace_role_binding instead of letting RuntimeError escape as generic internal_error
  • AC5: _build_runtime_task_capabilities applies the resolver's role-exclusive filter (entries labeled role_filtered/permission_only); matching_configured_profiles honors declared roles
  • Tests: tests/test_issue_723_role_stamp_and_submission.py (new) + extensions to tests/test_task_capability_role_invariants.py

Test evidence: focused 22 passed (+48 subtests); adjacent suites 72 passed; full suite 2929 passed / 6 skipped / 221 subtests (single pre-existing Starlette warning, #682).

Observed adjacent drift, documented not fixed: delete_branch maps to author but only the reconciler holds gitea.branch.delete; router RECONCILER_TASKS lists reconcile-landed-pr while the map says author; reconcile_already_landed absent from the map.

Canonical Issue State

STATE: implementation complete on pushed branch fix/issue-723-capability-role-invariants at head bc9366c; pull request not yet open because PR-creation tooling returns internal_error
WHO_IS_NEXT: author
NEXT_ACTION: retry gitea_lock_issue + gitea_create_pr for branch fix/issue-723-capability-role-invariants once the MCP daemon subprocess spawn failure clears (client reconnect or daemon restart), then hand off to an independent reviewer
NEXT_PROMPT:

Role: AUTHOR (continuation). Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools. Issue #723.
Branch fix/issue-723-capability-role-invariants is pushed at head bc9366c with full-suite test evidence recorded. Run gitea_lock_issue(723, fix/issue-723-capability-role-invariants) then gitea_create_pr(head=fix/issue-723-capability-role-invariants, base=master) with the prepared PR body, and stop (no self-review or merge).

WHAT_HAPPENED: author implemented AC1-AC5 with new and extended tests, ran the full suite green, committed bc9366c, and pushed the branch; lock/create-PR tools failed with internal_error so the PR remains to be opened
WHY: the shared role-exclusive constant plus stamp/structured-error fixes close both defects; the PR step is mechanically prepared and needs only working tooling
RELATED_PRS: implementing PR to be opened from fix/issue-723-capability-role-invariants
BLOCKERS: gitea_lock_issue and gitea_create_pr return internal_error (daemon subprocess spawn failure); comment path unaffected
VALIDATION: full suite 2929 passed / 6 skipped / 221 subtests in the worktree; focused suites recorded above
LAST_UPDATED_BY: prgs-author (jcwalker3), 2026-07-17

[THREAD STATE LEDGER]
what is true now: branch fix/issue-723-capability-role-invariants carries the complete AC1-AC5 implementation at pushed head bc9366c with the full test suite green; no pull request exists yet for it.
what changed: implementation, tests, commit, and push completed since the claim comment; lock/create-PR attempts recorded internal_error.
what is blocked: only the lock + PR-creation step; classification below.
who/what acts next: author retries lock + create_pr after tooling recovery, then an independent reviewer takes the PR.
Server-side decision state: no review or merge decision exists; no PR is open for this branch
Local verdict/state: worktree clean at bc9366c; all changes committed and pushed
Next actor: author
Required action: retry gitea_lock_issue then gitea_create_pr for the pushed branch after MCP daemon recovery
Blocker classification: environment/tooling blocker
Do not do: do not open the PR via raw API bypass; do not self-review or merge; do not rework the branch while the PR step is pending

## Implementation delivered — PR creation pending tooling recovery AC1–AC5 are implemented, tested, committed, and pushed. `gitea_lock_issue` and `gitea_create_pr` currently return `internal_error` (same daemon subprocess spawn failure noted in the claim comment), so the pull request itself is not yet open. No raw-API bypass was used. Branch: `fix/issue-723-capability-role-invariants` Head: `bc9366c` (pushed to prgs) Worktree: `branches/fix-issue-723-capability-role-invariants` Changes: - `task_capability_map.ROLE_EXCLUSIVE_TASKS` shared constant; resolver inline copy removed (AC1/AC5 drift surface gone) - AC3: denied resolve no longer stamps `_preflight_resolved_role`; stale stamps cleared via `_clear_resolved_capability_stamp` - AC4: `_evaluate_pr_review_submission` returns structured `reasons` + `blocker_kind=workspace_role_binding` instead of letting RuntimeError escape as generic `internal_error` - AC5: `_build_runtime_task_capabilities` applies the resolver's role-exclusive filter (entries labeled `role_filtered`/`permission_only`); `matching_configured_profiles` honors declared roles - Tests: `tests/test_issue_723_role_stamp_and_submission.py` (new) + extensions to `tests/test_task_capability_role_invariants.py` Test evidence: focused 22 passed (+48 subtests); adjacent suites 72 passed; full suite 2929 passed / 6 skipped / 221 subtests (single pre-existing Starlette warning, #682). Observed adjacent drift, documented not fixed: `delete_branch` maps to author but only the reconciler holds `gitea.branch.delete`; router `RECONCILER_TASKS` lists `reconcile-landed-pr` while the map says author; `reconcile_already_landed` absent from the map. ## Canonical Issue State STATE: implementation complete on pushed branch fix/issue-723-capability-role-invariants at head bc9366c; pull request not yet open because PR-creation tooling returns internal_error WHO_IS_NEXT: author NEXT_ACTION: retry gitea_lock_issue + gitea_create_pr for branch fix/issue-723-capability-role-invariants once the MCP daemon subprocess spawn failure clears (client reconnect or daemon restart), then hand off to an independent reviewer NEXT_PROMPT: ```text Role: AUTHOR (continuation). Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools. Issue #723. Branch fix/issue-723-capability-role-invariants is pushed at head bc9366c with full-suite test evidence recorded. Run gitea_lock_issue(723, fix/issue-723-capability-role-invariants) then gitea_create_pr(head=fix/issue-723-capability-role-invariants, base=master) with the prepared PR body, and stop (no self-review or merge). ``` WHAT_HAPPENED: author implemented AC1-AC5 with new and extended tests, ran the full suite green, committed bc9366c, and pushed the branch; lock/create-PR tools failed with internal_error so the PR remains to be opened WHY: the shared role-exclusive constant plus stamp/structured-error fixes close both defects; the PR step is mechanically prepared and needs only working tooling RELATED_PRS: implementing PR to be opened from fix/issue-723-capability-role-invariants BLOCKERS: gitea_lock_issue and gitea_create_pr return internal_error (daemon subprocess spawn failure); comment path unaffected VALIDATION: full suite 2929 passed / 6 skipped / 221 subtests in the worktree; focused suites recorded above LAST_UPDATED_BY: prgs-author (jcwalker3), 2026-07-17 [THREAD STATE LEDGER] what is true now: branch fix/issue-723-capability-role-invariants carries the complete AC1-AC5 implementation at pushed head bc9366c with the full test suite green; no pull request exists yet for it. what changed: implementation, tests, commit, and push completed since the claim comment; lock/create-PR attempts recorded internal_error. what is blocked: only the lock + PR-creation step; classification below. who/what acts next: author retries lock + create_pr after tooling recovery, then an independent reviewer takes the PR. Server-side decision state: no review or merge decision exists; no PR is open for this branch Local verdict/state: worktree clean at bc9366c; all changes committed and pushed Next actor: author Required action: retry gitea_lock_issue then gitea_create_pr for the pushed branch after MCP daemon recovery Blocker classification: environment/tooling blocker Do not do: do not open the PR via raw API bypass; do not self-review or merge; do not rework the branch while the PR step is pending
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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