feat: implement S2 Independence — extract coordination from orchestrator context#340
Open
michael-wojcik wants to merge 13 commits into
Open
feat: implement S2 Independence — extract coordination from orchestrator context#340michael-wojcik wants to merge 13 commits into
michael-wojcik wants to merge 13 commits into
Conversation
Add shared/s2_state.py with atomic read/write/update primitives for .pact/s2-state.json using fcntl.flock + atomic rename pattern. Add s2-state-template.json with v1 schema documentation including boundaries, append-only conventions array, scope_claims, and drift_alerts fields.
Add step to worktree-setup skill to create .pact/ directory with self-contained .gitignore (containing *) for transient S2 coordination state. Directory is created during worktree setup and cleaned up automatically with worktree removal.
Replace prompt-based S2 boundary injection in orchestrate.md and comPACT.md with file-based approach. Orchestrator now writes .pact/s2-state.json with agent boundaries, session metadata, and worktree path before dispatching coders, making coordination state persistent and compaction-safe.
Add S2 state reading to agent-teams SKILL.md On Start section (read boundaries and conventions from .pact/s2-state.json) and convention/ scope_claims writing to Before Completing section. Add cleanup note to worktree-cleanup skill confirming .pact/ is removed with worktree.
PreToolUse hook on Task that checks for overlapping agent scopes at dispatch time. Reads boundaries from .pact/s2-state.json and warns if a new agent's 'owns' scope overlaps with existing agents. Warns but does not block — overlapping scopes may be intentional. Graceful degradation: no-op if state file is missing or malformed.
PostToolUse hook on Edit|Write (async, non-blocking) that detects when an agent edits a file in another agent's 'owns' scope. Appends drift_alert entries to .pact/s2-state.json for orchestrator visibility. Performance ceiling: <50ms per invocation. Uses PACT_DEBUG env var for timing measurement. Graceful degradation: no-op if state file is missing or malformed.
Document the three mechanical S2 coordination layers: shared state file (.pact/s2-state.json), conflict detection hook (s2_conflict_check.py), and drift detection hook (s2_drift_check.py). Explains the full lifecycle from bootstrap through seed, agent read/write, to cleanup.
The platform provides CLAUDE_CODE_AGENT_NAME, not CLAUDE_AGENT_NAME. Using the wrong name caused agent_name to always resolve to "unknown", breaking self-edit exclusion and producing false drift alerts.
56 tests covering s2_state.py: schema validation, read/write round-trip (all field types), graceful degradation (missing/corrupted/empty file), concurrent writes via threading (3-5 threads), atomic update cycle, resolve_convention (last-per-key semantics), check_boundary_overlap (overlapping/disjoint/prefix/3-agent chains), file_in_scope matching, and path helpers. 90%+ coverage target for shared module.
57 tests covering s2_conflict_check.py (25) and s2_drift_check.py (32): conflict hook — overlapping/disjoint scopes, graceful degradation, agent name extraction, worktree discovery, main() stdin/stdout contract, exception handler, suppressOutput mutual exclusivity, debug timing. drift hook — cross-scope/within-scope edits, multiple affected agents, concurrent alert appends, CLAUDE_CODE_AGENT_NAME env var verification, relative path conversion, main() I/O contract, performance benchmark (<50ms with 10-agent/50-path state). 80%+ coverage for hooks.
The previous implementation acquired flock on the data file (s2-state.json) then atomically renamed a temp file over it. On POSIX, flock is tied to the inode via the file descriptor, so after rename the path points to a new inode — concurrent writers would lock different inodes, defeating serialization. Fix: lock a separate sentinel file (.pact/s2-state.lock) that is never replaced. All writers contend on the same inode. The data file is still atomically renamed, preserving the "no partial reads" guarantee for concurrent readers.
…mance M1: Normalize scope paths with trailing slash before prefix matching to prevent false positives (e.g., "src/server" matching "src/server_backup/"). Adds _normalize_scope() helper used by file_in_scope() and check_boundary_overlap(). M2: Extract duplicate _discover_worktree_path() from both hooks into shared/s2_state.py. Both hooks now import from the shared module. M4: Clarify write_s2_state docstring — single-writer initial seed only, use update_s2_state() for concurrent operations. M5: Add comment to read_s2_state explaining lockless reads are safe due to atomic rename guarantees. F1: Check PACT_WORKTREE_ROOT env var before subprocess call in _discover_worktree_path() (~5ms savings per invocation). F2: Cap drift_alerts array to last 50 entries to prevent unbounded growth. F5: Validate json.loads() result is a dict in read_s2_state(); return None with warning for non-dict JSON. F6: Replace string literal "callable[[dict], dict]" with proper Callable[[dict], dict] from typing module.
M3: Add docstring comment to concurrent write tests clarifying that threading tests verify crash-safety, not serialization — cross-process flock is the actual coordination mechanism. F3: Add TestUpdateWithoutFlock class that patches HAS_FLOCK = False and exercises the _update_without_flock path: basic update, concurrent updates (threading), and graceful degradation from empty state. F4: Add TestWriteFailureCleanup class that mocks OS-level failures and verifies temp file cleanup and original file preservation. Also adds tests for _normalize_scope (M1), _discover_worktree_path env var caching (F1/M2), and updates existing non-dict JSON tests to match new F5 validation behavior.
Collaborator
Author
Rebase Required (after PR #344)This PR uses # Before (always empty):
agent_name = os.environ.get("CLAUDE_CODE_AGENT_NAME", "")
# After:
from shared.pact_context import resolve_agent_name
import shared.pact_context as pact_context
# In main(), after json.load(sys.stdin):
pact_context.init(input_data)
agent_name = resolve_agent_name(input_data)The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts S2 (coordination) functions from the orchestrator's context window into mechanical, file-based mechanisms:
.pact/s2-state.json): Shared coordination state with agent boundaries, conventions, scope claims, and drift alerts. Atomic read/write viafcntl.flock+ rename.s2_conflict_check.py): PreToolUse hook on Task — warns when dispatching agents with overlapping scope boundaries.s2_drift_check.py): PostToolUse hook on Edit/Write (async) — records when agents edit files outside their assigned scope.Reduces orchestrator token consumption for coordination by >50% for parallel dispatch. S2 state survives context compaction.
Design doc:
docs/design/vsm-threat-1-s2-independence.mdPlan:
docs/plans/s2-independence-plan.mdFiles Changed (15 files, +2788/-10)
New files (7):
pact-plugin/hooks/shared/s2_state.py— Atomic read/write/update primitivespact-plugin/hooks/s2_conflict_check.py— PreToolUse conflict detectionpact-plugin/hooks/s2_drift_check.py— PostToolUse drift detection (async)pact-plugin/templates/s2-state-template.json— Schema template with docspact-plugin/tests/test_s2_state.py— 56 unit testspact-plugin/tests/test_s2_conflict_check.py— 25 unit testspact-plugin/tests/test_s2_drift_check.py— 32 unit testsModified files (8):
pact-plugin/commands/orchestrate.md— S2 state seeding at dispatchpact-plugin/commands/comPACT.md— S2 state seeding at dispatchpact-plugin/hooks/hooks.json— Hook registrationspact-plugin/hooks/shared/__init__.py— Module exportspact-plugin/protocols/pact-s2-coordination.md— Protocol updatepact-plugin/skills/pact-agent-teams/SKILL.md— Agent read/write instructionspact-plugin/skills/worktree-setup/SKILL.md— .pact/ directory bootstrappact-plugin/skills/worktree-cleanup/SKILL.md— Cleanup noteTest plan