fix/issue-787-kill-segment-separators
740
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  jcwalker3andClaude Opus 4.8
|
aa4fe1cc7b |
fix(guard): make shell segment splitting quote-aware (Closes #787)
Review #497 on PR #789 found that adding `&` to the separator alternation created a new false-positive class: the splitter was quote-unaware, so any quoted text carrying an ampersand and a kill verb classified as a manual daemon kill. Three commands regressed against base |
||
|
jcwalker3andClaude Opus 4.8
|
6b58f04d39 |
fix(guard): split on & and unwrap subshells so real daemon kills classify (Closes #787)
`runtime_recovery_guard._SEGMENT_SPLIT_RE` claimed to split a compound command
line "on shell separators", but omitted the background-job separator `&`, and no
caller stripped a subshell wrapper before tokenising. `_analyse_kill_segment`
only inspects the first command token of a segment, so in both forms the kill
verb never reached the classifier.
Measured against the previous implementation at
|
||
|
jcwalker3andClaude Opus 4.8
|
1ec4672fad |
fix(mcp): block manual daemon killing as workflow recovery (Closes #630)
A session that ran `pkill -f mcp_server.py`, waited for the IDE to respawn the daemons, and then closed an issue left no trace distinguishing that closure from one performed over a sanctioned runtime. Detection existed only as advisory strings (`native_mcp_preference.classify_command_path`, `review_workflow_boundary`) and never failed closed on the mutations that followed. Adds `runtime_recovery_guard.py`, mirroring the #671 stable-branch contamination model so the two cannot drift apart: classification of kill/pkill/killall commands and known-pid kills, a durable `runtime_recovery_contamination` marker, a fail-closed pre-flight gate over the shared review/merge/close/completion task set, and reconciler-only clearing. `comment_issue` and `lock_issue` stay allowed so a contaminated worker can still post its audit comment and hand off. The marker is recovery-critical, so contamination cannot expire into cleanliness with the session-state TTL. Read-only inspection (`ps aux | grep mcp_server`), sanctioned reconnects, and process management unrelated to the daemons are never flagged; a bare `kill` of an unknown pid is reported as ambiguous rather than contaminating, so ordinary subprocess work is not false-blocked. A pattern broad enough to sweep unrelated namespaces (`pkill -f python`) is contamination even when it never names MCP. Operator-authorized host maintenance stays permitted, but the authorization is read from GITEA_OPERATOR_DAEMON_MAINTENANCE_AUTHORIZATION in the process environment only and never from a tool argument, so a session cannot authorize itself. Final-report rules reject clean-session claims and require the contaminated recovery to be surfaced. Two tools are registered (inventory 112 to 114) and the sanctioned-versus-forbidden contrast is documented in the namespace recovery doc and the workflow skill. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
 sysadminandClaude Opus 4.8
|
0589ec8069 |
feat(control-plane): persist allocator dependency edges as durable state (Closes #784)
Umbrella #628 scope item 6 requires dependencies to be durable structured
state carrying source, target, type, blocking condition, completion
condition, current state, and evidence. Nothing stored any of that.
Dependency knowledge existed only as a per-run computation:
allocator_dependencies re-parsed the Depends: declaration out of every
issue body on every allocation, _allocator_candidates_from_gitea resolved
each reference against live issue state, and the result collapsed into two
in-memory WorkCandidate fields that classify_skip consumed and discarded.
Three consequences followed. Nothing could answer "what is waiting on #N"
without re-listing every open issue and re-parsing every body, so the
reverse edge automatic resumption needs did not exist in any form. Only
issue-blocked-by-issue was expressible, leaving the other six #628
relationships with nowhere to live. And no observation was recorded, so a
transient lookup failure and a real block were indistinguishable after the
fact.
Add the store:
- dependency_edges table under schema v4. Creating the table is itself the
v3 to v4 migration: additive, idempotent, and it never touches the
existing tables. Uniqueness is (scope, source, target, edge_type), so
re-observation updates one row rather than appending duplicates.
- dependency_graph.py owns the vocabulary: the seven #628 relationship
types, the three states, and fail-closed normalization for both plus
endpoint kinds. An unrecognized value writes nothing rather than landing
as unqueryable free text. Evidence is sanitized before storage, so no
credential or endpoint URL can be persisted or read back.
- upsert_dependency_edge, list_dependency_edges, and
record_dependency_edge_observation on ControlPlaneDB. Filtering by target
makes reverse lookup a single query. State transitions append to the
existing events table rather than a parallel audit table.
- The allocator persists what it already resolved. States map one-to-one
from the resolver's met/unmet/unavailable partitions, so nothing is
re-classified and unavailable evidence is never recorded as met.
- gitea_list_dependency_edges exposes stored edges read-only, gated on
gitea.read, and is added to the documented inventory the #781 drift guard
checks.
Selection is deliberately untouched: classify_skip still consumes the
in-memory dependency_unmet field. The write is best-effort and reports
failures through reasons, so a broken or absent store leaves allocation
behaving exactly as it did before — proven by allocating the same candidate
set through a store whose writes all raise and comparing the selection,
skip set, and candidate count.
Automatic blocking and resumption (#628 item 7), non-issue edge creation,
and defect auto-linking are later slices; this one only makes the graph
durable and queryable.
Tests: 31 new cases covering fresh-schema creation, a real v3-to-v4
migration with row retention, idempotent re-migration, enum rejection,
upsert idempotence, forward and reverse lookup, scope isolation, transition
events, redaction at rest, live allocation-run ingestion, write-failure
tolerance, and the tool's permission gate.
Verification: 4126 passed in the branch worktree. The 11 failures in
test_commit_payloads, test_issue_702_review_findings_f1_f6, test_mcp_server,
test_post_merge_moot_lease, and test_reconciler_supersession_close reproduce
identically on a clean detached checkout of master at
|
||
|
sysadminandClaude Opus 4.8
|
a002864a06 |
fix(mcp): add sanctioned gitea_edit_issue and a doc/registry drift guard (Closes #781)
The gitea-workflow documentation named a gitea_edit_issue tool that no
namespace had ever registered. There was no sanctioned MCP path to change an
issue title or body at all: the only edit tool, gitea_edit_pr, PATCHes the
pull-request endpoint. An authorized body correction on issue #780 therefore
had to be recorded as a discussion comment instead.
Add edit_issue.py as the authoritative rule and gitea_edit_issue as the tool
built on it:
- Only the fields the caller names are sent, so labels, state, assignee, and
milestone cannot be overwritten from a stale read.
- A pull-request number is refused. Gitea serves pull requests from the same
/issues/{n} collection, so without that check the issue path would quietly
become a second, ungated PR edit path. gitea_edit_pr stays PR-only.
- Structurally invalid requests raise before any credential or network work;
a request that would change nothing is reported as an explicit no-op with a
next action rather than a silent success.
- Read-after-write proves the applied title/body and proves that state,
labels, assignees, and milestone did not move. Transport failures on the
pre-read, the PATCH, and the read-back are each reported, redacted, with
the correct performed/verified state.
Gates match every other issue mutation: profile permission via the shared
capability map (resolver task edit_issue, gitea.issue.comment), preflight
purity, branches worktree validation, anti-stomp inventory membership, and
audited mutation.
Fix the drift that hid this. docs/mcp-tool-inventory.md is now the canonical
registered-tool list, and mcp_tool_inventory.py compares it to the live
registry in both directions, plus checks that every tool named under skills/
is registered. The new guard immediately found a second instance of the same
defect: gitea_record_pre_review_command had lost its @mcp.tool() decorator
while the canonical review workflow still instructed reviewers to call it, so
that registration is restored.
Validation:
PASSED: venv/bin/python -m pytest tests/test_issue_781_edit_issue_tool.py -s -q
— 50 passed
FAILED: venv/bin/python -m pytest -s -q — 4095 passed, 11 failed, 6 skipped.
The identical 11 tests fail on clean master
|
||
|
sysadminandClaude Opus 4.8
|
ed0e8c82de |
fix(workflow): retire status:pr-open on every terminal PR transition (Closes #780)
status:pr-open was applied by gitea_create_pr and never removed again. Every
terminal path finished without touching it, so a repository audit found 40
closed issues still advertising an open PR.
Add terminal_pr_label_cleanup.py as the single authoritative rule and route
every sanctioned terminal path through it, so the paths cannot drift:
- merge (gitea_merge_pr)
- close without merge (gitea_edit_pr)
- supersession/abandonment (gitea_reconcile_superseded_by_merged_pr)
- already-landed reconciliation (gitea_reconcile_already_landed_pr)
- controller closure (gitea_close_issue)
- retry/recovery (new gitea_cleanup_terminal_pr_labels)
The rule removes only status:pr-open, preserves every other label, allows an
empty resulting set, is a no-op when the label is absent (so retries are
safe), and confirms the outcome by read-after-write rather than assumption.
Controller closure runs the cleanup before the state change and fails closed
if it cannot be completed and verified; closing first would bake in the stale
label with no later step to catch it. Post-merge cleanup never blocks the
merge, which already happened, and reports failures with a safe next action.
Also:
- gitea_assess_terminal_label_hygiene: read-only terminal validation that
reports residual status:pr-open, exempting issues with a genuinely open PR.
- _put_issue_label_names now accepts Gitea's empty response body when the
requested set is empty, so clearing the last label works.
- test_audit's close_issue fixture keys on the request instead of call order,
since closing now also reads labels for the cleanup and its read-back.
Docs: label-taxonomy terminal-transition section, runbook pointer, and the
review-merge / reconcile-landed final-report terminal-label requirements.
Suite: 4045 passed, 11 failed, 6 skipped. The same 11 failures reproduce on
clean master
|
||
|
jcwalker3andClaude Opus 4.8
|
ddc9b97d40 |
feat: enforce self-propagating canonical handoffs through controller closure (Closes #626)
Adds the canonical cross-role handoff schema and its fail-closed validator, live-state recovery, role-limited continuation, mandatory durable posting, the merged-awaiting-controller boundary, controller accept/reject continuation, and workflow-failure escalation with duplicate handling. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadminandClaude Opus 4.8
|
d17f055e86 |
fix(allocator): pre-rank exclusions and candidates_json transport (#776)
Expose exclude_issue_numbers on gitea_allocate_next_work, remove excluded numbers before ranking, normalize decoded-list and JSON-string candidates_json fail-closed, and return candidate-set fingerprints for dry-run/apply CAS. Same-owner leases on excluded issues surface a structured resume/release blocker. Closes #776 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
jcwalker3andClaude Opus 4.8
|
296601647d |
fix: mutation budget counts server-side changes only (Closes #617)
Auto-mode classifier now distinguishes local validator rejection, capability-gate rejection, transport failure before API, and successful server-side mutation. Pre-API validator failures no longer consume server-side mutation budget; the final report separately accounts for local failed attempts, blocked API attempts, and successful server-side mutations. Recovered from preserved unpublished commit b46f0f9 via native MCP unpublished-claim recovery (#772) and author-worktree lock binding (#618), reconciled onto current master. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadmin
|
702ceb2480 | Merge pull request 'fix(mcp): recover clean unpublished author work after the owning session exits (Closes #772)' (#774) from fix/issue-772-unpublished-claim-recovery into master | ||
 sysadminandGrok 4.5
|
c31df2130c |
test(mcp): AC9 assessor/mutator parity + AC6 unpublished recovery MCP regressions (PR #774 F1/F2)
Local worktree commit only (publication blocked by dangling GITEA_AUTHOR_WORKTREE). Tests only; no production code change. Co-Authored-By: Grok 4.5 <[email protected]> |
||
 jcwalker3andClaude Opus 4.8 (1M context) <[email protected]>
|
ca76dacd73 |
fix(mcp): recover clean unpublished author work after the owning session exits
Closes #772 Recovery now dispatches on observed publication state: published_owning_pr (remote branch exists; head equality #753 or strict descendancy #768, unchanged) and unpublished_claim (no remote branch, no PR; ownership proven by the durable lock record plus a local HEAD strictly descending from the server-observed base the branch was cut from). The absence of a remote head is never itself permission. Recovery writes are compare-and-swap against a lock generation, and the mutating lock path and read-only diagnostic assessor share one evaluator. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
jcwalker3andClaude Opus 4.8
|
ad13d872df |
fix: exclude foreign-claimed work from allocation instead of blockading the queue (Closes #765)
Recovered preserved candidate d06198b onto current master
|
||
|
jcwalker3andClaude Opus 4.8
|
5ed2ab8a38 |
fix: durable author worktree resolution without control fallback (Closes #618)
Author mutation tools now resolve workspace via explicit worktree_path,
env bindings, or the active author issue lock — never silent fallback to
the control checkout/master. Missing configured bindings fail closed with
operator recovery; create_issue and create_issue_comment agree.
Recovered onto
|
||
|
sysadminandClaude Opus 4.8
|
ab34280f90 |
fix: correct runtime-gate alignment and per-call state for #615 (Closes #615)
Remediates reviewer findings F1, F2, and F3 from review 483 on PR #770. F1 (blocking): _current_runtime_mode_report derived the runtime-gate alignment input as realpath(workspace_root) == process_project_root, redefining workspace_roots_aligned. Its established meaning is ctx["roots_aligned"] (canonical_repo_root == process_project_root) — a repository-level question. Because the global worktree rule requires all task work to live in a branches/ worktree, the old derivation reported aligned False for exactly the sessions that are configured correctly, raising unsafe_process_root_workspace_alignment and refusing every non-read operation once an author, reviewer, or merger namespace held a worktree binding. The gate is now fed ctx["roots_aligned"]. F2 (blocking): _current_runtime_mode_report cached its first result into _STARTUP_RUNTIME_MODE, which was initialised to None rather than captured at import, and the read-only refresh=True path seeded it too. Session-scoped fields (active_task_workspace and its derived alignment) and mutable fields (dirty_files) were frozen for the process lifetime, so the acceptance criterion 7 dirty-runtime blocker stopped applying after the snapshot and one session's binding decided alignment for every later session. Immutable process facts (process root, branch, head, checkout-ness) are now captured at import as _STARTUP_RUNTIME_FACTS, matching the #420 parity baseline. Dirty state, workspace binding, and alignment are recomputed on every call. A new stable_control_runtime.observe_dirty_files() splits out the one runtime fact that legitimately changes during a process lifetime; observe_runtime() delegates to it so the parsing lives in one place. refresh is retained for the read-only reporting path but no longer selects a cache, so a read-only call can neither seed nor weaken a later mutation decision. F3 (major): no test exercised the real derivation — every server-wiring test asserting a healthy runtime permits mutations patched _current_runtime_mode_report with a fixture whose workspace_roots_aligned was True. New TestServerWiringRealDerivation patches only the derivation's inputs and lets the real function build the report: - a clean stable control checkout plus a correctly bound branches/ worktree permits an otherwise authorized mutation; - misaligned process/canonical roots fail closed; - newly dirty task state is detected after an earlier clean read; - a read-only refresh cannot freeze a permissive mutation result; - an unresolvable binding reports unknown alignment, never alignment proof. Acceptance criteria 6, 8, 10, and 11 are unchanged and untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadminandClaude Opus 4.8
|
9bf3acfef6 |
feat: enforce stable-control vs dev runtime modes for Gitea MCP (Closes #615)
The stable-control runtime ADR (docs/architecture/mcp-stable-control-runtime-policy-adr.md)
established the policy but had no runtime enforcement: a daemon relaunched from a
feature worktree still holds production credentials and will happily mutate real
issues. This adds the enforcement layer (acceptance criteria 6-11).
stable_control_runtime.py:
- classify_runtime_mode(): stable-control | dev-test | unknown, inferred from the
process root and checkout branch, with an explicit GITEA_MCP_RUNTIME_MODE
declaration for packaged layouts that have no git checkout.
- build_runtime_report(): runtime mode, git SHA, branch, checkout path, process
root, active workspace, repo binding, profile, identity, dirty files,
alignment, and real_mutations_allowed.
- assess_runtime_mutation_gate(): fails closed on dev-test targeting production,
unknown runtime, dirty stable checkout, dev-worktree launch, and unsafe
process-root/workspace alignment.
- Post-transport-flap re-proving tracked per namespace, so proving the author
namespace never implies reviewer, merger, or reconciler (#584).
- assess_promotion_record(): promotion must record previous and promoted SHAs
plus health, identity, profile, workspace, capability, and rollback proof.
Server wiring:
- _profile_operation_gate() consults the runtime gate alongside the #420 parity
gate. gitea.read is never blocked, so an operator can still diagnose a sick
runtime.
- The gate reads a startup snapshot rather than shelling out per mutation, for
the same reason parity uses a startup baseline: the runtime a process serves
from is fixed when it loads its code. Enforcement is decided from how the
process was loaded, so per-test production simulation cannot switch it on.
- gitea_get_runtime_context() reports the live runtime under
stable_control_runtime and points at the promotion runbook when blocked.
Docs and tooling:
- docs/stable-runtime-promotion-runbook.md: operator promotion procedure,
required record fields, per-namespace re-proving, rollback.
- scripts/promote-stable-runtime: read-only helper that emits and validates a
promotion record; it never restarts anything.
- Five canonical [THREAD STATE LEDGER] examples: runtime healthy, transport flap
recovered, namespace not yet re-proven, promotion completed, rollback required.
- ADR section 5 follow-ups marked landed; runbooks cross-link the new runbook.
Validation: 47 new tests in tests/test_stable_control_runtime.py. Full suite
3827 passed / 6 skipped / 2 failed; both failures are pre-existing on master
|
||
|
jcwalker3
|
bc968dd2e0 |
fix: post AC4 recurrence comments on linked incident issues (#607)
Remediates review 479/481 findings F1 and F2 on PR #767. F1: issue #607 AC4 requires that an existing linked Gitea issue receives a recurrence comment when Sentry events continue. That path did not exist. This adds: - incident_bridge.incident_recurred() — true only when event_count or last_seen genuinely advanced, compared against the pre-upsert link row, so an unchanged rescan stays silent and the exactly-once property holds. - incident_bridge.build_recurrence_comment_body() — reuses the same redact_text path as build_gitea_issue_body, so redaction is not re-implemented. - A comment_issue_fn hook on reconcile_incident, invoked only on OUTCOME_UPDATED with genuinely new events. The durable incident_links row is written first, so a comment failure degrades to "link updated, comment withheld" without corrupting the mapping or creating a duplicate issue. - _incident_recurrence_comment_fn() in gitea_mcp_server.py, routing through the sanctioned gitea_create_issue_comment path named in issue #607. It returns None on dry runs and when the profile lacks gitea.issue.comment, so the AC8 dry-run default and disabled-mode safety hold. - comment_issue_fn threading through sentry_incident_bridge.watchdog(), with the per-issue recurrence_comment outcome recorded in the scan result. F2: observation_from_issue never populates a fingerprint, so the "deduped by Sentry issue id and fingerprint" claim was unsupported. The gitea_sentry_reconcile_issue and gitea_sentry_watchdog docstrings now state the real dedupe basis: provider identity (provider, base URL, org, project, Sentry issue id). Tests: five new AC4 cases in tests/test_sentry_incident_bridge.py covering a comment on the second scan, dry-run silence, no-new-events silence, link durability when the comment fails, and comment redaction. Focused suite 43 passed (was 38). Refs #607 |
||
|
jcwalker3
|
716fc21a0d | Merge branch 'master' into feat/issue-607-sentry-incident-bridge | ||
|
jcwalker3andClaude Opus 4.8
|
5547399037 |
fix(mcp): accept strict-descendant dead-session recovery (Closes #768)
Permit fail-closed recovery when a clean local head is a strict descendant of the recorded PR/remote head, with server-side ancestry proof. Propagate recovery evidence through commit, push, and PR duplicate gates so an owning PR is not re-blocked as competing work. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadmin
|
cb6ae0ca50 | Merge branch 'master' into feat/issue-607-sentry-incident-bridge | ||
|
sysadmin
|
4b8a9219d8 | fix(mcp): enforce role capability invariants (Closes #723) | ||
|
sysadminandClaude Opus 4.8
|
e168978579 |
feat: add Sentry-to-Gitea incident bridge for MCP workflow failures (Closes #607)
Adds the read half of the inbound observability path: pull unresolved issues
and events from the self-hosted Sentry API, normalize them into #612
observations, and reconcile them into durable Gitea issues.
Design: the existing #612 incident_bridge already owns dedupe, linking,
redaction, and issue creation on the #613 incident_links substrate, so this
change adds only what was genuinely missing - a Sentry API read layer,
observation mapping, a policy gate, and a watchdog. No second linking store is
introduced, which is what makes the mapping survive restarts (AC6).
New module sentry_incident_bridge.py:
* list_issues() with Link-header cursor pagination and statsPeriod windowing
* get_issue_events() returning sanitized recent + latest event
* observation_from_issue() mapping onto the #612 observation contract
* should_bridge_issue() policy gate (unresolved + event-count threshold)
* watchdog() scanning and reconciling, dry-run by default
* HTTP access injected as http_fn so the surface is testable without Sentry
New MCP tools: gitea_sentry_list_issues, gitea_sentry_get_issue_events,
gitea_sentry_reconcile_issue, gitea_sentry_link_gitea_issue,
gitea_sentry_watchdog.
Safety:
* SENTRY_AUTH_TOKEN is read from the environment only and never returned,
logged, or stored; config projections cannot carry it by construction
* missing config fails closed as not_configured, and a missing token fails
closed as missing_token before any HTTP call is made
* apply=true requires both MCP_SENTRY_ISSUE_BRIDGE_ENABLED and issue-create
permission; Sentry outages create nothing
* secrets are redacted and absolute local paths reduced to a category token
before any value can reach a Gitea issue body
* raw Sentry incidents are never assignable control-plane work items
Tests: 38 new cases covering create, update/recurrence, dedupe, resolved-issue
non-reopen, redaction, pagination, missing token, unavailable server,
self-hosted base URL, restart persistence, policy gates, and the five tool
wrappers.
Full suite: 3733 passed, 6 skipped. The 2 remaining failures
(test_issue_702_review_findings_f1_f6, test_reconciler_supersession_close) were
verified to fail identically on unmodified master at
|
||
|
jcwalker3
|
d181d499d3 | Merge branch 'master' into feat/issue-605-workflow-dashboard | ||
|
sysadmin
|
6b568d8805 | fix: accept reviewer lease preflight transition (Closes #763) | ||
|
sysadmin
|
7f2b9f36de |
feat: add workflow dashboard for queue, leases, and next safe action (Closes #605)
Read-only MCP tool gitea_workflow_dashboard plus mcp-menu entry so operators and LLMs can see PR/issue queues, leases, terminal locks, blockers, and exact next-safe prompts without reconstructing state from comments. Never assigns work or presents blocked/terminal-locked items as safe. |
||
|
sysadmin
|
ad59053cd7 |
fix: rank complete allocator inventory and resolve declared dependencies (Closes #758)
The author allocator could select an ineligible issue for two independent reasons, both fixed here. Defect 1 — candidate truncation before ranking. The loader fetched the complete open inventory via api_get_all, then sliced it to `limit` items before constructing any WorkCandidate. Because every status:ready issue ties at priority 20 and the tie breaks on lowest number, the slice — not the ranking — decided the winner, so the same query returned different answers at different `limit` values. Candidate construction now consumes the full listing; `limit` bounds the reported skip list only, and any such truncation is reported explicitly rather than silently. Defect 2 — dependency state inferred from body substrings. Only "blocked on #" and "downstream of #" were recognized, so the repository's canonical `Depends: #N, #N` field never set dependency_unmet and dependency-blocked issues were emitted as eligible. The new allocator_dependencies module parses the canonical declaration into structured references and resolves each against live issue state: open means unmet, closed means met, and unavailable evidence fails closed. The complete open listing proves openness without extra calls; anything absent from it is confirmed by a cached targeted lookup instead of being assumed closed. The legacy "blocked on #" marker still parses. A failed listing now marks the inventory incomplete and the tool fails closed instead of ranking a partial set. Selection already advanced past a skipped candidate; with dependencies resolved correctly that fall-through now actually engages, and is covered by a regression. Tests: 35 new cases across parser, resolver, loader, and an MCP-level gitea_allocate_next_work regression, including limit-invariant selection over a 73-candidate inventory and a guard proving pre-ranking truncation would change the winner. No issue number is special-cased. |
||
|
sysadminandClaude Opus 4.8
|
9d2c652ae8 |
fix(mcp): require proven base equivalence for the create-issue bootstrap
Remediates review #473 (REQUEST_CHANGES) on PR #759 / issue #757 AC3/AC4. The bootstrap compared SHAs only inside: if remote_tip and local_tip and remote_tip != local_tip: so agreement was assumed whenever either tip was unknown. A missing local HEAD, an unresolvable live master, or a resolver exception silently granted the control-checkout exemption instead of blocking it. An unknown tip is missing evidence, not proof of equivalence. Changes: * assess_create_issue_bootstrap now blocks unless BOTH tips are known and equal, with distinct reasons for missing local HEAD, unknown live master, and resolver failure. Adds a remote_master_sha_error parameter so a failed resolution is reported as missing evidence rather than absence of a constraint. * Both normalized SHAs and a derived base_tips_verified flag are recorded on the assessment. normalize_sha() treats only case and surrounding whitespace as equivalent spellings of a commit. * bootstrap_permits_control_checkout re-derives the comparison from the recorded tips instead of trusting base_tips_verified, so a hand-built or truncated assessment cannot assert agreement it never proved. * _create_issue_bootstrap_assessment captures the resolver exception and forwards it, replacing the silent except -> None. One shared assessment still serves both the #274 and #604 guards, and _BOOTSTRAP_UNSET is preserved. No MCP tool signature gains a bootstrap argument (AC6). No issue or PR number is special-cased (AC10). Tests: 16 new assertions across two classes covering missing local, missing remote, both missing, empty/whitespace tips, mismatch, resolver exception at the assessment site, and resolver exception through the real verify_preflight_purity path; plus predicate rejection of stripped tips, a forged base_tips_verified flag, and a missing flag. All 16 fail against the sources at |
||
|
sysadminandClaude Opus 4.8
|
adc61255b2 |
fix(mcp): honor the create-issue bootstrap in anti-stomp preflight (Closes #757)
The sanctioned create_issue bootstrap from #749/#750 was unreachable in
production. Two guards assessed the same workspace for the same task and
reached opposite conclusions: the #274 branches-only guard consulted the
bootstrap and permitted a clean canonical control checkout, then the #604
anti-stomp preflight -- which never consulted it -- rejected that same
checkout as wrong_worktree.
The defect was wiring, not policy: the bootstrap decision was computed in
one guard and discarded, while the other re-derived a conflicting answer
from a lower-level assessor with no notion of the bootstrap phase.
Fix: one computation site, one interpretation site.
* create_issue_bootstrap.bootstrap_permits_control_checkout() is the single
predicate both guards use to interpret an assessment. It is fail-closed by
construction: missing, malformed, refused, incomplete, or contradictory
evidence returns False and leaves the ordinary block in force. It also
verifies the assessment describes the exact workspace and canonical root
being guarded, so a stale or foreign assessment cannot be reused.
* _create_issue_bootstrap_assessment() computes the assessment once per
preflight from inspected repository state. verify_preflight_purity threads
that single result into both guards.
* The #604 assessor accepts the assessment and waives ONLY the wrong-worktree
verdict. Root checkout, repo, role, stale runtime, lease, head-SHA,
workflow-hash, and contamination checks are evaluated independently and
still apply.
Evidence is server-derived only and travels an internal path: no MCP tool
signature gains a bootstrap argument, and no caller-controlled boolean can
manufacture eligibility. Behavior is unchanged for callers that supply no
evidence, and for every non-create_issue author mutation.
No issue or PR number is special-cased in production behavior.
Tests: new tests/test_issue_757_bootstrap_guard_agreement.py (38 tests, 26
subtests) covering the shared predicate, the narrow waiver, guard agreement
across the full workspace-state matrix, non-forgeable eligibility, and an
end-to-end native gitea_create_issue run with the #604 gate LIVE. All 38
fail against unfixed sources; the e2e reproduces the production error text
verbatim ("Anti-stomp preflight (#604) blocked mutation [wrong_worktree]").
tests/test_reconciler_close_workspace_guard.py: one case asserted that
create_issue stays blocked on the control checkout, which only held because
the bootstrap-blind #604 guard was overriding #750 -- it encoded the defect.
Re-pointed to lock_issue, which is issue-backed and legitimately still
requires a branches/ worktree. Its teardown now restores the module-level
preflight task/role so test order cannot leak resolved state.
Full suite: 3624 passed, 2 failed, 6 skipped (426 subtests).
Baseline at
|
||
|
jcwalker3
|
a517655aad | Merge branch 'master' into fix/issue-745-reconciler-moot-lease-gate | ||
|
jcwalker3
|
5e0e474350 | Merge branch 'master' into fix/issue-749-create-issue-bootstrap | ||
|
sysadminandClaude Opus 4.8
|
f858c1d1b2 |
fix(mcp): let a sanctioned recovery keep its own owning PR (Closes #755)
#753 added the dead-session author-lock recovery assessor, but no sanctioned
recovery could ever reach the lock write. A dead-session lock is by construction
a lock for work that already has an open PR, and the #400 duplicate-work gate
blocked unconditionally on any linked open PR. gitea_lock_issue computed
recovery_sanctioned, then discarded it one gate later:
open PR #750 already covers issue #749 (fail closed)
Because the recovered lock was never persisted, it stayed non-live, so
_prove_author_ownership_for_pr found no live author lock and
gitea_update_pr_branch_by_merge failed closed too. Both blockers had a single
cause, so only the first one is fixed here.
issue_lock_recovery.owning_pr_recovery_evidence distils a granted assessment
into the minimum evidence the duplicate gate needs: issue, branch, owning PR
number, and head. It returns None unless the outcome is RECOVERY_SANCTIONED and
the assessment's own evidence agrees that local head == remote head == PR head,
so a partial or hand-built assessment cannot authorize anything.
The duplicate gate takes that evidence and re-checks every element against the
live PR list it was handed: exactly one linked open PR, matching number, head
branch, head SHA, and locked branch. Only that exact self-owned PR is exempt.
A different PR, several linked PRs, a different branch or head, or evidence for
another issue all keep failing closed, with a diagnostic naming which element
disagreed. The competing-branch, claim-lease and commit/push/create_pr arms are
untouched, and with no evidence the gate behaves exactly as before.
Ownership proof needed no change: once the recovered lock persists under the
live session pid, _prove_author_ownership_for_pr matches it as before.
Out of scope: the PHASE_COMMIT/PHASE_PUSH/PHASE_CREATE_PR rechecks still block
on a linked open PR for every author, recovered or not. That is pre-existing
#400 behavior, unrelated to lock recovery, and #755 does not cover it.
Tests
- tests/test_issue_755_owning_pr_recovery.py (new, 31 tests) drives the real
mcp_server.gitea_lock_issue handler, not only the pure assessor: sanctioned
recovery relocks, persists a live lease with truthful dead_session_recovery
provenance, and satisfies _prove_author_ownership_for_pr; competing PR,
multiple linked PRs, mismatched head/branch/worktree, dirty worktree, live
prior pid, and a fresh claim with no prior lock all stay blocked.
- Neutralizing owning_pr_recovery_evidence regresses all three success tests to
the exact production error above, so the coverage is load-bearing.
- Focused suites (755, 753, duplicate gates, lock registration, provenance) —
102 passed.
- Lock/ownership/worktree/handoff suites — 172 passed, 16 subtests.
- Full suite tests/ — 3529 passed, 6 skipped, 2 failed, 365 subtests.
- The same 2 failures reproduce on a pristine detached worktree at
|
||
|
sysadminandClaude Opus 4.8
|
3edeba4d7f |
fix(mcp): allow author-lock recovery after the owning session exits (Closes #753)
A durable author issue lock records the PID of the MCP session that took it.
When that process exits, assess_lock_freshness marks the lock stale (live=False)
even while its lease is still within TTL, so every ownership check that needs a
live lock fails closed -- including gitea_update_pr_branch_by_merge.
Re-taking the lock was unreachable for real work. assess_issue_lock_worktree
requires the worktree to be base-equivalent to master/main/dev, and a branch
that already carries commits is ahead of its base by construction. The existing
assess_expired_lock_reclaim affordance does not apply either: it is only
consulted once the lease has expired, so a dead PID under an unexpired lease
never reaches it. assess_own_branch_adoption already speaks of "lock recovery",
but it runs after the base-equivalence gate and so was never reached.
This adds issue_lock_recovery, a pure assessor that grants a narrow waiver only
when every element of durable ownership still matches exactly and the recorded
process is demonstrably dead: same remote/org/repo/issue, same branch (and the
worktree actually on it), same registered worktree, clean worktree, local head
== remote head == open PR head, same claimant identity/profile, no competing
live lock or lease, and no ambiguous branch claims. A malformed or incomplete
lock record can never prove ownership.
The waiver suppresses base-equivalence and nothing else. Cleanliness and every
other precondition still apply, and brand-new issue claims keep the full
requirement. A refused assessment never raises: it withholds the waiver and
lets the pre-existing guard fail closed exactly as before, so recovery can only
ever add permission, never remove a guard. Refusal reasons are appended to the
block message so a caller sees the exact missing evidence.
A completed recovery is recorded on the lock as dead_session_recovery with the
prior and replacement session PIDs, heads, and claimant, so the takeover is
auditable and never looks like an original claim. Rebinding sets the live
session PID, so the recovered lock satisfies verify_lock_for_mutation and the
downstream PR update paths.
Validation:
* new tests/test_issue_753_dead_pid_lock_recovery.py -- 33 passed
* issue-lock, adoption, store, provenance, registration, duplicate-gate,
worktree, create-issue-guard suites -- 120 passed
* MCP server, commit payloads, handoff ledger, PR ownership, branch cleanup
suites -- 286 passed
* full suite -- 3498 passed, 6 skipped, 2 failed
* the same 2 failures reproduce identically on pristine master
|
||
|
jcwalker3
|
03c64a3219 | Merge branch 'master' into fix/issue-745-reconciler-moot-lease-gate | ||
|
sysadminandClaude Opus 4.8
|
eac7afe5cb |
fix(mcp): derive checks requirement from live branch protection (Closes #751)
`gitea_assess_pr_sync_status` could permanently block a merge-ready PR whose head commit had no status contexts. Gitea's combined commit-status endpoint reports `state: pending` both when a check is executing and when the status-context collection is empty. The wrapper took that `state` verbatim, and `checks_required` defaulted to True with no production path ever setting it, so "no CI configured" was indistinguishable from "CI is running" and waiting could never resolve it. Root cause spans the whole dataflow, not one call site: * the wrapper read only the combined `state` and never counted contexts; its `checks_status="none"` fallback was unreachable for any truthy state * `pr_sync_status.assess_pr_sync_status` defaulted `checks_required=True` * the MCP wrapper exposed no derived `checks_required` and never passed one * the branch-protection reader fetched the payload carrying `enable_status_check` / `status_check_contexts` and discarded both Changes: * `pr_sync_status.py`: add `classify_commit_checks` plus explicit CHECKS_* classifications (success / failure / pending / none / not_required / missing_required / unknown). The combined state is recorded for observability but is never evidence that CI is executing. Aggregation is fail-closed and newest-wins per context. * `pr_sync_status.py`: rewrite the merge_now checks gate to consume the derived `checks_required`, distinguish the new classifications with precise blocker reasons, and fail closed on unrecognized vocabulary. The previous gate let any value outside its fixed vocabulary fall through to merge_now. * `gitea_mcp_server.py`: add `_branch_protection_policy` deriving both the current-base rule and the status-check requirement from one live read; `_branch_protection_requires_current_base` is retained as a thin accessor with unchanged semantics. Add `_commit_checks_snapshot` which returns the context collection alongside the combined state. * `gitea_mcp_server.py`: wire the derived `checks_required` into the production assessment path and report `checks_evidence`. `checks_required` is always derived from live evidence and is deliberately not a caller-supplied input, so no session can declare checks optional without proof. Live classification also outranks a caller-supplied `checks_status`, which can no longer mask a real required-check failure. An unreadable protection policy or status collection stays fail-closed. Approval, current-head, current-base, mergeability, conflict, role, lease and merge-authorization gates are unchanged. Tests: `tests/test_issue_751_checks_assessor.py` (40 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadmin
|
e349839fd7 |
fix(mcp): allow create_issue from clean control checkout (#749)
Provide a narrow phase-scoped bootstrap so gitea_create_issue can run from a clean canonical control checkout when no issue number—and therefore no issue-backed worktree—can exist yet. Dirty roots, non-base branches, base races, foreign workspaces, and all post-creation author mutations remain fail-closed under the ordinary branches-only guard. Closes #749. |
||
|
sysadminandClaude Opus 4.8
|
277ec5269d |
feat(mcp): use a 10-minute sliding TTL for reviewer and merger PR leases (Closes #747)
Reviewer and merger PR leases minted a fixed 120-minute expiry (#407 AC6/AC7)
and derived staleness from two further activity bands: stale at 30 minutes,
reclaimable at 60. A session that died — daemon crash, transport flap, client
restart — therefore kept a PR blocked for an hour before anyone could reclaim
it, and two hours before manual cleanup was sanctioned. #718 records the
resulting deadlock: a merge stalled behind a reviewer lease that had stopped
being live long before it stopped being authoritative.
The flaw was using a long fixed expiry as a proxy for "the owner is probably
still alive" instead of making the owner continuously prove liveness.
Sliding window
--------------
`LEASE_TTL_MINUTES = 10` now governs acquisition, and every write of the lease
marker re-derives `expires_at` from the moment of the write, so each heartbeat
slides the window forward. An actively heartbeating session is never evicted
and has no maximum lifetime; a dead one releases its hold within one TTL.
`LEASE_RENEWAL_MINUTES` is named separately from the acquisition TTL. Renewal
previously had no seam at all: the heartbeat slid the expiry only as a side
effect of re-defaulting the acquisition constant, so the two durations could
not be reasoned about or tuned independently. `format_lease_body` now takes an
explicit `ttl_minutes`, and the heartbeat passes the renewal window rather than
relying on that default.
Merger leases acquire through the same lease-body formatter, so they inherit
the identical window by construction rather than by a parallel constant.
Removal of the reclaim tier
---------------------------
`classify_lease_freshness` no longer returns `reclaimable`. Beyond being an
extra waiting tier, that band is unreachable under a sliding TTL: a heartbeat
stamps `last_activity` and `expires_at` together, so a lease idle for a full
TTL is necessarily already expired. Expiry is now the only takeover gate.
No reclaim path is lost. `find_active_reviewer_lease` already ignores expired
markers, so an expired foreign lease never gated acquisition; and the
`foreign_expired` classification carries the same
`NEXT_ACTION_RELEASE_EXPIRED_LEASE` the retired `foreign_reclaimable` did. The
two updated tests in `test_reviewer_pr_lease.py` assert exactly that: the
classification label changes, the sanctioned next action does not.
`STALE_WARNING_MINUTES` drops to 5 — half the window — so the warning still
fires while the owner can heartbeat and recover.
Diagnostics
-----------
Adds `lease_seconds_remaining`, and the heartbeat tool now returns
`ttl_minutes`, `expires_at`, and `seconds_remaining`, so an operator can
distinguish "held and live" from "held and dying" instead of only seeing that
a lease exists. All additions are additive; no existing key changed.
Also removes `pr_work_lease.DEFAULT_REVIEWER_LEASE_TTL_MINUTES`, a duplicate of
the reviewer TTL with no readers anywhere in the tree, which could only drift.
Legacy markers minted under the old 120-minute TTL still parse and are judged
against their own recorded `expires_at`, so no lease is retroactively expired
by this change.
Full suite: 3425 passed, 6 skipped, 2 failed. Both failures
(`test_issue_702_review_findings_f1_f6`, `test_reconciler_supersession_close`)
reproduce identically on clean master
|
||
|
sysadminandClaude Opus 4.8
|
b00e09a781 |
fix(mcp): bind post-merge moot-lease cleanup to the reconciler capability (Closes #745)
gitea_cleanup_post_merge_moot_lease (#515) posts a terminal `phase: released` lease marker — a durable mutation of the PR lease ledger — but had no entry in the canonical task-capability map and no role binding. Entry gated on gitea.read, apply gated only on gitea.pr.comment, so any profile holding the comment permission (author, reviewer, merger) reached the mutation path, while the reconciler could not satisfy the operator-required resolve-exact-task -> mutation sequence because no cleanup task was resolvable at all. The apply path also called verify_preflight_purity(remote) with no task/org/repo, skipping the resolved-task, canonical-root and explicit-target checks its siblings perform, and passed request org/repo straight into _resolve. Capability map and router: - Map `cleanup_post_merge_moot_lease` and the tool-name alias `gitea_cleanup_post_merge_moot_lease` to gitea.pr.comment + reconciler. Both names carry an identical contract; unknown names keep failing closed on the map's KeyError. - Add both to role_session_router RECONCILER_TASKS and TASK_REQUIRED_ROLE so the map and the router cannot disagree (the #723 defect-A class). Tool enforcement (apply path only): - Require the session to have resolved exactly the cleanup task; resolving a different task, including a sibling reconciler task, does not authorize it. - Require the reconciler role, checked independently of the permission gate. - Require gitea.pr.comment. - Validate explicit org/repo against the canonical repository identity derived from the session binding, so request parameters can never redirect the mutation, and forward worktree_path/task/org/repo to verify_preflight_purity for canonical-root, workspace and anti-stomp binding (#733/#739). - Require matching dry-run evidence proving lease_moot and cleanup_allowed for the same PR, lease session, candidate head and lease marker id, with optional caller expectations checked against the live lease. New post_merge_moot_lease_gate module holds the pure authorization logic and an append-only dry-run ledger: entries are only ever appended, lookup is newest-wins, and dry_run_history hands out copies. Live, non-moot, superseded, mismatched, malformed and foreign-repository leases all fail closed; already-terminal cleanup stays idempotent. The read-only apply=false assessment deliberately stays reachable under gitea.read with no role gate, matching gitea_cleanup_stale_review_decision_lock and gitea_cleanup_obsolete_reviewer_comment_lease, so an operator can diagnose a stuck lease from any attached namespace. This choice is documented in the map, the tool docstring and docs/gitea-execution-profiles.md, and is directly tested. _delete_branch_repository_binding_block is generalized into _repository_binding_block(required_permission=...) and retained as a thin delete-path alias so #733/#739 coverage keeps exercising its permission label. Tests: new tests/test_issue_745_moot_lease_reconciler_gate.py (39 tests, 35 subtests) covers the map/router contract, alias parity, unknown-name rejection, role and task gates, dry-run/apply sequencing, superseded and malformed leases, repository binding, ledger append-only behavior, idempotency, and asserts no production PR/session/marker is referenced. tests/test_post_merge_moot_lease.py is updated from the permission-only model to reconciler + dry-run evidence, with a new negative test pinning that a merger can no longer apply. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadminandClaude Opus 4.8
|
61c3a57df5 |
fix(mcp): consume canonical target root in cross-repository operations (Closes #741)
#706 introduced the immutable `canonical_repository_root` and #739/#740 routed
three consumption paths through it. The paths #740 left behind all shared one
shape: the *filesystem* guards had been migrated to the canonical root, but
*repository identity* still bottomed out in `_local_git_remote_url`, which ran
`git remote get-url` with `cwd=PROJECT_ROOT` — always the Gitea-Tools install
checkout. The two halves of a single assessment therefore described two
different repositories.
For a namespace bound to another repository this inverts the guards rather than
merely weakening them: an operation naming the genuinely bound target is
rejected, while one naming Gitea-Tools is accepted.
Central helper
--------------
Adds `_canonical_local_git_root()` as the single place a target-repository root
is resolved: the configured canonical root when one is declared, else
`PROJECT_ROOT`. A configured-but-invalid root is never silently replaced by the
install checkout. Single-repository behaviour is byte-for-byte unchanged.
Defects fixed
-------------
* `_local_git_remote_url` now runs in the canonical target root, which corrects
every downstream identity consumer at once (`_resolve`, the #530 remote/repo
guard, the anti-stomp org/repo fill, `_workspace_repository_slug`).
* `_verify_role_mutation_workspace` omitted `configured_canonical_root`, so the
#274 branches-only / worktree-membership guards validated
`Gitea-Tools/branches/` instead of the bound target. It now threads the root
exactly as `_resolve_namespace_mutation_context` does.
* `_resolve` derived omitted coordinates by looking a remote up *by name*. A
target checkout commonly names its remote `origin` rather than `prgs`, so the
lookup returned None and the coordinates fell through to the remote-wide
default target — an unrelated repository. It now prefers
`_canonical_repository_slug`, which probes candidate remote names.
* Explicit `org`/`repo` short-circuit the #530 match check, so caller
coordinates bypassed validation entirely. They may now only *confirm* a
canonical binding, never override it, and fail closed in both directions.
* Reconciler ancestry, fetch, worktree-inventory and cleanup call sites, the
author worktree derivation in `gitea_lock_issue`/`gitea_create_pr`, and the
`control_clean` porcelain probe now use the canonical target root.
* `gitea_config`: v2-`environments` silently *dropped* `canonical_repository_root`
during flattening, so such a namespace fell back to the install root — a
fail-open. It is now validated and propagated. The v1 path validates it too,
so all three loaders behave identically.
Preserved as installation-scoped
--------------------------------
Server parity (`master_parity_gate`), workflow/schema/skill loading, self-code
hashing and stale-runtime detection, and `mirror_refs.sh` lookup remain anchored
to `PROJECT_ROOT`. The `server_implementation` vs `target_repository` parity
dimensions from #740 stay separately labelled, and only the server dimension
gates mutations.
Tests
-----
`tests/test_issue_741_canonical_root_consumers.py` (51 tests, 52 subtests) drives
a role-by-operation matrix over author/reviewer/merger/reconciler against:
correct cross-repository target, wrong repository, wrong worktree, explicit
matching and mismatched coordinates, missing/invalid canonical root, immutable
first-bind, and request-override attempts — plus both cross-repository
directions and all three config loaders. Real git repositories, no network, no
branch deletion, no merge.
The module reinstalls the genuine `_local_git_remote_url` per test: an autouse
conftest fixture permanently reassigns it to a working-directory-independent
stub, which is precisely the behaviour under test.
Full suite: 3408 passed, 6 skipped, 2 failed. Both failures
(`test_issue_702_review_findings_f1_f6`, `test_reconciler_supersession_close`)
reproduce identically on clean master
|
||
|
sysadmin
|
c908ed6050 | Merge pull request 'fix(mcp): complete canonical-root consumption for cross-repository namespaces (Closes #739)' (#740) from feat/issue-739-canonical-root-consumption into master | ||
|
sysadminandClaude Opus 4.8
|
fde95b9266 |
fix(mcp): chain-scoped newest-wins reviewer lease reads in pr_work_lease (#742)
Second author remediation on PR #743. Fixes the full-ledger defect surfaced by the first remediation. Root cause: pr_work_lease.find_active_reviewer_lease iterated the reviewer markers newest-first and used `continue` on a terminal marker. On a realistic append-only ledger (claimed -> validating -> terminal) it therefore stepped over the terminal marker and returned the older claim of the very chain that marker had just ended. reviewer_pr_lease got strict newest-wins under #577; pr_work_lease never did, so the two modules disagreed for released, blocked, done, and abandoned alike, and merger owner finalization did not leave "no active lease" for pr_work_lease consumers (conflict-fix acquire, PR sync inventory). Fix: a claim is skipped when a later marker terminates its own chain. Chain identity is (repo, pr_number, candidate_head, session_id, reviewer_identity, profile) via the new _reviewer_chain_key; _chain_terminated_after scans only markers appended after the candidate. Consequences: - a valid terminal marker ends its matching earlier claim (no resurrection); - a foreign-session, wrong-repo, wrong-PR, wrong-head, wrong-identity, or wrong-profile terminal marker cannot cancel another session's active lease, which strict newest-wins alone would have allowed; - a malformed terminal marker has no provable chain key, so it cancels nothing and cannot hide a valid active claim; - expiry, freshness, phase sets, ownership and integrity checks, both parsers, and find_active_conflict_fix_lease are untouched; - history stays append-only; no marker is deleted or rewritten. Reviewer lease markers carry no token field, so token-fingerprint validation remains where it already lives (session provenance, merger finalization) and is not weakened here. Tests: new TestFullLedgerNewestWins in tests/test_merger_lease_finalization.py covers claimed->released/blocked/done/abandoned, claimed->validating->terminal, foreign-session and mismatched repo/PR/head/identity/profile terminals, the malformed terminal marker, a newer active chain surviving an older terminated chain, newest-valid-chain selection across multiple histories, all-chains- terminated, single-marker parity between the two modules across eight phases, expired-claim/freshness non-regression, and the real ledger written by gitea_release_merger_pr_lease reading as terminal in both modules with the prior marker preserved. TestCrossModuleTerminalPhaseAgreement's scope-limited placeholder test is replaced by a real both-modules full-ledger assertion. Verified 10 of the new tests fail at the prior head |
||
|
sysadminandClaude Opus 4.8
|
7ae5f3a541 |
fix(mcp): mirror abandoned terminal phase into pr_work_lease (#742 review 460)
Addresses REQUEST_CHANGES review 460 on PR #743 @ |
||
|
sysadminandClaude Opus 4.8
|
22d0fdd251 |
fix(mcp): accept acquired merger-lease provenance and add owner finalization (Closes #742)
Root cause (confirmed on PR #740, session 33780-7168cbeeba58, marker 12354): merger_lease_adoption.SANCTIONED_PROVENANCE_SOURCES already contained SOURCE_ACQUIRE_MERGER, so the membership check passed, but is_sanctioned_session_lease() branched only for SOURCE_ADOPT and for {SOURCE_ACQUIRE, SOURCE_HEARTBEAT} and fell through to `return False` for a lease minted by gitea_acquire_merger_pr_lease. assess_mutation_lease_gate() therefore appended "in-session lease lacks sanctioned provenance" and gitea_merge_pr fail-closed on the merger's own freshly acquired lease. describe_session_lease_proof() likewise had no branch for that source and reported the lease as lease_proof_kind=unsanctioned. Separately, no merger-role owner-session operation existed to end that lease, so a failed merger lease could only expire. A. Acquired merger provenance is_sanctioned_session_lease() now accepts SOURCE_ACQUIRE_MERGER, but only via the new assess_acquired_merger_lease_integrity(), which fails closed unless the in-session record is complete and self-consistent: comment marker present and matching between provenance and session, non-empty session id, holder identity, merger profile, repository, valid PR number, and a normalized 40-hex candidate_head. describe_session_lease_proof() reports the explicit kind sanctioned_acquire_merger. Manual _SESSION_LEASE seeding, forged or incomplete records, mismatched fields, and unknown provenance all remain rejected; reviewer-acquire, reviewer-heartbeat, and merger-adoption paths are untouched. B. Merger owner-session finalization New native tool gitea_release_merger_pr_lease terminally releases or abandons a merger's own comment-backed lease when the merge does not occur. It is merger-only (gitea.read + gitea.pr.comment + gitea.pr.merge; reviewer profiles lack pr.merge) with an explicit capability-map task release_merger_pr_lease / gitea_release_merger_pr_lease bound to gitea.pr.comment + role merger, and it is listed as role-exclusive in the resolver. assess_merger_lease_finalization() verifies exact session ownership, repository, PR, candidate head vs live head, profile, identity, marker presence on the thread, and the native runtime token fingerprint recorded at acquisition; foreign-session release, reviewer-profile use, and any mismatch fail closed. Finalization appends a terminal lease marker and never edits or deletes ledger history; an already-terminal lease returns already_terminal and posts nothing. The PR, approval, decision lock, branch, and worktree are all preserved. gitea_release_reviewer_pr_lease is unchanged and is not repurposed for merger sessions. "abandoned" is added to reviewer_pr_lease._TERMINAL_PHASES; without it an abandoned marker would fall through the generic non-empty phase branch and keep re-arming the lease as active. Tests: new tests/test_merger_lease_finalization.py (38 tests, 11 subtests) covers all twelve acceptance criteria — provenance sanctioning, explicit proof kind, merge-gate acceptance, manual-seed and unknown-provenance rejection, profile/role/session/head/repository/fingerprint mismatches, owner release, foreign-session refusal, reviewer refusal, append-only idempotent finalization, no surviving lease after finalization, and no regression in adoption or reviewer-lease behavior. The defect was reproduced on clean baseline |
||
|
sysadminandClaude Opus 4.8
|
15a8a76e99 |
fix(mcp): complete canonical-root consumption for cross-repo namespaces (Closes #739)
#706 routed the #274 filesystem guards and the session repository slug through
the configured canonical_repository_root. Three consumption paths were left
deriving from the Gitea-Tools installation checkout.
F1 — gitea_get_runtime_context did not normalize its `remote` argument, while
gitea_whoami did. On a prgs-hosted namespace whose first native call was the
runtime-context path, the 'dadeschools' argument default was pinned: identity
resolved against the wrong host, and because first-bind is first-write-wins a
later correct gitea_whoami(remote="prgs") could not repair the binding. It now
calls _effective_remote before any host lookup, identity resolution, or session
seeding. Explicit remotes pass through untouched and a dadeschools-hosted
profile still resolves to dadeschools.
F2 — _delete_branch_repository_binding_block derived its expected slug from
_workspace_repository_slug, which reads _local_git_remote_url in PROJECT_ROOT
(always Gitea-Tools). For a cross-repository namespace this inverted the guard:
the genuinely bound target was rejected and Gitea-Tools was accepted. The
canonical-root branch already present in _trusted_session_repository is
extracted as _canonical_repository_slug and shared by both call sites. A
configured-but-unresolvable root now fails closed instead of falling back to
the installation identity; unconfigured namespaces keep the install-derived
default unchanged.
F3 — gitea_assess_master_parity measures Gitea-Tools server implementation
parity only. That is intentional and is preserved: startup_head, current_head,
in_parity, stale, and restart_required keep their existing meaning and values,
and only the server dimension gates mutations. Two separately labelled
dimensions are added — server_implementation (installation root and commit) and
target_repository (canonical root, repository slug, checkout head, last-known
remote master head, staleness) — so cross-repository commissioning evidence can
distinguish them. The target assessment makes no network call: the remote side
is read from the existing remote-tracking ref, and an unfetched target is
reported indeterminate rather than guessed at.
Verified intentional and left unchanged: the #274 workspace-membership,
author-mutation-worktree, and branches-only guards already validate against the
configured canonical root; explicit org/repo may confirm but never authorize a
binding; and the four-role repository-specific configuration surface already
passes audit and bind-time validation with a wrong root failing closed.
Tests: 31 new cases in tests/test_cross_repo_canonical_consumption.py using
real git repositories and no network. Full suite 3298 passed / 6 skipped
against a clean-baseline 3267; the 2 failures
(test_issue_702_review_findings_f1_f6, test_reconciler_supersession_close)
reproduce identically on unmodified master
|
||
|
sysadminandClaude Opus 4.8
|
61c0d73cd1 |
fix(mcp): seed cross-repo session identity from configured canonical root (#706 F1)
Addresses REQUEST_CHANGES review 457 on PR #736. Root cause: _trusted_session_repository derived the session org/repository solely from _workspace_repository_slug -> _local_git_remote_url, which runs `git remote get-url` in PROJECT_ROOT (the Gitea-Tools install checkout). A cross-repository namespace with a configured canonical_repository_root then pinned the Gitea-Tools identity while #274 filesystem membership bound the external target, so _enforce_canonical_repository_root failed closed on a self-inflicted identity mismatch and the mcp-control-plane / eagenda namespaces stayed blocked end-to-end. Fix: when a canonical_repository_root is configured (env over profile) and passes existence / git-toplevel / resolvable-remote-identity validation, the session repository slug is derived from repository_identity_slug(configured root) instead of the install remote. The derived identity is still authorized by the profile allowed_repositories allowlist (never self-authorizing); the canonical filesystem root and org/repo identity remain immutable for the session; invalid / non-git / unresolvable / unallowlisted / forged / drift cases fail closed; unconfigured single-repo Gitea-Tools namespaces keep the install-derived slug unchanged. Tests: new tests/test_issue_706_f1_seed_identity_integration.py drives the real _seed_session_context -> _enforce_canonical_repository_root path with two real temporary git repositories (install=Gitea-Tools, configured target= mcp-control-plane): env + profile config, reviewer + merger role kinds, and fail-closed cases (nonexistent / non-git / unallowlisted / forged identity drift). Reproduces the F1 block before the fix; green after. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
jcwalker3
|
6a0d7bbef4 | Merge branch 'master' into feat/issue-706-canonical-repository-root | ||
|
sysadmin
|
8d8d2d8f81 |
fix(mcp): forward explicit org/repo through author mutation preflight (Closes #735)
Session-bound author mutations (create_issue, lock_issue, create_pr, etc.) called verify_preflight_purity without org/repo, so anti-stomp filled the remote-wide REMOTES default (prgs → Timesheet) and false-positived wrong_repo against a Gitea-Tools checkout even when callers passed explicit coordinates. Mirror _resolve: prefer workspace-derived identity for omitted coordinates on session-bound tasks; keep delete_branch's explicit-target contract (#733). Also forward org/repo at author mutation call sites. Add cross-repo regression coverage. Do not change REMOTES["prgs"]["repo"]. |
||
|
sysadminandClaude Opus 4.8
|
0d8a2c2b1d |
feat(mcp): immutable canonical_repository_root for cross-repo namespaces (Closes #706)
The MCP server derived the canonical repository root from the install checkout the server script lives in (PROJECT_ROOT), so any namespace that ran the server against an external repository (e.g. eagenda-author, or the mcp-control-plane reviewer/merger namespaces) failed every mutation: the branches-only / worktree-membership guards (#274) compared the task workspace against the Gitea-Tools .git directory it can never belong to. Separate the two concepts: - PROJECT_ROOT stays the immutable code/install root. - A new, optional, namespace-scoped canonical_repository_root binds the session to the working root of the target repository. Implementation: - canonical_repository_root.py: resolve the binding from profile/env (GITEA_CANONICAL_REPOSITORY_ROOT overrides the profile field), validate existence + git identity, fail closed on missing/conflicting/forged bindings. Preserves the single-repo default when unconfigured. - session_context_binding.py: pin canonical_repository_root into the immutable #714 session context; drift fails closed. - namespace_workspace_binding.py: route the configured target root through resolve_namespace_mutation_context so #274 / membership guards evaluate against the target repository. - gitea_mcp_server.py: seed + enforce the binding in the mutation preflight (both purity-order and FORCE_PRODUCTION_GUARDS paths); validate repository identity and git common-directory membership. - gitea_config.py: validate the optional absolute-path profile field. - gitea_auth.py: surface the config-sourced field through get_profile. No weakening of root-checkout, remote/repository, or anti-stomp guards; the single-repo Gitea-Tools path is unchanged. Tests: two distinct real git repositories/worktrees prove cross-repository bindings resolve, enforce membership, and fail closed on forged/conflicting identity and simultaneous prgs/mdcps isolation. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadminandClaude Opus 4.8
|
3990fc684f |
fix(reconciler): forward explicit repository through delete_branch anti-stomp preflight (Closes #733)
gitea_delete_branch accepted explicit org/repo but did not propagate them through the anti-stomp preflight. During a deletion targeting Scaled-Tech-Consulting/Gitea-Tools, preflight resolved the remote-wide prgs default (Scaled-Tech-Consulting/Timesheet) and failed closed with wrong_repo before any deletion — because verify_preflight_purity was called without org/repo and the shared #604 anti-stomp resolution fell back to the REMOTES default. Fix (delete_branch only; REMOTES untouched): 1. Forward the explicit org/repo into verify_preflight_purity so the shared #604 anti-stomp resolution validates the *targeted* repository instead of the remote-wide default. Explicit Scaled-Tech-Consulting/Gitea-Tools now propagates through role/workspace verification, verify_preflight_purity, anti-stomp repository resolution, and the final native deletion request. 2. Add a workspace-derived repository-binding gate (_delete_branch_repository_binding_block) that validates explicit coordinates against the immutable, workspace-aligned repository identity. Wrong, substituted, or unverified coordinates fail closed — independent of the anti-stomp remote/repo guard, which by the #530 contract trusts explicit caller intent. REMOTES defaults are never consulted as an authorization scope. Preserved: reconciler-only ownership of gitea.branch.delete; author/reviewer/ merger denial; protected and preservation/evidence gates; missing coordinates still fail closed via the anti-stomp default resolution. Regression matrix (tests/test_issue_733_delete_branch_repo_forwarding.py): Timesheet-default + explicit Gitea-Tools permits an eligible deletion and forwards the coordinates; wrong/substituted/unverified coordinates fail closed; author/reviewer/merger remain denied; protected and preservation branches remain blocked; anti-stomp resolution marks explicit coordinates authoritative and fails closed on the remote-wide default for omitted coordinates; the task-capability and role-routing maps stay consistent. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01UVDxVKANhuGYzZgayDU3QS |
||
|
sysadminandClaude Opus 4.8
|
7e5eca08b3 |
fix(mcp): surface safe diagnostics for opaque mutation internal_error
Mutation tools (create_issue, create_pr, delete_branch, issue-lock) returned reason_code=internal_error / retryable=false with no class, message, HTTP status, or actionable detail. The #699/#701 tool-error boundary intentionally collapses every non-typed exception to a fixed "Internal tool error" and drops the class and message, so a mutation failure is undiagnosable — and the #695 runtime guard blocks standalone reproduction. The true failing stage was therefore invisible by two independent mechanisms. Make the failure observable and actionable without weakening the secret-free contract: - Boundary (internal_error path ONLY): capture a safe exception class (a Python type identifier, never instance text) plus a strictly-redacted `detail` (token/Authorization/Bearer credentials, JSON/kv secret values incl. short passwords, raw URLs/hostnames, and absolute filesystem paths all stripped; whitespace collapsed; length-bounded) and an optional `mutation_stage`. Typed auth/authz/network/config/http paths are unchanged and still emit no detail. - Convert raw exceptions into typed structured errors via a safe `gitea_reason_code` attribute contract: server raisers may self-declare a known reason and the boundary emits it (fixed message) instead of an opaque internal_error. Pre-flight order violations now raise `_PreflightOrderError` (a RuntimeError subclass → preflight_order_violation). - Add a `_mutation_stage` context manager tagging escaping exceptions with the stage name (preflight_purity / resolve_auth / api_*), applied to delete_branch, create_issue, create_pr. Fail-closed identity/repo/worktree/role/permission/lock/lease/ancestry gates are untouched; only error *reporting* changed. New suite test_mutation_error_diagnostics.py (18 tests) pins the redaction, class capture, stage tagging, typed conversion, and adversarial no-leak contract. Existing boundary suite (25), core server (209), and preflight (92) suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
sysadminandClaude Opus 4.8
|
7eb4884658 |
fix(auth): route delete_branch capability to the reconciler role (Closes #729)
Remote post-merge branch deletion was blocked because the capability
resolver classified delete_branch as an author-role task while
gitea.branch.delete is granted only to prgs-reconciler. No configured
profile could satisfy both the permission gate and the role gate, so
every deletion failed closed with performed=false.
Re-home delete_branch from the author role to the reconciler role in
both single-source-of-truth maps:
- task_capability_map.TASK_CAPABILITY_MAP["delete_branch"].role
- role_session_router: TASK_REQUIRED_ROLE + AUTHOR_TASKS/RECONCILER_TASKS
resolve_task_capability("delete_branch") now resolves to prgs-reconciler.
In gitea_delete_branch, the reconciler is now the delete-capable role and
performs raw deletion through the existing reconciliation-cleanup-phase
authorization gate (operator approval + safe_to_delete proof) plus the
preservation/protected guards; the prior reconciler->cleanup redirect is
removed as it would orphan that guarded flow. Author, reviewer, and
merger stay denied by the permission gate and the required-role gate.
gitea_cleanup_merged_pr_branch remains a separate reconciler-owned path
with independent merged/ancestry/ownership verification.
Tests: reconciler resolves + performs eligible deletion; author/reviewer/
merger denied even when holding the permission; protected/preservation
branches remain fail-closed; both role maps asserted in agreement.
Updated pre-existing tests that encoded the superseded author-delete /
#687 reconciler-redirect contract. Full suite: 3190 passed, 6 skipped,
1 pre-existing unrelated failure (test_issue_702 create_pr stale-binding).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01UracxyRHQw49rZBaexHdft
|