Reviewed-on: #679
Reviewed-by: sysadmin <[email protected]>
Operator break-glass merge of PR #679 authorized because the native MCP
gitea_adopt_merger_pr_lease path returned non-retryable internal_error
despite an authoritative active reviewer lease. PR #679 was formally
approved by Review #446 at exact head
78cc37a977 and was conflict-free and
mergeable. No LLM received credentials or used a direct API/CLI fallback.
EXCEPTIONAL OPERATOR-AUTHORIZED BREAK-GLASS RECOVERY - incident #722.
Commit 970e68b ("fix: adopt_merger_pr_lease requires merger role") was an
unreviewed direct push that flipped 11 task_capability_map.py entries to
role="merger" instead of the intended 1. Merger profiles forbid the review
permissions, so no configured profile could resolve review_pr / approve_pr /
request_changes_pr (matching_configured_profile was empty repository-wide)
and no fix PR could be formally reviewed. The operator resolved the catch-22
with a one-time break-glass authorization recorded on issue #722
(comment 11918); this commit is the authorized minimum and nothing more.
Changes:
- task_capability_map.py restored to blob 02826af (the preserved intended
correction, local commit c071f8a1): the 10 regressive entries return to
role="reviewer"; adopt_merger_pr_lease keeps role="merger"; merge_pr is
untouched.
- tests/test_task_capability_role_invariants.py added (#723 AC1/AC2):
profile-coverage invariant for formal review tasks, map/router agreement,
merger-only adopt_merger_pr_lease and merge_pr, merger profiles cannot
hold review permissions.
Validation: focused role-mapping tests 72 passed (+22 subtests); full suite
2900 passed, 6 skipped, 1 known warning, 195 subtests.
Recovery step 1 for incident #722; code-defect follow-ups tracked in #723.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EnHNSVQvJ8nCk7KZ9kL4Ym
Terminal review-decision ledgers are recovery-critical provenance, not
disposable session cache. A generic four-hour TTL previously made
fresh_review_on_current_head_allowed unreachable after age expiry on open
PRs (PR #616 / review 443 reproduction).
- Classify KIND_DECISION_LOCK as RECOVERY_CRITICAL so load/mark_final work
after >4h without hand-editing session-state files
- Stamp kind + recovery_critical on save for compatibility
- Add inspect_state_envelope so assessment reports on-disk evidence instead
of silent "no lock" when TTL would hide non-critical kinds
- Surface disk_inspect on stale decision-lock cleanup assessment
Preserves same-head #332 hard-stop, #620 head-scoped fresh review, #594
moot cleanup, and #709 irrecoverable authorization (no ordinary-profile
permission grant).
Require branch-scoped post-delete not-found (not generic/repo/host 404),
emit consistent top-level cleanup fields, fail closed on ownership inventory
errors, never auto-reclaim expired control-plane leases, include active
comment-backed reviewer leases, apply the same gates to reconcile_merged_cleanups,
and match ownership with normalized host identity.
Require authoritative not-found readback after merged-PR branch DELETE, and
block cleanup when active author/reviewer/merger/controller/reconciler
session, lease, or worktree-binding ownership still uses the target branch.
Address formal review 438 REQUEST_CHANGES on PR #710 (F6/F7/F8).
F6 — HMAC key-version validation fails closed:
- verify_authorization_artifact validates key_version BEFORE any MAC work, so an
attacker-chosen version can never select the signing key.
- Require exactly one nonempty, well-formed key-version field; missing, empty,
unknown, malformed, duplicated (including identical-valued and nested
aliases), and mismatched versions all fail. No versionless legacy fallback.
- Artifact version must equal the configured active version; production now
requires GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION explicitly (an implicit
default made rotation ambiguous). Version stays inside the signed material.
F7 — strictly canonical incident evidence:
- Replace substring/first-match parsing with an exact schema: marker on line 1,
every field once, fixed order, no duplicate/unknown/empty/conflicting fields
in or outside the signed block. The parsed body is re-rendered and compared
for exact equality before acceptance.
- content_digest now binds the full recovery scope: repository identity, PR,
decision-lock identity, destroyed subject, recovery action, recorded and
expected head, incident issue, evidence author, minting actor, key version,
nonce and issued_at.
- Actor identity is the immutable user id with login consistency; conflicting
ids/logins and display-name-only identities fail closed. Edited comments are
rejected. The independent-author rule is preserved and enforced by stable id.
- build_canonical_incident_body is the single source of the accepted format and
refuses to emit ambiguous evidence.
F8 — archival is a prerequisite for clearing terminal evidence:
- _clear_decision_lock_for_profile no longer swallows archive failures. It
requires a successful write plus a durable read-back matching the PR/head,
and otherwise returns a structured, retry-safe failure that retains the lock
and records actionable recovery evidence.
- Fixes a latent bug the read-back exposed: the archive payload inherited the
source lock's session_profile_lock, so save_state keyed the archive under the
reviewer profile instead of the archive identity and it never read back.
Adversarial regressions added for every listed case: key-version missing/empty/
unknown/malformed/duplicate/rotation/wrong-key-after-restart, reordered fields,
duplicate identical and conflicting fields, conflicting actor ids/names,
digest-preserving substitution, decision-lock and recovery-action substitution,
cross-PR/repo/org/remote/head replay, archive exception/timeout/false/empty/
partial-readback with proof the lock survives, exactly-one permitted clear, and
retry after archive failure. No existing assertion was weakened.
Validation: focused 150 passed in three module orders; full tests/ 2809 passed,
6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in
tests/test_webui_audit.py:8), 161 subtests passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Prevent merger-local empty decision locks from standing in for reviewer
terminal cleanup, refuse silent re-init overwrite of unresolved terminal
evidence, record post-merge recovery-required state when audit fails, and
add a truthful irrecoverable-provenance path that never claims applied=true.
Closes#709
Remove the public allow_test_bootstrap production seam so caller-controlled
flags cannot forge native mutation provenance. Bind provenance to the resolved
canonical entrypoint path plus a live stdio transport bind; basename-only
mcp_server.py stack frames and import-only launch no longer authorize
mutations. Test-mode install_test_native_runtime is pytest-only and cannot
reach production Gitea mutation endpoints. Add AC9 regressions for both
reviewer-found bypasses and related spoof vectors.
Refs: PR #696 REQUEST_CHANGES at 253269c; issue #695 comments 11002/11005.
Bind mutation/credential paths to a process-local native MCP runtime so env
spoofing, direct imports, and offline helpers cannot reconstruct session gates
after native transport failure. Quarantine contaminated formal reviews under
controller authority and honor quarantine in review feedback, merge eligibility,
merge mutation, and canonical handoff validation. Add regression coverage for
the second (PR #694 / review 427) incident class.
Fixes the final-report validator defects from Issue #698 (original lead
plus the independent reproduction recorded during the PR #703 formal
review, comment 11246):
- Legacy fields: the review/merger required-field tables no longer demand
'Pinned reviewed head', 'Scratch worktree used', 'Worktree path',
'Worktree dirty', 'Mutations', 'Next', 'Issue/PR', 'Branch/SHA', or
'Files changed' — names the canonical review-merge-final-report schema
forbids or replaces. The canonical names ('Reviewed head SHA', 'Review
worktree path/dirty', 'Safe next action', mutation categories) are
required instead, with backward-compatible aliases where the schema
permits them.
- Structured proof: workflow-load helper results are recognized in
colon, key=value, and JSON renderings; validation pass proof is
accepted anywhere in the Validation field value (for example
'focused 50 passed; full 2665 passed').
- Mutation inference: review mutations are inferred only from
authoritative evidence (performed=true and not gated); read-only
diagnostics and pre-API rejections no longer count as mutations.
- Lease release vs cleanup: canonical reviewer lease release (release
tool call or terminal phase=released marker) is lease lifecycle, not
post-merge cleanup, and no longer triggers the branch/worktree cleanup
checklist; genuine delete/remove claims still require full proof.
- Blocked reports: a legitimately blocked run that states an explicit
'Reviewed/Candidate head SHA: none' with no verdict, merge, or started
validation owes no head proofs; approval-time and merge-time live-head
proofs are demanded only once the corresponding phase begins, and a
report that states no head at all still fails closed.
- action_log robustness: malformed (non-dict) entries and non-list logs
are reported as clear sanitized findings (position and type only, no
content echo) instead of crashing with AttributeError; a defective
validator rule now fails closed with a sanitized block finding rather
than raising a secondary exception.
27 new regression tests, including canonical fixtures modeled on the
PR #703 formal-review handoff and the blocked preflight report from the
prior #698 reproductions. Two legacy-field test fixtures updated to the
canonical schema. Full suite: 2663 passed, 6 skipped, 161 subtests.
Co-Authored-By: Claude Fable 5 <[email protected]>
Add fail-closed assessment and MCP tool for expired/superseded-head
comment-backed reviewer leases that lack a control-plane lease_id.
- assess_obsolete_reviewer_comment_lease_cleanup eligibility matrix
- diagnose classifications: foreign_active_current_head,
foreign_expired_current_head, foreign_completed_superseded_head,
foreign_expired_superseded_head, orphaned_owner_missing,
ambiguous_conflicting_evidence
- gitea_cleanup_obsolete_reviewer_comment_lease with confirmation
CLEANUP OBSOLETE REVIEWER LEASE <n> and controller_recovery_authorized
- preserve audit history via append-only phase=released marker
- never repoint, transfer validation, use PID ownership, or steal
active current-head foreign leases
- regression for PR #688 lease comment 10749 after recorded expiry
Merged-PR source-branch cleanup is reconciler work (task_capability_map maps
cleanup_merged_pr_branch -> reconciler / gitea.branch.delete), but the
reconciler profile schema, execution-profile docs, and tests never covered
the permission, so no configured profile could run the guarded
gitea_cleanup_merged_pr_branch path.
- reconciler_profile.py: add gitea.branch.delete to
RECONCILER_RECOMMENDED_OPERATIONS (not required; not forbidden)
- docs/gitea-execution-profiles.md: document merged-branch cleanup
ownership, least-privilege constraints, and the no-alias caveat
- tests/test_reconciler_profile.py: reconciler profile with branch.delete
stays valid and classified reconciler; missing grant is reported as
missing-recommended
- tests/test_branch_cleanup_guard.py: author- and merger-shaped profiles
without gitea.branch.delete fail closed on gitea_cleanup_merged_pr_branch
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Add an env-var-gated, off-by-default Sentry SDK integration so MCP runtime
errors, fail-closed workflow blockers, lease/terminal-lock/stale-runtime
collisions, and recurring watchdog check-ins are visible in a self-hosted
Sentry at https://sentry.prgs.cc/. Gitea stays the source of truth; Sentry is
observe-only.
New module `sentry_observability.py` mirrors the `gitea_audit` conventions
(env-gated, best-effort, redacting):
- Config from MCP_SENTRY_ENABLED / SENTRY_DSN / SENTRY_ENVIRONMENT /
SENTRY_RELEASE / MCP_SENTRY_TRACES_SAMPLE_RATE / MCP_SENTRY_ENABLE_LOGS.
Active only when enabled AND a DSN is present; otherwise a hard no-op.
- Fail OPEN for observability (never blocks a tool success path) and fail
CLOSED for redaction (drop a field rather than risk leaking it).
- `scrub_event` before_send/before_send_log hook + allowlisted tags: no
tokens/passwords/keychain ids/DSNs/cookies, no raw session-state or full
prompt bodies (session_id -> 12-char hash), no full filesystem paths
(worktree path -> coarse category). Reuses incident_bridge + gitea_audit
scrubbers.
- capture_exception, capture_workflow_blocker (with canonical next action),
and monitor_checkin with six stable cron slugs (stale lease scan, terminal
lock scan, allocator health, namespace health, dashboard freshness,
reconciler cleanup).
- `sentry_sdk` is a lazily-imported optional dependency; the module imports
and no-ops cleanly when the package is absent.
Wiring in gitea_mcp_server.py (additive, guarded, best-effort):
- init_sentry() in __main__ before mcp.run.
- capture_exception in the `_audited` failure path; capture_workflow_blocker
in `_audit_pr_result` BLOCKED/FAILED path.
- allocator + namespace-health watchdog check-ins at their MCP tool sites
(domain modules left pure).
Also: pin `sentry-sdk==2.20.0` (optional), document the six env vars in
`.env.example`, and add `docs/observability/sentry-integration.md` covering
project creation in https://sentry.prgs.cc/, DSN handling, local/dev/prod
config, redaction guarantees, and coexistence with the #612 incident bridge.
Tests: tests/test_sentry_observability.py (36 cases) cover disabled / enabled /
missing-DSN / missing-SDK, redaction, exception capture, workflow-blocker
capture, and cron check-in behaviour. Full suite: 2632 passed, 6 skipped.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Prevention hardening for the #670 incident (bare direct-to-master commit
2fa97c26 and the PR #654 merger stable-branch push attempt). Worker sessions
must never publish stable branches directly; stable updates land only through
sanctioned Gitea merge tooling or an authorized reconciler path.
New pure module stable_branch_push_guard.py:
- classify_push_command: detects git push <remote> <stable-ref> equivalents
including refspecs (HEAD:<ref>, +refs/heads/x:refs/heads/<ref>), --force,
--delete/:<ref>, and --dry-run/-n no-op probes (dry-run still proves intent).
Fetch/pull and feature-branch pushes are never flagged; sanctioned
gitea_merge_pr / API merge is not a push.
- assess_root_checkout_local_commit: detects control-checkout commits not on an
issue feature branch (branches/ worktrees exempt).
- redact_command: strips URL userinfo/token assignments before logging.
- build_contamination_record + assess_contamination_gate: durable marker shape
and fail-closed gate (reconciler-exempt; comment/lock stay allowed for handoff).
Server wiring (gitea_mcp_server.py, mcp_session_state.py):
- KIND_STABLE_BRANCH_CONTAMINATION durable session marker (per profile identity).
- _enforce_stable_branch_contamination_gate wired into verify_preflight_purity
so review/merge/close/completion mutations fail closed while contaminated.
- gitea_record_stable_branch_push_attempt: classify + mark on detection.
- gitea_audit_stable_branch_contamination: reconciler-only inspect/clear; a
worker session can never self-clear.
Tests (AC5): no-op dry-run push, real direct push, sanctioned Gitea merge,
fetch-only, root-checkout local commit, feature-branch push allowed, gate
block/reconciler-exempt, redaction. 51 new tests; full suite 2596 passed.
Docs: llm-project-workflow SKILL.md — universal rule, blocker class, and a
dedicated "Stable Branch Push Protection" section (worker sessions must never
push stable branches directly).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Add orthogonal role:* (single-active owner) and hazard:* (additive warning)
lifecycle labels plus status:changes-requested, folding the #603 state:*
vocabulary into the canonical status:* set rather than a conflicting parallel
prefix.
- issue_workflow_labels.py: ROLE_/HAZARD_ specs + frozensets; role/hazard
transition maps and helpers (transition_role_labels, add/clear_hazard_label,
canonical_role_label, canonical_hazard_label, is_discussion,
is_implementation_candidate, requires_blocking_reason); state:* -> status:*
synonym transitions; assess_issue_labels surfaces role/hazard dims and flags
multiple active role labels
- docs/label-taxonomy.md: role, hazard, state->canonical migration, and
allocator cross-check sections
- tests: role/hazard/discussion/blocking-reason/state-synonym coverage
- manage_labels.py seeds the new labels automatically via CANONICAL_LABEL_SPECS
Labels remain advisory; control-plane leases (#601) and live PR state stay the
source of truth for mutations (#603 AC2).
Closes#603
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Closes#627
Merges paginated repository-label inventory for gitea_set_issue_labels so later-page labels are not falsely rejected during full-set replacement.
Replace single-page GET labels?limit=100 with api_get_all-backed
_repo_label_id_map so later-page labels (e.g. type:feature,
workflow-hardening) are not falsely rejected during full-set replacement.
Also add post-mutation verification, fix related MCP/CLI label inventory
call sites, and add multi-page regression tests for the #601 reconciler
failure mode.
Closes#627
- Update MCP expired-lock recovery test for sticky expired ownership
(live pid + present worktree still fail closed under #601 reclaim rules)
- Remove trailing blank line at EOF in control_plane_db.py
Add phase-1 observability bridge that turns provider observations into
normal Gitea issues and control-plane incident_links rows. Dry-run is
default; apply creates/links through sanctioned Gitea issue paths only.
Raw incidents remain non-assignable; the #600 allocator sees bridge work
only as ordinary Gitea issues. Builds on #613 substrate and #600 allocator.
Closes#612
jcwalker3
sysadmin
