fix(mcp): forward worktree_path into publication preflight (Closes #815) #817

Merged
sysadmin merged 1 commits from fix/issue-815-preflight-worktree-forwarding into master 2026-07-22 18:34:31 -05:00
Owner

Closes #815

Head: 95a5eb254f7758b694a26b3f5629558cb88db328.

The defect

gitea_publish_unpublished_issue_branch declares worktree_path as a required parameter but resolved it only after verify_preflight_purity had already run:

verify_preflight_purity(remote, task=task, org=org, repo=repo)   # 9176 — no worktree_path
...
workspace = os.path.realpath(os.path.abspath((worktree_path or "").strip() or "."))  # 9180

Every workspace-resolution layer behind that preflight — canonical root, root checkout, create-issue bootstrap, the #618 branches-only guard, issue scope, and anti-stomp — therefore received None and fell back to the MCP process root. A daemon rooted at the stable control checkout refused a valid registered issue worktree the caller had explicitly supplied, before the publication assessor at 9215 was ever constructed. Confirmed against master 910b6edbdcf0 with mutation_safe: true — not a staleness defect.

This is the only verify_preflight_purity call site that accepts a caller-supplied worktree and drops it; all thirteen others (lines 1804, 3783, 4011, 4482, 10485, 12302, 12460, 14468, 14952, 17363, 17505, 18806, 18866) forward it.

The fix

Resolve the workspace once, before preflight, and forward it:

explicit_worktree = (worktree_path or "").strip()
workspace = os.path.realpath(os.path.abspath(explicit_worktree or "."))
verify_preflight_purity(
    remote,
    worktree_path=workspace if explicit_worktree else None,
    task=task, org=org, repo=repo,
)

A blank or absent path forwards None and keeps the ordinary #618 fail-closed fallback, so guard strictness is unchanged for missing, empty, unregistered, foreign, or control-checkout worktrees. The value bound to workspace and handed to the assessor is byte-for-byte what the old line 9180 produced, so ownership, cleanliness, hash, ancestry, and read-after-write protections from PR #814 are untouched. The public tool signature and return contract do not change, so docs/mcp-tool-inventory.md is not regenerated.

Why the #812 suite missed it

The AC20 fixture sets self.worktree = os.path.realpath(self.repo) and patches PROJECT_ROOT to that same path, so the fallback resolved to the very worktree the argument named. The production topology — control checkout on a stable branch, issue worktree elsewhere — was never built, and preflight additionally no-ops under pytest unless production guards are forced on.

Tests

tests/test_issue_815_preflight_worktree_forwarding.py, 21 cases against synthetic fixtures only (real git repo + real bare remote):

  • Capture (#735 pattern): the argument reaches verify_preflight_purity; a blank path forwards None; org/repo/task forwarding preserved.
  • Faithful production reproduction: a control-rooted daemon with no session lock and only the explicit worktree — clears #618 on this source, and is trapped at #618 on the unpatched source. This case fails against master, which is what makes the suite a regression rather than a smoke test.
  • End-to-end publication in the real topology: PROJECT_ROOT is the stable control checkout, the issue worktree is a distinct registered path under branches/, GITEA_TEST_FORCE_PRODUCTION_GUARDS=1 so the #618 guard genuinely runs; publish, dry-run, dry-run/apply agreement, and read-after-write verification.
  • Negative coverage: blank and control-checkout paths trap at #618; unregistered and missing paths fail closed; dirty tree, changed head, foreign claimant, and competing PR are still refused by the assessor; the durable lock file is byte-identical across a refusal.

Reverting only the source hunk turns 3 of these red (2 capture + the production reproduction); restoring it turns them green.

Full suite: 11 failed, 4375 passed, 6 skipped, 537 subtests passed. The 11 failures are the documented pre-existing drift baseline — test_commit_payloads.py, test_issue_702_review_findings_f1_f6.py, test_mcp_server.py, test_post_merge_moot_lease.py, test_reconciler_supersession_close.py — unchanged in count and identity; none touch this tool.

Prior completed work

  • #812 / #814 — specified and landed AC20. This is a newly discovered production regression in what #814 shipped, tracked under its own issue #815, not a reopening of #812.
  • #735 / #733 — the same defect class (an explicit argument dropped at the preflight boundary) for org/repo; prior art for the correction shape.
  • #635 — the protected branch this unblocks in production. Its worktree, branch, commit b2f6e9a, hashes, and durable lock were not read into, written to, or otherwise disturbed by this work; a test asserts the fixture never names it.

🤖 Generated with Claude Code

Closes #815 Head: `95a5eb254f7758b694a26b3f5629558cb88db328`. ## The defect `gitea_publish_unpublished_issue_branch` declares `worktree_path` as a **required** parameter but resolved it only *after* `verify_preflight_purity` had already run: ```python verify_preflight_purity(remote, task=task, org=org, repo=repo) # 9176 — no worktree_path ... workspace = os.path.realpath(os.path.abspath((worktree_path or "").strip() or ".")) # 9180 ``` Every workspace-resolution layer behind that preflight — canonical root, root checkout, create-issue bootstrap, the #618 branches-only guard, issue scope, and anti-stomp — therefore received `None` and fell back to the MCP process root. A daemon rooted at the stable control checkout refused a valid registered issue worktree the caller had explicitly supplied, before the publication assessor at 9215 was ever constructed. Confirmed against master `910b6edbdcf0` with `mutation_safe: true` — not a staleness defect. This is the **only** `verify_preflight_purity` call site that accepts a caller-supplied worktree and drops it; all thirteen others (lines 1804, 3783, 4011, 4482, 10485, 12302, 12460, 14468, 14952, 17363, 17505, 18806, 18866) forward it. ## The fix Resolve the workspace once, before preflight, and forward it: ```python explicit_worktree = (worktree_path or "").strip() workspace = os.path.realpath(os.path.abspath(explicit_worktree or ".")) verify_preflight_purity( remote, worktree_path=workspace if explicit_worktree else None, task=task, org=org, repo=repo, ) ``` A blank or absent path forwards `None` and keeps the ordinary #618 fail-closed fallback, so guard strictness is unchanged for missing, empty, unregistered, foreign, or control-checkout worktrees. The value bound to `workspace` and handed to the assessor is byte-for-byte what the old line 9180 produced, so ownership, cleanliness, hash, ancestry, and read-after-write protections from PR #814 are untouched. The public tool signature and return contract do not change, so `docs/mcp-tool-inventory.md` is not regenerated. ## Why the #812 suite missed it The AC20 fixture sets `self.worktree = os.path.realpath(self.repo)` and patches `PROJECT_ROOT` to that same path, so the fallback resolved to the very worktree the argument named. The production topology — control checkout on a stable branch, issue worktree elsewhere — was never built, and preflight additionally no-ops under pytest unless production guards are forced on. ## Tests `tests/test_issue_815_preflight_worktree_forwarding.py`, 21 cases against synthetic fixtures only (real git repo + real bare remote): - **Capture** (#735 pattern): the argument reaches `verify_preflight_purity`; a blank path forwards `None`; org/repo/task forwarding preserved. - **Faithful production reproduction**: a control-rooted daemon with **no** session lock and only the explicit worktree — clears #618 on this source, and is trapped at #618 on the unpatched source. This case *fails* against master, which is what makes the suite a regression rather than a smoke test. - **End-to-end publication** in the real topology: `PROJECT_ROOT` is the stable control checkout, the issue worktree is a distinct registered path under `branches/`, `GITEA_TEST_FORCE_PRODUCTION_GUARDS=1` so the #618 guard genuinely runs; publish, dry-run, dry-run/apply agreement, and read-after-write verification. - **Negative** coverage: blank and control-checkout paths trap at #618; unregistered and missing paths fail closed; dirty tree, changed head, foreign claimant, and competing PR are still refused by the assessor; the durable lock file is byte-identical across a refusal. Reverting only the source hunk turns 3 of these red (2 capture + the production reproduction); restoring it turns them green. Full suite: **11 failed, 4375 passed, 6 skipped, 537 subtests passed**. The 11 failures are the documented pre-existing drift baseline — `test_commit_payloads.py`, `test_issue_702_review_findings_f1_f6.py`, `test_mcp_server.py`, `test_post_merge_moot_lease.py`, `test_reconciler_supersession_close.py` — unchanged in count and identity; none touch this tool. ## Prior completed work - **#812** / **#814** — specified and landed AC20. This is a newly discovered production regression in what #814 shipped, tracked under its own issue #815, not a reopening of #812. - **#735** / **#733** — the same defect class (an explicit argument dropped at the preflight boundary) for `org`/`repo`; prior art for the correction shape. - **#635** — the protected branch this unblocks in production. Its worktree, branch, commit `b2f6e9a`, hashes, and durable lock were not read into, written to, or otherwise disturbed by this work; a test asserts the fixture never names it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jcwalker3 added 1 commit 2026-07-22 17:28:39 -05:00
gitea_publish_unpublished_issue_branch accepted a required worktree_path
but resolved it only after verify_preflight_purity had already run, so the
#618 branches-only guard and every other workspace-resolution layer behind
preflight received None and fell back to the MCP process root. A daemon
rooted at the stable control checkout therefore refused a valid registered
issue worktree the caller had explicitly supplied, before the publication
assessor could use it — the sole verify_preflight_purity call site that
accepted a worktree argument and dropped it.

Resolve the workspace once, before preflight, and forward it. A blank or
absent path forwards None and keeps the ordinary #618 fail-closed fallback,
so guard strictness is unchanged for missing, empty, unregistered, foreign,
or control-checkout worktrees. Public tool contract, ownership, cleanliness,
hash, ancestry, and read-after-write protections from PR #814 are untouched.

Adds tests/test_issue_815_preflight_worktree_forwarding.py: a #735-style
capture proving the argument reaches verify_preflight_purity, a faithful
production reproduction (control-rooted daemon, no session lock, explicit
worktree) that clears #618 on the fixed source and is trapped at #618 on the
unpatched source, an end-to-end control-rooted publication in the real
topology (PROJECT_ROOT is the stable control checkout, the issue worktree is
a distinct registered path, production guards forced on), and negative
coverage keeping every #618 and assessor refusal intact. The prior #812
suite masked the defect by patching PROJECT_ROOT to equal the issue worktree.

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

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50184-e42e27f730de
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T22:59:21Z
expires_at: 2026-07-22T23:09:21Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50184-e42e27f730de worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817 phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-22T22:59:21Z expires_at: 2026-07-22T23:09:21Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50184-e42e27f730de
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T23:01:08Z
expires_at: 2026-07-22T23:11:08Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50184-e42e27f730de worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817 phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-22T23:01:08Z expires_at: 2026-07-22T23:11:08Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50184-e42e27f730de
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T23:01:58Z
expires_at: 2026-07-22T23:11:58Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50184-e42e27f730de worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817 phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-22T23:01:58Z expires_at: 2026-07-22T23:11:58Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50184-e42e27f730de
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T23:03:55Z
expires_at: 2026-07-22T23:13:55Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50184-e42e27f730de worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817 phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-22T23:03:55Z expires_at: 2026-07-22T23:13:55Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50184-e42e27f730de
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T23:04:42Z
expires_at: 2026-07-22T23:14:42Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50184-e42e27f730de worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-worktree-pr817 phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-22T23:04:42Z expires_at: 2026-07-22T23:14:42Z blocker: none
sysadmin approved these changes 2026-07-22 18:05:48 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE: review-in-progress
WHO_IS_NEXT: reviewer
NEXT_ACTION: Complete formal PR review submission
NEXT_PROMPT:

Complete formal PR review submission

WHAT_HAPPENED: Independent reviewer sysadmin conducted preflight checks, source diff analysis, security boundary verification, regression proof verification, and test suite execution for PR #817. All acceptance criteria AC1-AC10 for Issue #815 are satisfied.
WHY: PR #817 correctly resolves explicit worktree_path before verify_preflight_purity is called in gitea_publish_unpublished_issue_branch, allowing control-rooted daemons to publish registered issue worktrees.
ISSUE: #815
HEAD_SHA: 95a5eb254f
REVIEW_STATUS: PENDING
MERGE_READY: false
BLOCKERS: none
VALIDATION: 21 focused regression tests passed in tests/test_issue_815_preflight_worktree_forwarding.py. Source revert proof confirmed 3 failures on unpatched code. Full suite matching pre-existing baseline (11 failed, 4375 passed, 6 skipped).
LAST_UPDATED_BY: sysadmin

PR #817 Independent Review Details

## Canonical PR State STATE: review-in-progress WHO_IS_NEXT: reviewer NEXT_ACTION: Complete formal PR review submission NEXT_PROMPT: ```text Complete formal PR review submission ``` WHAT_HAPPENED: Independent reviewer sysadmin conducted preflight checks, source diff analysis, security boundary verification, regression proof verification, and test suite execution for PR #817. All acceptance criteria AC1-AC10 for Issue #815 are satisfied. WHY: PR #817 correctly resolves explicit worktree_path before verify_preflight_purity is called in gitea_publish_unpublished_issue_branch, allowing control-rooted daemons to publish registered issue worktrees. ISSUE: #815 HEAD_SHA: 95a5eb254f7758b694a26b3f5629558cb88db328 REVIEW_STATUS: PENDING MERGE_READY: false BLOCKERS: none VALIDATION: 21 focused regression tests passed in tests/test_issue_815_preflight_worktree_forwarding.py. Source revert proof confirmed 3 failures on unpatched code. Full suite matching pre-existing baseline (11 failed, 4375 passed, 6 skipped). LAST_UPDATED_BY: sysadmin ### PR #817 Independent Review Details - **Reviewed Head SHA:** 95a5eb254f7758b694a26b3f5629558cb88db328 - **Owning Issue:** #815 - **Author:** jcwalker3 - **Reviewer:** sysadmin (prgs-reviewer)
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #817
issue: #815
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 33243-d5fa2dafb10e
worktree: /Users/jasonwalker/Development/Gitea-Tools
phase: claimed
candidate_head: 95a5eb254f
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T23:34:02Z
expires_at: 2026-07-22T23:44:02Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #817 issue: #815 reviewer_identity: sysadmin profile: prgs-merger session_id: 33243-d5fa2dafb10e worktree: /Users/jasonwalker/Development/Gitea-Tools phase: claimed candidate_head: 95a5eb254f7758b694a26b3f5629558cb88db328 target_branch: master target_branch_sha: none last_activity: 2026-07-22T23:34:02Z expires_at: 2026-07-22T23:44:02Z blocker: none
sysadmin merged commit 14c9c4d702 into master 2026-07-22 18:34:31 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-22T23:34:33.366645+00:00` - last terminal: `approve` on PR #817 - PR state: `closed` (merged=True) - merge_commit_sha: `14c9c4d702055056ff1b25aa93aa6b55df79d1ac` - prior live_mutations_count: `1` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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