Exp #36: Harden tmux key submission and add structured completion detection - #1364
Exp #36: Harden tmux key submission and add structured completion detection#1364xukai92 wants to merge 1 commit into
Conversation
…on (#36) Replace C-m with two-step hex 0d approach for reliable key submission in interactive Claude Code sessions. Add structured JSON sentinel files for completion detection with backward compat, and _check_claude_agents_state advisory function for session lifecycle queries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1364 +/- ##
==========================================
- Coverage 84.66% 84.65% -0.01%
==========================================
Files 228 228
Lines 25395 25450 +55
Branches 4075 4084 +9
==========================================
+ Hits 21500 21545 +45
- Misses 2926 2932 +6
- Partials 969 973 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ Factory Review: KEEPVerdict: KEEP Posted by Factory CEO |
|
@ceo-review |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 60/60 tests pass, 0 critical issues. 1 important finding (shell escaping of session_id in single-quoted printf — low-likelihood, local filesystem paths only). Adversarial QA verified all 4 features with evidence. All deliverables implemented.
QA Analysis
Adversarial QA Report — PR #1364
PR: fix: harden tmux key submission and add structured completion detection (#36)
Detected project type: Library (Python CLI with tmux integration)
Date: 2026-08-21
Smoke Test
Status: PASS
$ python -c "from factory.runners._tmux_persist import _tmux_send_enter, _verify_post_send, _parse_sentinel, _check_claude_agents_state, _generate_settings, _wait_for_sentinel, run_in_tmux, tmux_available; print('OK')"
All 10 functions imported successfully
All new functions import cleanly. No import errors, no missing dependencies.
Test Plan
Derived from PR #1364 deliverables:
_tmux_send_enter()— hex 0d two-step Enter key approach_verify_post_send()— 15s verification after key submission- JSON sentinel completion detection with backward compat
_check_claude_agents_state()— advisory agent state query
Feature Tests
1. _tmux_send_enter() helper using hex 0d
Status: VERIFIED
Evidence — mock test (verifies two-step call structure):
$ python -c "from unittest.mock import patch, MagicMock; from factory.runners._tmux_persist import _tmux_send_enter; calls = []; ..."
Number of subprocess calls: 2
Call 1: ['tmux', 'send-keys', '-t', 'test-sess:test-win', '/exit']
Call 2: ['tmux', 'send-keys', '-t', 'test-sess:test-win', '-H', '0d']
PASS: _tmux_send_enter uses two-step hex 0d approach correctly
Confirmed: No Enter or C-m literal strings used. Text and Enter are sent as two separate send-keys commands. The -H 0d flag correctly sends a raw carriage return byte.
Evidence — real tmux test (actual tmux 3.4):
$ python -c "... _tmux_send_enter('factory-test-adversarial:0', 'hello world') ..."
tmux version: tmux 3.4
Create session: rc=0
Post-send verify result: True
Pane content: 'hello world\nhello world\n...'
Session cleaned up
Confirmed: The hex 0d approach works with real tmux — text was sent and Enter was registered by the cat process.
2. _verify_post_send() with 15s verification
Status: VERIFIED
Evidence — functional tests:
$ python -c "... _verify_post_send tests ..."
Case 1 (has content): PASS - returned True
Case 2 (empty pane): PASS - returned False
Case 3 (command failed): PASS - returned False
PASS: _verify_post_send works correctly for all cases
Evidence — integration with run_in_tmux (15s delay verified):
$ python -c "... track sleep and verify calls in run_in_tmux ..."
_verify_post_send called: 1 times
Sleep calls: [15.0]
Has 15s sleep: True
PASS: _verify_post_send is called with correct delay in run_in_tmux
Confirmed: _verify_post_send() is called exactly once after a 15-second delay (the _POST_SEND_VERIFY_DELAY constant) within the run_in_tmux flow. Uses capture-pane -p to check for pane content. Returns False on command failure or empty pane, True when content is present.
3. JSON sentinel completion detection with backward compat
Status: VERIFIED
Evidence — _parse_sentinel behavior:
$ python -c "... _parse_sentinel tests ..."
Case 1 (valid JSON): PASS - got dict with completion_reason=normal
Case 2 (empty sentinel): PASS - backward compat returns True
Case 3 (whitespace sentinel): PASS - backward compat returns True
Case 4 (invalid JSON): PASS - graceful fallback returns True
Case 5 (stop_failure): PASS - got dict with completion_reason=stop_failure
Case 6 (nonexistent file): PASS - returns True on OSError
Evidence — hook commands produce valid JSON:
$ python -c "... execute Stop hook command via bash ..."
Sentinel content after Stop hook: {"completion_reason":"normal","timestamp":"2026-08-21T01:54:36Z","session_id":"test-sess:test-win"}
StopFailure sentinel: {"completion_reason":"stop_failure","timestamp":"2026-08-21T01:54:36Z","session_id":"test-sess:test-win"}
PASS: Hook commands produce valid JSON sentinel files
Evidence — backward compat truthiness:
$ python -c "... verify all sentinel formats are truthy ..."
PASS: All sentinel formats are truthy - backward compat holds
Confirmed: New JSON sentinels return a dict with completion_reason, timestamp, and session_id. Old empty sentinels return True. Invalid JSON returns True (graceful degradation). Both True and dict are truthy, so the if not sentinel_result check on line 289 of run_in_tmux works correctly for both old and new formats.
Evidence — _wait_for_sentinel async integration:
$ python -c "... async _wait_for_sentinel tests ..."
JSON sentinel: PASS - got dict
Empty sentinel: PASS - backward compat True
Timeout sentinel: PASS - returns False
Evidence — dict sentinel in run_in_tmux (no crash):
$ python -c "... run_in_tmux with dict sentinel_result ..."
Exit code: 1, stdout contains output: True
PASS: Dict sentinel result handled correctly in run_in_tmux
4. _check_claude_agents_state() advisory function
Status: VERIFIED
Evidence — all edge cases:
$ python -c "... _check_claude_agents_state tests ..."
Case 1 (matching session): PASS - state=busy
Case 2 (no match): PASS - result=None
Case 3 (cmd failure): PASS - result=None
Case 4 (file not found): PASS - result=None
Case 5 (timeout): PASS - result=None
Case 6 (bad json): PASS - result=None
Case 7 (empty list): PASS - result=None
Case 8 (non-list json): PASS - result=None
Case 9 (name field match): PASS - state=waiting
Case 10 (None session_id, name match): PASS - state=idle
PASS: _check_claude_agents_state handles all edge cases correctly
Confirmed: Returns "busy", "waiting", or "idle" for matching sessions. Returns None for all failure modes: command failure, file not found (claude not installed), timeout (10s), invalid JSON, non-list JSON, empty list, no matching session. Searches both session_id and name fields. Handles None session_id gracefully via or fallback to name.
Edge Case Tests
Session ID with special characters
Status: VERIFIED (with minor note)
$ python -c "... session_id with special chars ..."
Session ID preserved correctly: factory-persist-my-project-a1b2c3:builder-d4e5f6
PASS: Special characters in session_id handled correctly
Colons, hyphens work correctly. However, a session_id containing double quotes (") would produce invalid JSON in the sentinel file. This is a theoretical edge case only — session_ids are generated internally from project name + hash + role + uuid, none of which contain double quotes. The _parse_sentinel function gracefully falls back to True for invalid JSON, so even this case would not break the system.
Subprocess timeout on _check_claude_agents_state
Status: VERIFIED
$ python -c "... inspect source for timeout=10 ..."
PASS: subprocess.run has timeout=10
PASS: TimeoutExpired is caught
PASS: Uses text=True for stdout
Test suite
Status: VERIFIED
$ python -m pytest tests/test_tmux_persist.py -v
60 passed in 0.46s
All 60 tests pass, covering:
TestTmuxAvailable(3 tests)TestStripAnsi(7 tests)TestWindowExists(2 tests)TestGenerateSettings(5 tests)TestWaitForSentinel(4 tests)TestWaitForExitcode(3 tests)TestRunInTmux(14 tests)TestTmuxSendEnter(2 tests)TestGenerateSettingsJsonSentinel(4 tests)TestParseSentinel(5 tests)TestWaitForSentinelJson(3 tests)TestCheckClaudeAgentsState(6 tests)TestClaudeRunnerTmuxPersist(3 tests)
Process cleanup
No orphaned tmux sessions:
$ tmux ls 2>&1 | grep factory
No factory tmux sessions running (good)
Acceptance Criteria Verification
| # | Criterion | Status | Notes |
|---|---|---|---|
| 1 | _tmux_send_enter() sends text + hex 0d as two separate commands |
VERIFIED | No Enter/C-m literals used; works with real tmux 3.4 |
| 2 | _verify_post_send() captures pane and checks for response |
VERIFIED | Called with 15s delay; returns bool based on pane content |
| 3 | JSON sentinel with completion_reason, timestamp, session_id |
VERIFIED | Hook commands produce valid JSON; parsed correctly |
| 4 | Backward compat: empty/text sentinels still return truthy | VERIFIED | True for empty/invalid; both old and new formats pass if not sentinel_result check |
| 5 | _check_claude_agents_state() returns state or None |
VERIFIED | All 10 edge cases handled correctly |
| 6 | Existing test suite passes | VERIFIED | 60/60 tests pass |
| 7 | No orphaned processes after testing | VERIFIED | All tmux sessions cleaned up |
Adversarial Verdict: PASS
All four features work as described. Backward compatibility is preserved. Error handling is robust across all tested edge cases. The real tmux integration test confirms the hex 0d approach works with actual tmux. The one theoretical weakness (double quotes in session_id producing invalid JSON) is mitigated by the internal generation of session_ids and the graceful fallback in _parse_sentinel.
Posted by Factory CEO
Closes #1259
Closes #1317
Changes
Key submission hardening (#1259):
_tmux_send_enter()helper that uses two-step approach: send text first, then send Enter as hex0dvia-Hflag — avoids silent failures withC-min interactive Claude Code sessions_verify_post_send()that captures pane content and logs whether a response appearedrefactory.mdandsessions.mdto use the hex approachCompletion detection (#1317):
_generate_settings()Stop/StopFailure hooks to write structured JSON sentinel (completion_reason, timestamp, session_id) instead of empty touch_wait_for_sentinel()to parse JSON sentinel and return metadata dict, with backward compat for empty sentinels_check_claude_agents_state()advisory function that queriesclaude agents --jsonfor session staterefactory.mdwith three detection methods (sentinel JSON, claude agents API, tmux has-session)claude agents --jsonusage examples tosessions.mdTesting:
_tmux_send_enter, JSON sentinel generation,_parse_sentinel(with backward compat),_check_claude_agents_state