feat: harden issue lock store against concurrent session clobbering (Closes #438) #464

Merged
sysadmin merged 1 commits from feat/issue-438-lock-hardening into master 2026-07-07 17:37:47 -05:00
Owner

Summary

Closes #438. Replaces the global read-modify-write issue lock with atomic per-issue locks so concurrent author sessions working different issues no longer clobber /tmp/gitea_issue_lock.json.

Changes

  • issue_lock_store.py — flock-serialized per-issue locks under GITEA_ISSUE_LOCK_DIR, PID/session metadata, stale recovery gates, lock proof formatting, live-lock inventory
  • gitea_mcp_server.py — wire atomic acquire into gitea_lock_issue; re-check lock freshness before gitea_create_pr; expose live_issue_locks in claim inventory
  • merged_cleanup_reconcile.py — treat live per-issue locks as active branch protection
  • scripts/worktree-start — resolve locks by branch via per-issue store
  • tests/test_issue_lock_store.py — concurrency + stale recovery regression cases
  • docs/llm-workflow-runbooks.md — lock proof reporting guidance

Verification

python -m unittest tests.test_issue_lock_store tests.test_mcp_server.TestIssueLocking tests.test_merged_cleanup_reconcile -q
# 31 tests OK

Worktree

branches/issue-438-lock-hardening @ 992d055

## Summary Closes #438. Replaces the global read-modify-write issue lock with atomic per-issue locks so concurrent author sessions working different issues no longer clobber `/tmp/gitea_issue_lock.json`. ## Changes - `issue_lock_store.py` — flock-serialized per-issue locks under `GITEA_ISSUE_LOCK_DIR`, PID/session metadata, stale recovery gates, lock proof formatting, live-lock inventory - `gitea_mcp_server.py` — wire atomic acquire into `gitea_lock_issue`; re-check lock freshness before `gitea_create_pr`; expose `live_issue_locks` in claim inventory - `merged_cleanup_reconcile.py` — treat live per-issue locks as active branch protection - `scripts/worktree-start` — resolve locks by branch via per-issue store - `tests/test_issue_lock_store.py` — concurrency + stale recovery regression cases - `docs/llm-workflow-runbooks.md` — lock proof reporting guidance ## Verification ```bash python -m unittest tests.test_issue_lock_store tests.test_mcp_server.TestIssueLocking tests.test_merged_cleanup_reconcile -q # 31 tests OK ``` ## Worktree `branches/issue-438-lock-hardening` @ `992d055`
jcwalker3 force-pushed feat/issue-438-lock-hardening from 992d055998 to 2cace4f7cd 2026-07-07 16:25:18 -05:00 Compare
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #464
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 47646-67c1146dc04f
worktree: branches/review-feat-issue-438-lock-hardening
phase: claimed
candidate_head: 2cace4f7cd
target_branch: master
target_branch_sha: none
last_activity: 2026-07-07T21:54:23Z
expires_at: 2026-07-07T23:54:23Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #464 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 47646-67c1146dc04f worktree: branches/review-feat-issue-438-lock-hardening phase: claimed candidate_head: 2cace4f7cd8dd7903a83c6cbcc735089f2c5b654 target_branch: master target_branch_sha: none last_activity: 2026-07-07T21:54:23Z expires_at: 2026-07-07T23:54:23Z blocker: none
sysadmin requested changes 2026-07-07 16:54:28 -05:00
Dismissed
sysadmin left a comment
Owner

Independent audit — REQUEST CHANGES (syntax error & rebase required)

Auditor: sysadmin / prgs-reviewer (not PR author jcwalker3).
Pinned HEAD: 2cace4f7cd8dd7903a83c6cbcc735089f2c5b654

Worktree + Cleanliness

  • Fresh isolated review worktree created successfully.
  • git status clean before and after running tests.

Blockers & Regression Failures

Running the full test suite (pytest) revealed 10 failures:

1. Syntax error in scripts/worktree-start:

6 tests in tests/test_worktrees.py failed with:
scripts/worktree-start: line 50: syntax error near unexpected token '('
Root Cause: On line 45, the command substitution closing parenthesis was incorrectly placed at the end of the line: locked_branch=$(PYTHONPATH="$repo_root" python3 - <<'PY' "$branch"). This ends the substitution immediately, causing bash to attempt parsing the python heredoc content on lines 46-55 as bash commands.
Fix: Move the closing parenthesis from the end of line 45 to line 56, right after the heredoc terminator PY.

2. Network connection regressions:

4 other tests (in test_agent_temp_artifacts.py, test_issue_work_duplicate_gate.py, and test_mcp_server.py) failed with:
RuntimeError: network error contacting Gitea: [Errno 8] nodename nor servname provided, or not known
Root Cause: The branch is not rebased onto the current master (post-PR #449/post-PR #462), causing a mock-safety mismatch for the Gitea API requests.

Required Action

  1. Fix the syntax error in scripts/worktree-start by correcting the command substitution parenthesis.
  2. Rebase the branch onto current master to inherit the mock-safety test alignment fixes.
## Independent audit — REQUEST CHANGES (syntax error & rebase required) Auditor: `sysadmin` / `prgs-reviewer` (not PR author `jcwalker3`). Pinned HEAD: `2cace4f7cd8dd7903a83c6cbcc735089f2c5b654` ### Worktree + Cleanliness - Fresh isolated review worktree created successfully. - `git status` clean before and after running tests. ### Blockers & Regression Failures Running the full test suite (`pytest`) revealed **10 failures**: #### 1. Syntax error in `scripts/worktree-start`: 6 tests in `tests/test_worktrees.py` failed with: `scripts/worktree-start: line 50: syntax error near unexpected token '('` *Root Cause*: On line 45, the command substitution closing parenthesis was incorrectly placed at the end of the line: `locked_branch=$(PYTHONPATH="$repo_root" python3 - <<'PY' "$branch")`. This ends the substitution immediately, causing bash to attempt parsing the python heredoc content on lines 46-55 as bash commands. *Fix*: Move the closing parenthesis from the end of line 45 to line 56, right after the heredoc terminator `PY`. #### 2. Network connection regressions: 4 other tests (in `test_agent_temp_artifacts.py`, `test_issue_work_duplicate_gate.py`, and `test_mcp_server.py`) failed with: `RuntimeError: network error contacting Gitea: [Errno 8] nodename nor servname provided, or not known` *Root Cause*: The branch is not rebased onto the current master (post-PR #449/post-PR #462), causing a mock-safety mismatch for the Gitea API requests. ### Required Action 1. Fix the syntax error in `scripts/worktree-start` by correcting the command substitution parenthesis. 2. Rebase the branch onto current master to inherit the mock-safety test alignment fixes.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #464
issue: #438
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 47646-67c1146dc04f
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-feat-issue-438-lock-hardening
phase: claimed
candidate_head: 2cace4f7cd
target_branch: master
target_branch_sha: none
last_activity: 2026-07-07T21:56:53Z
expires_at: 2026-07-07T23:56:53Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #464 issue: #438 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 47646-67c1146dc04f worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-feat-issue-438-lock-hardening phase: claimed candidate_head: 2cace4f7cd8dd7903a83c6cbcc735089f2c5b654 target_branch: master target_branch_sha: none last_activity: 2026-07-07T21:56:53Z expires_at: 2026-07-07T23:56:53Z blocker: none
jcwalker3 added 1 commit 2026-07-07 17:07:48 -05:00
Rebase onto master (#443 keyed store) and port #438 hardening: per-issue
fcntl.flock around bind_session_lock, assess_lock_freshness with dead-PID
detection, verify_lock_for_mutation before create_pr, and live lock
inventory for claim visibility.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 force-pushed feat/issue-438-lock-hardening from 842776f916 to d306c0a5ec 2026-07-07 17:07:48 -05:00 Compare
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #464
issue: #438
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 47646-67c1146dc04f
worktree: branches/review-pr464-issue-438-lock-hardening
phase: claimed
candidate_head: d306c0a5ec
target_branch: master
target_branch_sha: 3c41d2af5c
last_activity: 2026-07-07T22:36:04Z
expires_at: 2026-07-08T00:36:04Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #464 issue: #438 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 47646-67c1146dc04f worktree: branches/review-pr464-issue-438-lock-hardening phase: claimed candidate_head: d306c0a5ec0c33ce8168c2c0db6afc8062ebba58 target_branch: master target_branch_sha: 3c41d2af5cec9aefb66adf66419d8e8c5c83d5e6 last_activity: 2026-07-07T22:36:04Z expires_at: 2026-07-08T00:36:04Z blocker: none
sysadmin approved these changes 2026-07-07 17:36:18 -05:00
sysadmin left a comment
Owner

Fresh review at head d306c0a5ec0c33ce8168c2c0db6afc8062ebba58

Supersedes stale review/REQUEST_CHANGES at 2cace4f.

Prior REQUEST_CHANGES resolution

Blocker Verdict
scripts/worktree-start bash syntax (bash -n failure on heredoc) Resolved — PR head uses master's valid python3 -c + resolve_locked_branch_for_session; bash -n passes; pytest tests/test_worktrees.py passes
Lock-store rebase/integration gaps Resolved — flock hardening ported onto #443 bind_session_lock foundation; merge simulation into current master (3c41d2a) auto-merges cleanly and retains #461 boundary-aware adoption

Preflight

  • Root: clean master @ 3c41d2a
  • Identity: sysadmin / prgs-reviewer
  • review_pr: allowed; self_author false (author jcwalker3)
  • Pinned head: d306c0a exact match; mergeable true
  • #465 landed: 54a7515; #461 landed: 3c41d2a

Scope reviewed (vs prgs/master)

  • issue_lock_store.py — flock serialization, assess_lock_freshness, verify_lock_for_mutation, list_live_locks, format_lock_proof
  • gitea_mcp_server.py — lock proof on gitea_lock_issue, verify_lock_for_mutation before gitea_create_pr, live_issue_locks inventory
  • tests/test_issue_lock_store.py — concurrency + stale-lock cases

Note: branch tip is based on 54a7515 (pre-#461) but merges cleanly into 3c41d2a; simulated merge keeps _branch_carries_issue_marker from #461.

Clobber-hazard verdict — PASS

  • fcntl.flock wraps bind_session_lock read-check-write
  • assess_foreign_lock_overwrite + assess_same_issue_lease_conflict enforced inside critical section
  • LockContentionError surfaces concurrent same-issue contention fail-closed
  • No manual lock seeding, raw API fallback, branch deletion, or force-push recovery added by this diff
  • Adoption path unchanged at merge time (#461 preserved)

Tests

  • pytest tests/test_issue_lock_store.py tests/test_issue_lock_adoption.py tests/test_issue_lock_worktree.py -q → 27 passed
  • pytest tests/test_mcp_server.py -k "lock or adopt" → 47 passed
  • Full suite on PR head → 1697 passed, 6 skipped
  • Merge-sim focused lock/worktree tests → 44 passed
  • git diff --check → clean

Decision: APPROVE at d306c0a. Separate merge session recommended. Keep #473 parked until #464 merges and #438 closes.

## Fresh review at head `d306c0a5ec0c33ce8168c2c0db6afc8062ebba58` Supersedes stale review/REQUEST_CHANGES at `2cace4f`. ### Prior REQUEST_CHANGES resolution | Blocker | Verdict | |---|---| | `scripts/worktree-start` bash syntax (`bash -n` failure on heredoc) | **Resolved** — PR head uses master's valid `python3 -c` + `resolve_locked_branch_for_session`; `bash -n` passes; `pytest tests/test_worktrees.py` passes | | Lock-store rebase/integration gaps | **Resolved** — flock hardening ported onto #443 `bind_session_lock` foundation; merge simulation into current master (`3c41d2a`) auto-merges cleanly and retains #461 boundary-aware adoption | ### Preflight - Root: clean `master` @ `3c41d2a` - Identity: `sysadmin` / `prgs-reviewer` - `review_pr`: allowed; self_author **false** (author `jcwalker3`) - Pinned head: `d306c0a` exact match; mergeable **true** - #465 landed: `54a7515`; #461 landed: `3c41d2a` ### Scope reviewed (vs `prgs/master`) - `issue_lock_store.py` — flock serialization, `assess_lock_freshness`, `verify_lock_for_mutation`, `list_live_locks`, `format_lock_proof` - `gitea_mcp_server.py` — lock proof on `gitea_lock_issue`, `verify_lock_for_mutation` before `gitea_create_pr`, `live_issue_locks` inventory - `tests/test_issue_lock_store.py` — concurrency + stale-lock cases Note: branch tip is based on `54a7515` (pre-#461) but merges cleanly into `3c41d2a`; simulated merge keeps `_branch_carries_issue_marker` from #461. ### Clobber-hazard verdict — PASS - `fcntl.flock` wraps `bind_session_lock` read-check-write - `assess_foreign_lock_overwrite` + `assess_same_issue_lease_conflict` enforced inside critical section - `LockContentionError` surfaces concurrent same-issue contention fail-closed - No manual lock seeding, raw API fallback, branch deletion, or force-push recovery added by this diff - Adoption path unchanged at merge time (#461 preserved) ### Tests - `pytest tests/test_issue_lock_store.py tests/test_issue_lock_adoption.py tests/test_issue_lock_worktree.py -q` → 27 passed - `pytest tests/test_mcp_server.py -k "lock or adopt"` → 47 passed - Full suite on PR head → 1697 passed, 6 skipped - Merge-sim focused lock/worktree tests → 44 passed - `git diff --check` → clean **Decision: APPROVE** at `d306c0a`. Separate merge session recommended. Keep #473 parked until #464 merges and #438 closes.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #464
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 47646-67c1146dc04f
worktree: branches/review-feat-issue-438-lock-hardening
phase: claimed
candidate_head: d306c0a5ec
target_branch: master
target_branch_sha: none
last_activity: 2026-07-07T22:37:43Z
expires_at: 2026-07-08T00:37:43Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #464 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 47646-67c1146dc04f worktree: branches/review-feat-issue-438-lock-hardening phase: claimed candidate_head: d306c0a5ec0c33ce8168c2c0db6afc8062ebba58 target_branch: master target_branch_sha: none last_activity: 2026-07-07T22:37:43Z expires_at: 2026-07-08T00:37:43Z blocker: none
sysadmin merged commit a1de4ab8f9 into master 2026-07-07 17:37:47 -05:00
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#464