feat(hooks): add hook system, CLI management, and worker runtime tests (F54-F56)

[wendeus0]

Summary

• F54 (Hook System): HookDispatcher with pre/post step hooks, pipeline integration, spec validation hooks, and e2e tests. 56 tests covering dispatcher, contracts, spec validation, pipeline integration, and e2e scenarios.
• F55 (Hook CLI Management): CLI commands for hooks list [--spec], hooks validate <handler>, and hooks status via Typer sub-app. 10 tests.
• F56 (Worker Runtime Tests): Dedicated tests for RuntimeWorker covering polling, lock acquisition, owner skip deduplication, exception handling, sleep behavior, and factory function. 10 tests (4 new + 6 pre-existing).

Technical Changes

New Files

• src/synapse_os/hooks.py — HookDispatcher, HookRejectedError, merge/load/dispatch logic
• src/synapse_os/cli/hooks.py — CLI commands for hook management
• tests/unit/test_hook_dispatcher.py — 23 unit tests for dispatcher
• tests/unit/test_hooks_cli.py — 10 unit tests for CLI commands
• tests/unit/test_spec_validator_hooks.py — 10 unit tests for spec validation hooks
• tests/unit/test_pipeline_hook_integration.py — 5 integration tests
• tests/integration/test_hook_system_e2e.py — 8 e2e tests
• tests/unit/test_worker_runtime.py — 4 new tests (10 total)

Modified Files

• src/synapse_os/config.py — Added hooks field to AppSettings
• src/synapse_os/pipeline.py — Integrated HookDispatcher into PipelineEngine
• src/synapse_os/runtime_contracts.py — Added hooks_active to PipelineContext
• src/synapse_os/specs/validator.py — Added hooks validation
• src/synapse_os/cli/app.py — Registered hooks CLI sub-app

How to Test

uv run pytest tests/ -v --tb=short  # 540 tests passing
uv run mypy src/synapse_os/         # clean
uv run ruff check src/synapse_os/   # 7 pre-existing, 0 from this PR

Risks

• Hook system adds execution paths to pipeline — mitigated by comprehensive tests (56 tests)
• CLI commands depend on AppSettings — tested with proper mocking patterns
• Worker tests use real SQLite — acceptable for integration-level tests

---

Summary by cubic

Adds a pluggable hook system, a hooks CLI, and worker runtime tests to improve pipeline flexibility and reliability. Also tightens hook validation/logging and preserves exception chains on hook rejections.

• New Features

  • F54: Hook system with pre_step, post_step, pre_state_transition, post_state_transition; merges AppSettings.hooks with SPEC frontmatter, supports hard_fail and supervisor_delegate, runs post hooks async, exposes active hooks in PipelineContext.
  • Pipeline integration: hooks wrap step execution and state transitions; SPEC validator accepts and validates hooks frontmatter; added AppSettings.hooks.
  • F55: CLI — hooks list [--spec], hooks validate <handler>, hooks status.
  • F56: Worker runtime tests for polling, lock acquisition, owner-skip dedupe, exception handling, idle sleep, and the factory builder.

• Bug Fixes

  • Ignore disabled global hooks; add stack traces to post-hook error logs; derive valid hook points from the HookConfig type in the SPEC validator.
  • hooks validate now ensures the imported target is callable; removed dead exception handling.
  • Preserve exception chaining with “from exc” when a hook rejects a step.
  • Tests: replace sleeps with threading.Event, improve worker exception testing, extract shared SPEC_TEXT, remove unused vars, and apply ruff/CI formatting fixes (including pipeline hook integration).

^{Written for commit eece88d. Summary will update on new commits.}

[cubic-dev-ai]

9 issues found across 18 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/unit/test_worker_runtime.py">

<violation number="1" location="tests/unit/test_worker_runtime.py:366">
P2: This test doesn't actually exercise exception handling. No exception is injected — the run is created with a valid spec and will complete normally, so the assertion `status in ("completed", "failed")` always passes via the `"completed"` branch. To test graceful exception handling, mock `runner.run_existing` to raise and then assert the run is marked `"failed"` with an appropriate error event.</violation>
</file>

<file name="src/synapse_os/cli/hooks.py">

<violation number="1" location="src/synapse_os/cli/hooks.py:86">
P3: The `except ValueError` block is unreachable. The earlier `if "." not in handler` guard already exits when there's no dot, so `rsplit(".", 1)` on a string with a dot will always succeed. Remove the redundant try/except and call `rsplit` directly.</violation>

<violation number="2" location="src/synapse_os/cli/hooks.py:107">
P2: `func.__name__` will raise an unhandled `AttributeError` if the resolved attribute is not a function (e.g., a module-level constant or string). Add a callable check before accessing `__name__`.</violation>
</file>

<file name="src/synapse_os/pipeline.py">

