Skip to content

Exp #36: Harden tmux key submission and add structured completion detection - #1364

Open
xukai92 wants to merge 1 commit into
mainfrom
factory/run-2bd479d6
Open

Exp #36: Harden tmux key submission and add structured completion detection#1364
xukai92 wants to merge 1 commit into
mainfrom
factory/run-2bd479d6

Conversation

@xukai92

@xukai92 xukai92 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Closes #1259
Closes #1317

Changes

Key submission hardening (#1259):

  • Extracted _tmux_send_enter() helper that uses two-step approach: send text first, then send Enter as hex 0d via -H flag — avoids silent failures with C-m in interactive Claude Code sessions
  • Added 15-second post-submission verification via _verify_post_send() that captures pane content and logs whether a response appeared
  • Updated tmux send-keys documentation in refactory.md and sessions.md to use the hex approach

Completion detection (#1317):

  • Modified _generate_settings() Stop/StopFailure hooks to write structured JSON sentinel (completion_reason, timestamp, session_id) instead of empty touch
  • Modified _wait_for_sentinel() to parse JSON sentinel and return metadata dict, with backward compat for empty sentinels
  • Added _check_claude_agents_state() advisory function that queries claude agents --json for session state
  • Added Completion Detection section to refactory.md with three detection methods (sentinel JSON, claude agents API, tmux has-session)
  • Added claude agents --json usage examples to sessions.md

Testing:

  • Added tests for _tmux_send_enter, JSON sentinel generation, _parse_sentinel (with backward compat), _check_claude_agents_state
  • Updated existing tests to work with new two-step send pattern and JSON sentinel hooks
  • All 60 tests passing, lint and type checks clean

…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>
@github-actions

Copy link
Copy Markdown

Sentrux Quality Report

Absolute

Scanning ....
[scan] git ls-files: 665 total, 651 kept, 14 dropped (ext:14, meta:0, big:0)
[build_project_map] 651 files, 109 unique dirs, 101 cache misses, 6.3ms
[resolve] 1223 resolved, 1631 unresolved (of 2854 total specs)
[resolve_imports] project_map 6.4ms, suffix_idx 1.2ms, suffix_resolve 18.9ms, total 26.5ms
[build_graphs] 651 files | maps 2.4ms, imports 26.7ms, calls+inherit 6.7ms, total 35.8ms | 1222 import, 9477 call, 11 inherit edges
sentrux check — 3 rules checked

Quality: 4479

✗ [Error] max_cc: 3 function(s) exceed max cyclomatic complexity of 30
    factory/cli/_ceo_helpers.py:_validate_ceo_flags (cc=43)
    factory/cli/_ceo_helpers.py:_execute_ceo (cc=43)
    factory/cli/run.py:cmd_run (cc=32)

✗ 1 violation(s) found

Diff (vs base branch)

Scanning ....
[scan] git ls-files: 665 total, 651 kept, 14 dropped (ext:14, meta:0, big:0)
[build_project_map] 651 files, 109 unique dirs, 101 cache misses, 6.2ms
[resolve] 1223 resolved, 1631 unresolved (of 2854 total specs)
[resolve_imports] project_map 6.4ms, suffix_idx 1.4ms, suffix_resolve 19.4ms, total 27.1ms
[build_graphs] 651 files | maps 2.4ms, imports 27.2ms, calls+inherit 7.8ms, total 37.4ms | 1222 import, 9477 call, 11 inherit edges
sentrux gate — structural regression check

Quality:      4479 -> 4479
Coupling:     0.79 → 0.79
Cycles:       4 → 4
God files:    3 → 3

Distance from Main Sequence: 0.38

✓ No degradation detected

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.36508% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.65%. Comparing base (6632909) to head (e65cbb7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
factory/runners/_tmux_persist.py 79.36% 8 Missing and 5 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xukai92

xukai92 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

✅ Factory Review: KEEP

Verdict: KEEP
Reason: Experiment #36 KEEP: All QA gates passed. Health check +0.0074 (0.5112 → 0.5186). Code review CLEAN (7/7 categories). Adversarial QA PASS (10/10 criteria verified with evidence). Hardens tmux key submission with -H 0d hex bytes (fixes #1259) and adds structured JSON sentinel completion detection (fixes #1317).


Posted by Factory CEO

@xukai92
xukai92 marked this pull request as ready for review August 20, 2026 19:28
@osilkin98

Copy link
Copy Markdown
Collaborator

@ceo-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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:

  1. _tmux_send_enter() — hex 0d two-step Enter key approach
  2. _verify_post_send() — 15s verification after key submission
  3. JSON sentinel completion detection with backward compat
  4. _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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactory agent doesn't know when ceo is done refactory agent doesn't always press Enter

2 participants