fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations (retro t_4ba269e5) - #37
Merged
Merged
Conversation
…ax_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: ## Fix 1: execute_code single-tool gate (tools/code_execution_tool.py) - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py ## Fix 2: Dispatcher missing-heartbeat enforcement (hermes_cli/kanban_db.py) - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py ## Fix 3: Task-scoped max_iterations field (hermes_cli/kanban_db.py, tools/kanban_tools.py) - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py ## Fix 4: Skill updates - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat)
📝 WalkthroughWalkthroughThis PR adds per-task LLM iteration budgets (max_iterations) persisted in the kanban DB and injected into spawned workers, a missing-heartbeat enforcer that warns and auto-blocks stalled runs, an AST-based single-tool-call gate in execute_code that rejects trivial single-tool scripts, plus related schema, dispatch wiring, and tests. ChangesPer-Task Iteration Budgets and Missing Heartbeat Enforcement
Single Tool Call Gate and Code Execution Refactoring
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
2 |
First entries
tests/hermes_cli/test_kanban_worker_discipline.py:32: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/tools/test_execute_code_gate.py:15: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
✅ Fixed issues: none
Unchanged: 5049 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Owner
|
auto-review: changes requested. Failing checks
Root causeThe execute_code gate in code = (
'from hermes_tools import terminal\n'
"r = terminal('echo x')\n"
'print(r.get("output", "MISSING"))\n'
)
result = self._run(code, mode='strict') # mode='strict' not reachable — gate fires first
self.assertEqual(result['status'], 'success') # KeyError: gate returned tool_errorThe gate returns Fix required: the gate must not fire inside these tests. Options:
Also: AC4 unmet — the live skill files ( Passing checks
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tools/code_execution_tool.py (1)
2028-2031: ⚡ Quick winMake the Hermes-home example profile-aware.
This schema text hardcodes
~/.hermes/.env, so the model gets the wrong path for non-default profiles. Build the example fromdisplay_hermes_home()instead of embedding the default location. As per coding guidelines,In tool schemas that mention file paths (e.g., default output directories), use display_hermes_home() to make them profile-aware.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/code_execution_tool.py` around lines 2028 - 2031, The cwd_note currently hardcodes "~/.hermes/.env"; update it to be profile-aware by building the example path from display_hermes_home() instead of the literal string. Locate the cwd_note definition and replace the embedded "~/.hermes/.env" with a constructed example using display_hermes_home() (e.g., display_hermes_home() + "/.env" or os.path.join(display_hermes_home(), ".env")) so the message reflects the active profile's Hermes home; keep the rest of the explanatory text unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hermes_cli/kanban_db.py`:
- Around line 8468-8473: _in _default_spawn() you currently only set
HERMES_MAX_ITERATIONS when task.max_iterations is not None, but since env begins
as dict(os.environ) an existing dispatcher-level HERMES_MAX_ITERATIONS can leak
into tasks that don't override it; update _default_spawn() to explicitly
remove/clear HERMES_MAX_ITERATIONS from env when task.max_iterations is None
(e.g. env.pop("HERMES_MAX_ITERATIONS", None)) so only tasks that explicitly set
task.max_iterations inject the variable and others fall back to profile/default
agent.max_turns.
- Around line 6361-6391: The idempotence check must be moved inside the same
write_txn that inserts the warning to avoid a race: wrap the SELECT that looks
for an existing "missing_heartbeat_warning" and the call to _append_event inside
write_txn(conn) (i.e., perform conn.execute("SELECT id ...", (tid,
run_id)).fetchone() inside the transaction) and only call _append_event and then
append to warned if that SELECT returns no row; remove the pre-transaction
SELECT so the check+insert are atomic.
In `@tools/code_execution_tool.py`:
- Around line 1150-1174: _count_hermes_tool_calls currently treats any bare name
in _HERMES_TOOL_NAMES or any attribute with those names as Hermes calls; change
it to first walk the AST to collect import provenance (build sets like
hermes_module_aliases for "import hermes_tools as X" and hermes_imported_names
for "from hermes_tools import name" and a flag for "from hermes_tools import
*"), then only increment when a Call node's function is a Name whose id is in
hermes_imported_names or (import-star flag is set and id in _HERMES_TOOL_NAMES),
or when the function is an Attribute whose root object is a Name whose id is in
hermes_module_aliases or is "hermes_tools". Use helper logic to resolve the root
Name for nested attributes and update _count_hermes_tool_calls to consult these
provenance sets before counting.
---
Nitpick comments:
In `@tools/code_execution_tool.py`:
- Around line 2028-2031: The cwd_note currently hardcodes "~/.hermes/.env";
update it to be profile-aware by building the example path from
display_hermes_home() instead of the literal string. Locate the cwd_note
definition and replace the embedded "~/.hermes/.env" with a constructed example
using display_hermes_home() (e.g., display_hermes_home() + "/.env" or
os.path.join(display_hermes_home(), ".env")) so the message reflects the active
profile's Hermes home; keep the rest of the explanatory text unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20fadb43-33d3-4c30-8657-6a9ba34cdc83
📒 Files selected for processing (5)
hermes_cli/kanban_db.pytests/hermes_cli/test_kanban_worker_discipline.pytests/tools/test_execute_code_gate.pytools/code_execution_tool.pytools/kanban_tools.py
Comment on lines
+6361
to
+6391
| if warn_after_seconds > 0 and elapsed >= warn_after_seconds: | ||
| # Idempotent: skip if a warning event already exists for this run. | ||
| existing = conn.execute( | ||
| "SELECT id FROM task_events " | ||
| "WHERE task_id = ? AND kind = 'missing_heartbeat_warning' " | ||
| " AND (run_id = ? OR run_id IS NULL) " | ||
| "ORDER BY id DESC LIMIT 1", | ||
| (tid, run_id), | ||
| ).fetchone() | ||
| if existing: | ||
| continue | ||
|
|
||
| with write_txn(conn): | ||
| _append_event( | ||
| conn, | ||
| tid, | ||
| "missing_heartbeat_warning", | ||
| { | ||
| "elapsed_seconds": elapsed, | ||
| "threshold_seconds": warn_after_seconds, | ||
| "action": "warned", | ||
| "message": ( | ||
| f"You have been running {elapsed // 60} minutes " | ||
| f"without a heartbeat. Call kanban_heartbeat(note=...) " | ||
| f"now or block the task. At {block_after_seconds // 60} " | ||
| f"minutes the dispatcher will auto-block this task." | ||
| ), | ||
| }, | ||
| run_id=run_id, | ||
| ) | ||
| warned.append(tid) |
There was a problem hiding this comment.
Make the warning emission atomic per run.
The idempotence check happens before write_txn(), so two dispatcher processes can both observe “no warning yet” and each append a missing_heartbeat_warning. That breaks the “one warning per run” contract.
Suggested fix
if warn_after_seconds > 0 and elapsed >= warn_after_seconds:
- # Idempotent: skip if a warning event already exists for this run.
- existing = conn.execute(
- "SELECT id FROM task_events "
- "WHERE task_id = ? AND kind = 'missing_heartbeat_warning' "
- " AND (run_id = ? OR run_id IS NULL) "
- "ORDER BY id DESC LIMIT 1",
- (tid, run_id),
- ).fetchone()
- if existing:
- continue
-
with write_txn(conn):
+ existing = conn.execute(
+ "SELECT id FROM task_events "
+ "WHERE task_id = ? AND kind = 'missing_heartbeat_warning' "
+ " AND (run_id = ? OR run_id IS NULL) "
+ "ORDER BY id DESC LIMIT 1",
+ (tid, run_id),
+ ).fetchone()
+ if existing:
+ continue
_append_event(
conn,
tid,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hermes_cli/kanban_db.py` around lines 6361 - 6391, The idempotence check must
be moved inside the same write_txn that inserts the warning to avoid a race:
wrap the SELECT that looks for an existing "missing_heartbeat_warning" and the
call to _append_event inside write_txn(conn) (i.e., perform conn.execute("SELECT
id ...", (tid, run_id)).fetchone() inside the transaction) and only call
_append_event and then append to warned if that SELECT returns no row; remove
the pre-transaction SELECT so the check+insert are atomic.
Comment on lines
+8468
to
+8473
| # Per-task iteration budget: inject as HERMES_MAX_ITERATIONS so the | ||
| # spawned worker's AIAgent picks it up (cli.py reads this env var). | ||
| # Only override when task.max_iterations is explicitly set — otherwise | ||
| # let the profile's config.yaml agent.max_turns (or the env default) win. | ||
| if task.max_iterations is not None: | ||
| env["HERMES_MAX_ITERATIONS"] = str(int(task.max_iterations)) |
There was a problem hiding this comment.
Clear inherited HERMES_MAX_ITERATIONS when there is no task override.
_default_spawn() starts from env = dict(os.environ). If the dispatcher itself has HERMES_MAX_ITERATIONS set, tasks with task.max_iterations is None will still inherit that value instead of falling back to the profile/default config.
Suggested fix
if task.max_iterations is not None:
env["HERMES_MAX_ITERATIONS"] = str(int(task.max_iterations))
+ else:
+ env.pop("HERMES_MAX_ITERATIONS", None)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hermes_cli/kanban_db.py` around lines 8468 - 8473, _in _default_spawn() you
currently only set HERMES_MAX_ITERATIONS when task.max_iterations is not None,
but since env begins as dict(os.environ) an existing dispatcher-level
HERMES_MAX_ITERATIONS can leak into tasks that don't override it; update
_default_spawn() to explicitly remove/clear HERMES_MAX_ITERATIONS from env when
task.max_iterations is None (e.g. env.pop("HERMES_MAX_ITERATIONS", None)) so
only tasks that explicitly set task.max_iterations inject the variable and
others fall back to profile/default agent.max_turns.
Comment on lines
+1150
to
+1174
| def _count_hermes_tool_calls(code: str) -> int: | ||
| """Return the number of distinct hermes_tools function calls in *code*. | ||
|
|
||
| Counts top-level calls whose name is in ``_HERMES_TOOL_NAMES``, e.g. | ||
| ``terminal("ls")`` or ``read_file(path)``. Also handles the imported | ||
| form (``from hermes_tools import terminal; terminal(...)``) and the | ||
| module-attribute form (``hermes_tools.terminal(...)``). | ||
|
|
||
| Raises ``SyntaxError`` if ``code`` is not valid Python. | ||
| """ | ||
| import ast as _ast | ||
|
|
||
| tree = _ast.parse(code) | ||
| count = 0 | ||
| for node in _ast.walk(tree): | ||
| if not isinstance(node, _ast.Call): | ||
| continue | ||
| func = node.func | ||
| # Direct name call: terminal(...), read_file(...), etc. | ||
| if isinstance(func, _ast.Name) and func.id in _HERMES_TOOL_NAMES: | ||
| count += 1 | ||
| # Attribute call: hermes_tools.terminal(...), etc. | ||
| elif isinstance(func, _ast.Attribute) and func.attr in _HERMES_TOOL_NAMES: | ||
| count += 1 | ||
| return count |
There was a problem hiding this comment.
Resolve Hermes tool calls by import provenance.
Line 1169 treats any bare patch(...)/read_file(...) call as a Hermes tool call, and Line 1172 does the same for any obj.terminal(...) attribute call. That lets _check_single_tool_call() reject valid scripts that never imported hermes_tools at all, e.g. from unittest.mock import patch. Count only names proven to come from hermes_tools or one of its import aliases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/code_execution_tool.py` around lines 1150 - 1174,
_count_hermes_tool_calls currently treats any bare name in _HERMES_TOOL_NAMES or
any attribute with those names as Hermes calls; change it to first walk the AST
to collect import provenance (build sets like hermes_module_aliases for "import
hermes_tools as X" and hermes_imported_names for "from hermes_tools import name"
and a flag for "from hermes_tools import *"), then only increment when a Call
node's function is a Name whose id is in hermes_imported_names or (import-star
flag is set and id in _HERMES_TOOL_NAMES), or when the function is an Attribute
whose root object is a Name whose id is in hermes_module_aliases or is
"hermes_tools". Use helper logic to resolve the root Name for nested attributes
and update _count_hermes_tool_calls to consult these provenance sets before
counting.
… execute_code gate
The execute_code gate rejects scripts with exactly 1 hermes tool call and no
non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing
tests were legitimate but structurally matched the rejected pattern:
tests/tools/test_code_execution.py:
test_single_tool_call — 1 terminal call → 2 terminal calls
test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls
test_web_search_tool — 1 web_search call → 2 web_search calls
tests/tools/test_code_execution_modes.py:
test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal
test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal
Each test is testing tool dispatch, import paths, or mode CWD behavior — not
the number of tool calls. Two calls make the gate pass while leaving the
original assertion intact (import behavior, dispatch result, excluded-tool
error, etc.).
Owner
|
auto-review: changes requested.
Rules that passed: U1, U2, U3, U5 (UNSTABLE ok), C2, C3, C4, C5, C6 N/A, U6 N/A. 391/391 tests pass locally. Lint clean. Worker identity correct on both commits. |
Owner
|
auto-review: approved, awaiting human merge + kanban_approve. Matrix checks (U1–U5, C1–C5):
|
… + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/hermes_cli/test_kanban_worker_discipline.py`:
- Around line 92-122: In
test_kanban_create_without_isolation_fixture_uses_tmp_path replace the hardcoded
live_kanban_db = os.path.expanduser("~/.hermes/kanban.db") with a path built
from get_hermes_home() from hermes_constants (e.g. join get_hermes_home() with
"kanban.db" or Path(get_hermes_home()) / "kanban.db") so the test uses the
profile-aware Hermes home; update the live_kanban_db reference accordingly and
keep the rest of the assertions (which compare kb.kanban_db_path()) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4dc81c7-ae35-49f2-b98d-5c7b757b0af3
📒 Files selected for processing (3)
tests/hermes_cli/test_kanban_worker_discipline.pytests/tools/test_code_execution.pytests/tools/test_code_execution_modes.py
Comment on lines
+92
to
+122
| def test_kanban_create_without_isolation_fixture_uses_tmp_path(tmp_path, monkeypatch): | ||
| """Regression: kanban_create must NOT reach the live kanban.db. | ||
|
|
||
| This test simulates a 'naked' kanban_db call made without the | ||
| isolation fixture — we ensure the global conftest already pins | ||
| HERMES_KANBAN_HOME (or HERMES_HOME) to a tmp_path before any | ||
| kanban_db operation can reach the live ~/.hermes/kanban.db. | ||
|
|
||
| If the global conftest's _hermetic_environment fixture is working, | ||
| HERMES_KANBAN_DB and HERMES_KANBAN_HOME are already cleared and | ||
| HERMES_HOME points to a temp dir. We verify that creating a task | ||
| from this baseline state writes to the per-test tmpdir, NOT the | ||
| real kanban.db. | ||
| """ | ||
| live_kanban_db = os.path.expanduser("~/.hermes/kanban.db") | ||
|
|
||
| # If we're running as a dispatched worker, HERMES_KANBAN_DB is set. | ||
| # The global conftest _hermetic_environment should have cleared it. | ||
| # Verify it's gone: | ||
| assert os.environ.get("HERMES_KANBAN_DB", "") == "", ( | ||
| "HERMES_KANBAN_DB is set — global conftest _hermetic_environment " | ||
| "should have cleared it. Test environment is not hermetic." | ||
| ) | ||
|
|
||
| # kanban_db_path() must NOT resolve to the live path. | ||
| resolved_path = str(kb.kanban_db_path().resolve()) | ||
| assert resolved_path != os.path.abspath(live_kanban_db), ( | ||
| f"kanban_db_path() resolved to the live DB at {live_kanban_db}. " | ||
| "HERMES_HOME is not pointing to a tmpdir. " | ||
| "The global conftest _hermetic_environment fixture is broken." | ||
| ) |
There was a problem hiding this comment.
Use get_hermes_home() instead of hardcoding the path.
Line 106 hardcodes ~/.hermes/kanban.db, which violates the coding guideline: "never hardcode ~/.hermes/ paths in tests". Use get_hermes_home() from hermes_constants to construct the live kanban DB path dynamically.
🛠️ Proposed fix
+from hermes_constants import get_hermes_home
+
def test_kanban_create_without_isolation_fixture_uses_tmp_path(tmp_path, monkeypatch):
"""Regression: kanban_create must NOT reach the live kanban.db.
...
"""
- live_kanban_db = os.path.expanduser("~/.hermes/kanban.db")
+ live_kanban_db = str(get_hermes_home() / "kanban.db")As per coding guidelines, use get_hermes_home() from hermes_constants for all code paths that reference the Hermes home directory to ensure profile-aware behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_kanban_create_without_isolation_fixture_uses_tmp_path(tmp_path, monkeypatch): | |
| """Regression: kanban_create must NOT reach the live kanban.db. | |
| This test simulates a 'naked' kanban_db call made without the | |
| isolation fixture — we ensure the global conftest already pins | |
| HERMES_KANBAN_HOME (or HERMES_HOME) to a tmp_path before any | |
| kanban_db operation can reach the live ~/.hermes/kanban.db. | |
| If the global conftest's _hermetic_environment fixture is working, | |
| HERMES_KANBAN_DB and HERMES_KANBAN_HOME are already cleared and | |
| HERMES_HOME points to a temp dir. We verify that creating a task | |
| from this baseline state writes to the per-test tmpdir, NOT the | |
| real kanban.db. | |
| """ | |
| live_kanban_db = os.path.expanduser("~/.hermes/kanban.db") | |
| # If we're running as a dispatched worker, HERMES_KANBAN_DB is set. | |
| # The global conftest _hermetic_environment should have cleared it. | |
| # Verify it's gone: | |
| assert os.environ.get("HERMES_KANBAN_DB", "") == "", ( | |
| "HERMES_KANBAN_DB is set — global conftest _hermetic_environment " | |
| "should have cleared it. Test environment is not hermetic." | |
| ) | |
| # kanban_db_path() must NOT resolve to the live path. | |
| resolved_path = str(kb.kanban_db_path().resolve()) | |
| assert resolved_path != os.path.abspath(live_kanban_db), ( | |
| f"kanban_db_path() resolved to the live DB at {live_kanban_db}. " | |
| "HERMES_HOME is not pointing to a tmpdir. " | |
| "The global conftest _hermetic_environment fixture is broken." | |
| ) | |
| from hermes_constants import get_hermes_home | |
| def test_kanban_create_without_isolation_fixture_uses_tmp_path(tmp_path, monkeypatch): | |
| """Regression: kanban_create must NOT reach the live kanban.db. | |
| This test simulates a 'naked' kanban_db call made without the | |
| isolation fixture — we ensure the global conftest already pins | |
| HERMES_KANBAN_HOME (or HERMES_HOME) to a tmp_path before any | |
| kanban_db operation can reach the live ~/.hermes/kanban.db. | |
| If the global conftest's _hermetic_environment fixture is working, | |
| HERMES_KANBAN_DB and HERMES_KANBAN_HOME are already cleared and | |
| HERMES_HOME points to a temp dir. We verify that creating a task | |
| from this baseline state writes to the per-test tmpdir, NOT the | |
| real kanban.db. | |
| """ | |
| live_kanban_db = str(get_hermes_home() / "kanban.db") | |
| # If we're running as a dispatched worker, HERMES_KANBAN_DB is set. | |
| # The global conftest _hermetic_environment should have cleared it. | |
| # Verify it's gone: | |
| assert os.environ.get("HERMES_KANBAN_DB", "") == "", ( | |
| "HERMES_KANBAN_DB is set — global conftest _hermetic_environment " | |
| "should have cleared it. Test environment is not hermetic." | |
| ) | |
| # kanban_db_path() must NOT resolve to the live path. | |
| resolved_path = str(kb.kanban_db_path().resolve()) | |
| assert resolved_path != os.path.abspath(live_kanban_db), ( | |
| f"kanban_db_path() resolved to the live DB at {live_kanban_db}. " | |
| "HERMES_HOME is not pointing to a tmpdir. " | |
| "The global conftest _hermetic_environment fixture is broken." | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/hermes_cli/test_kanban_worker_discipline.py` around lines 92 - 122, In
test_kanban_create_without_isolation_fixture_uses_tmp_path replace the hardcoded
live_kanban_db = os.path.expanduser("~/.hermes/kanban.db") with a path built
from get_hermes_home() from hermes_constants (e.g. join get_hermes_home() with
"kanban.db" or Path(get_hermes_home()) / "kanban.db") so the test uses the
profile-aware Hermes home; update the live_kanban_db reference accordingly and
keep the rest of the assertions (which compare kb.kanban_db_path()) unchanged.
Owner
|
auto-review (run 475): approved. Matrix checks (U1–U5, C1–C5)
Code-quality judgment (role-reviewer)
Awaiting human merge + kanban_approve. |
sahilm-ti
pushed a commit
that referenced
this pull request
May 29, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: ## Fix 1: execute_code single-tool gate (tools/code_execution_tool.py) - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py ## Fix 2: Dispatcher missing-heartbeat enforcement (hermes_cli/kanban_db.py) - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py ## Fix 3: Task-scoped max_iterations field (hermes_cli/kanban_db.py, tools/kanban_tools.py) - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py ## Fix 4: Skill updates - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 3, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 5, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 15, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 17, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 22, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 3, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 9, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 10, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 11, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 13, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 15, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 17, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 21, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 23, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 28, 2026
…ax_iterations (retro t_4ba269e5) (#37) * fix(worker-discipline): execute_code gate + heartbeat enforcement + max_iterations field Retro from t_4ba269e5: that run burned $9.26 and 42 minutes without shipping. Three orthogonal failures are fixed here structurally: - Adds _check_single_tool_call(), _count_hermes_tool_calls(), _has_nontrivial_logic() - Rejects scripts that make exactly one hermes tool call with no loops/comprehensions/ regex/JSON processing. Returns a guidance message naming the tool to call directly. - 2× cost amplifier pattern (120/154 tool calls in t_4ba269e5) is now impossible. - Gate is intentionally lenient: passes 0 calls, 2+ calls, or 1 call + real logic. - 29 unit tests in tests/tools/test_execute_code_gate.py - Adds enforce_missing_heartbeat() function: 15-min soft warning event, 30-min hard block. - Targets workers that have NEVER sent a heartbeat (distinct from detect_stuck_workers which fires only after the first heartbeat goes stale). - Adds missing_heartbeat_warned/blocked fields to DispatchResult. - Wired into dispatch_once() (best-effort, never breaks dispatch). - Adds MISSING_HB_WARN_SECONDS / MISSING_HB_BLOCK_SECONDS constants. - 6 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - Adds max_iterations to Task dataclass, DB schema, migration, create_task(). - _default_spawn injects HERMES_MAX_ITERATIONS=N when task.max_iterations is set. - kanban_create tool exposes max_iterations parameter with guidance in schema. - End-to-end: schema → DB column → dispatcher env injection → worker reads at startup. - 5 unit tests in tests/hermes_cli/test_kanban_worker_discipline.py - kanban-worker skill: explicit execute_code anti-pattern section with good/bad examples - kanban-orchestrator skill: max_iterations sizing guide with card-type table Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) * fix(tests): update 5 single-tool test scripts to use 2 calls — bypass execute_code gate The execute_code gate rejects scripts with exactly 1 hermes tool call and no non-trivial processing logic (the t_4ba269e5 anti-pattern). Five existing tests were legitimate but structurally matched the rejected pattern: tests/tools/test_code_execution.py: test_single_tool_call — 1 terminal call → 2 terminal calls test_excluded_tool_returns_error — 1 terminal call → 2 terminal calls test_web_search_tool — 1 web_search call → 2 web_search calls tests/tools/test_code_execution_modes.py: test_project_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal test_strict_mode_can_still_import_hermes_tools — 1 terminal → 2 terminal Each test is testing tool dispatch, import paths, or mode CWD behavior — not the number of tool calls. Two calls make the gate pass while leaving the original assertion intact (import behavior, dispatch result, excluded-tool error, etc.). * fix(tests): harden kanban_home fixture isolation — HERMES_KANBAN_HOME + live-DB regression guard The previous kanban_home fixture only set HERMES_HOME, but kanban_db.kanban_home() resolves HERMES_KANBAN_HOME first. A dispatcher-spawned worker running pytest while HERMES_KANBAN_DB (or HERMES_KANBAN_HOME) is set in its environment would write fixture tasks into the live ~/.hermes/kanban.db instead of the tmp_path. This is exactly what happened: orphan tasks leaked during this PR's test runs. Changes: - Fixture now sets HERMES_KANBAN_HOME (highest-priority override) instead of HERMES_HOME - Fixture now clears HERMES_KANBAN_DB and HERMES_KANBAN_BOARD to prevent dispatcher pins - Fixture now clears kb._INITIALIZED_PATHS module cache to prevent cross-test leakage - Fixture yields (not returns) so cleanup runs after each test - Fixture has a hard assertion: kanban_db_path() must resolve inside tmp_path (fails immediately if isolation is broken — not silently after DB writes) - Added test_kanban_create_without_isolation_fixture_uses_tmp_path: fail-loud regression guard that asserts HERMES_KANBAN_DB is cleared by the global conftest and kanban_db_path() does not resolve to the live DB - Renamed title 'e2e-max-iter' → 'dispatch-max-iter' (cosmetic, avoids 'e2e' prefix that confused the orphan-scan query in the rejection) - Added module-level docstring explaining the TEST ISOLATION RULE Also updates test-isolation.md reference in kanban-worker skill to document the correct fixture pattern (HERMES_KANBAN_HOME, not HERMES_HOME) and the resolution order: HERMES_KANBAN_DB > HERMES_KANBAN_HOME > get_default_hermes_root(). Live kanban.db scan: 0 orphan rows (the 3 previously-found orphans were deleted in the rejection review; no new ones created by this run). Related: t_80581b0d retro of t_4ba269e5 (120/154 execute_code, 42min no heartbeat) --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
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
2 participants
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.
Motivation (retro from t_4ba269e5)
t_4ba269e5 (Ledger Bot missing elapsed timer) burned $9.26, 22M cache-read tokens, 42 minutes without shipping. Post-mortem revealed three compounding failures:
execute_codewrapping a singleterminal()command — a 2× cost amplifierkanban_heartbeatcalls over 42 minutes — zero observability, no chance to interveneAll three are now structurally impossible.
Fix 1:
execute_codepre-execution gate (tools/code_execution_tool.py)Adds three helpers + gate check at the top of
execute_code():_count_hermes_tool_calls(code)— AST-parses the script and counts hermes tool calls_has_nontrivial_logic(code)— detects loops, comprehensions, try/except, regex/JSON processing_check_single_tool_call(code)— returns a guidance message when the gate fires, None otherwiseGate rule: reject scripts that make exactly 1 hermes tool call with no processing logic.
Gate is intentionally lenient — passes:
29 unit tests in
tests/tools/test_execute_code_gate.pycovering all acceptance criteria from the task body.Fix 2: Dispatcher missing-heartbeat enforcement (
hermes_cli/kanban_db.py)Adds
enforce_missing_heartbeat()— targets workers that have never sent a heartbeat (different fromdetect_stuck_workerswhich fires only after the first heartbeat goes stale):missing_heartbeat_warningevent on the task. Idempotent (one warning per run).block_task()with a clear protocol-violation reason + SIGTERMs the worker if host-local.Adds
MISSING_HB_WARN_SECONDS/MISSING_HB_BLOCK_SECONDSconstants. Addsmissing_heartbeat_warned/missing_heartbeat_blockedtoDispatchResult. Wired intodispatch_once()in a try/except (never breaks dispatch on enforcer fault).6 unit tests in
tests/hermes_cli/test_kanban_worker_discipline.py.Fix 3: Task-scoped
max_iterations(hermes_cli/kanban_db.py,tools/kanban_tools.py)Taskdataclass: newmax_iterations: Optional[int]fieldmax_iterations INTEGERcolumncreate_task(): acceptsmax_iterations=Nparameter_default_spawn(): injectsHERMES_MAX_ITERATIONS=Ninto worker env when non-NULLkanban_createtool schema: newmax_iterationsparameter with docstring5 unit tests covering schema→DB→dispatch_once E2E.
Fix 4: Skill updates
execute_code anti-patternsection with good/bad code examples and gate explanationWhen to set max_iterations on a cardsection with sizing guide tableTest summary
All 248 existing
test_kanban_db.pytests pass unchanged.Fixes: t_80581b0d
Summary by CodeRabbit
New Features
Tests