<violation number="1" location="src/synapse_os/pipeline.py:479">
P2: Exception chain is lost: `HookRejectedError` is caught and re-raised as `RetryableStepError` without `from exc`. The original rejection reason (included in the `HookRejectedError` message) is discarded, making hook rejections hard to diagnose.</violation>
</file>

<file name="features/F55-hook-cli-management/SPEC.md">

<violation number="1" location="features/F55-hook-cli-management/SPEC.md:62">
P3: Artifact paths in the SPEC do not match the actual implementation. `src/synapse_os/cli/commands/hooks.py` should be `src/synapse_os/cli/hooks.py` (no `commands/` subdirectory exists), `src/synapse_os/cli/__init__.py` should be `src/synapse_os/cli/app.py`, and `tests/integration/test_hooks_cli_e2e.py` was never created. Keeping the SPEC aligned with reality prevents confusion for future contributors.</violation>
</file>

<file name="src/synapse_os/specs/validator.py">

<violation number="1" location="src/synapse_os/specs/validator.py:15">
P2: `VALID_HOOK_POINTS` duplicates the `Literal` values already defined on `HookConfig.point`. If a hook point is added to or removed from the `Literal` type, this set will silently drift. Derive it from the type annotation instead using `typing.get_args`.</violation>
</file>

<file name="tests/integration/test_hook_system_e2e.py">

<violation number="1" location="tests/integration/test_hook_system_e2e.py:110">
P2: Project convention (CLAUDE.md) prohibits bare `time.sleep` in tests — use time mocks instead. The sleep here makes the test depend on wall-clock timing and slows it down. Consider replacing with a `threading.Event` or similar synchronization primitive that the handler sets after appending to `results`, so the test can wait deterministically without a real sleep.</violation>
</file>

<file name="src/synapse_os/hooks.py">

<violation number="1" location="src/synapse_os/hooks.py:52">
P2: Global hooks with `enabled=False` are never filtered out. The `disabled` set is built only from `spec_hooks`, so a global hook explicitly marked `enabled: false` still passes through to the result.</violation>

<violation number="2" location="src/synapse_os/hooks.py:88">
P2: Post-hook exception is logged without the actual error details. Pass `exc_info=True` so the traceback appears in logs; otherwise post-hook failures are effectively impossible to debug.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[cubic-dev-ai]

2 issues found across 19 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/.aignt-os/runtime/runtime-state.json">

<violation number="1" location="scripts/.aignt-os/runtime/runtime-state.json:1">
P1: Runtime state file should not be committed to version control. This contains machine-specific ephemeral data (PID, start time, process identity) that is meaningless—and potentially confusing—for other developers or CI. Add `scripts/.aignt-os/runtime/runtime-state.json` (or the entire `scripts/.aignt-os/runtime/` directory) to `.gitignore` and remove this file from tracking.</violation>
</file>

<file name=".github/workflows/security-review.yml">

<violation number="1" location=".github/workflows/security-review.yml:19">
P1: Pin `anthropics/claude-code-security-review` to a specific commit SHA instead of `@main`. A mutable branch ref means a compromise or force-push to the upstream repo would silently execute arbitrary code with your `CLAUDE_API_KEY` secret and PR write access.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[cubic-dev-ai]

P1: Runtime state file should not be committed to version control. This contains machine-specific ephemeral data (PID, start time, process identity) that is meaningless—and potentially confusing—for other developers or CI. Add scripts/.aignt-os/runtime/runtime-state.json (or the entire scripts/.aignt-os/runtime/ directory) to .gitignore and remove this file from tracking.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/.aignt-os/runtime/runtime-state.json, line 1:

<comment>Runtime state file should not be committed to version control. This contains machine-specific ephemeral data (PID, start time, process identity) that is meaningless—and potentially confusing—for other developers or CI. Add `scripts/.aignt-os/runtime/runtime-state.json` (or the entire `scripts/.aignt-os/runtime/` directory) to `.gitignore` and remove this file from tracking.</comment>

<file context>
@@ -0,0 +1 @@
+{"status": "running", "pid": 1010871, "started_at": "2026-03-13T04:35:33.348884+00:00", "process_identity": "33c64d60fdefb7e4591224553831c580"}
\ No newline at end of file
</file context>

[cubic-dev-ai]

P1: Pin anthropics/claude-code-security-review to a specific commit SHA instead of @main. A mutable branch ref means a compromise or force-push to the upstream repo would silently execute arbitrary code with your CLAUDE_API_KEY secret and PR write access.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/security-review.yml, line 19:

<comment>Pin `anthropics/claude-code-security-review` to a specific commit SHA instead of `@main`. A mutable branch ref means a compromise or force-push to the upstream repo would silently execute arbitrary code with your `CLAUDE_API_KEY` secret and PR write access.</comment>

<file context>
@@ -0,0 +1,23 @@
+          ref: ${{ github.event.pull_request.head.sha || github.sha }}
+          fetch-depth: 2
+
+      - uses: anthropics/claude-code-security-review@main
+        with:
+          comment-pr: true
</file context>