feat(#124): backport CLI link_commit registration to triage (cherry-pick of 431e202 with §10.5.3 adaptations)

cli/_link_commit_runner.py

@@ -0,0 +1,37 @@
+"""Sync wrapper around handle_link_commit. Shared by branch-scan and
+link_commit CLI subcommands. Lazy-imports SurrealDB-touching modules
+so callers don't pay the import cost when no ledger is configured.
+
+Promoted from cli/branch_scan.py (#48) to a shared module under #124
+when the link_commit CLI subcommand was added.
+"""
+
+from __future__ import annotations
+
+import asyncio
+from pathlib import Path
+
+from contracts import LinkCommitResponse
+
+
+def invoke_link_commit(commit_hash: str = "HEAD") -> LinkCommitResponse | None:
+    """Drive the async ``handle_link_commit`` from sync context.
+
+    Returns ``None`` when:
+      - ``~/.bicameral/ledger.db`` does not exist (no configured ledger), OR
+      - the underlying handler raises (graceful skip — caller decides on
+        loud vs. silent failure semantics).
+    """
+    if not (Path.home() / ".bicameral" / "ledger.db").exists():
+        return None
+    from context import BicameralContext
+    from handlers.link_commit import handle_link_commit
+
+    async def _run() -> LinkCommitResponse:
+        ctx = BicameralContext.from_env()
+        return await handle_link_commit(ctx, commit_hash=commit_hash)
+
+    try:
+        return asyncio.run(_run())
+    except Exception:  # noqa: BLE001 — caller decides loud vs. silent
+        return None

cli/link_commit_cli.py

@@ -0,0 +1,31 @@
+"""link_commit CLI subcommand entry point (#124).
+
+Wraps the shared ``cli._link_commit_runner.invoke_link_commit`` for
+human-driven invocation. JSON-to-stdout by default; ``--quiet`` for
+hook scripts that pipe to /dev/null.
+
+Always exits 0 — the post-commit hook depends on this so commits are
+never blocked. Hook-side loudness (stderr) is handled in the installed
+shell script, not here.
+"""
+
+from __future__ import annotations
+
+import json
+
+from cli._link_commit_runner import invoke_link_commit
+
+
+def main(commit_hash: str = "HEAD", *, quiet: bool = False) -> int:
+    """Run link_commit against ``commit_hash`` (default HEAD).
+
+    Returns 0 on success, on no-ledger graceful skip, and on
+    handler-exception graceful skip — the runner already collapses
+    those cases to ``None``. Print JSON to stdout unless ``quiet``.
+    """
+    response = invoke_link_commit(commit_hash)
+    if response is None:
+        return 0
+    if not quiet:
+        print(json.dumps(response.model_dump(), default=str, indent=2))
+    return 0

server.py

@@ -28,6 +28,7 @@
 import asyncio
 import sys
 from argparse import ArgumentParser
+from typing import Any
 
 import mcp.server.stdio
 from mcp.server import Server
@@ -1129,22 +1130,31 @@ async def serve_stdio() -> None:
 
 
 def cli_main(argv: list[str] | None = None) -> int:
+    """Entry point — build parser, register subcommands via _register_subparsers, dispatch via _dispatch."""
     parser = ArgumentParser(description="Bicameral MCP server")
     subparsers = parser.add_subparsers(dest="command")
+    _register_subparsers(parser, subparsers)
+    args = parser.parse_args(argv)
+    return _dispatch(args)
+
+
+def _register_subparsers(parser: ArgumentParser, subparsers: Any) -> None:
+    """Wire all subparser definitions + top-level flags onto parser.
 
-    # config subcommand
+    triage-adapt: omits dev's branch-scan subparser and the setup
+    --with-push-hook flag — both are #48 prerequisites that aren't on
+    this branch. Keeps dev's link_commit subparser (the actual #124 fix)
+    and the helper-extraction shape (so test_hook_command_registration.py
+    can introspect registered subcommands).
+    """
     subparsers.add_parser(
         "config",
         help="interactive config editor — update mode, guided, and telemetry settings",
     )
-
-    # reset subcommand
     subparsers.add_parser(
         "reset",
         help="interactive ledger reset — wipes state with confirmation",
     )
-
-    # setup subcommand
     setup_parser = subparsers.add_parser(
         "setup",
         help="interactive setup — configure MCP client to use this server",
@@ -1161,7 +1171,21 @@ def cli_main(argv: list[str] | None = None) -> int:
         metavar="PATH",
         help="separate directory for .bicameral/ history storage (default: same as repo)",
     )
-
+    link_parser = subparsers.add_parser(
+        "link_commit",
+        help="hash-level sync — link the given commit (default HEAD) into the ledger (#124)",
+    )
+    link_parser.add_argument(
+        "commit_hash",
+        nargs="?",
+        default="HEAD",
+        help="commit hash to link (default: HEAD)",
+    )
+    link_parser.add_argument(
+        "--quiet",
+        action="store_true",
+        help="suppress JSON output (still exits 0 on success)",
+    )
     parser.add_argument(
         "--smoke-test",
         action="store_true",
@@ -1172,27 +1196,31 @@ def cli_main(argv: list[str] | None = None) -> int:
         action="version",
         version=f"%(prog)s {SERVER_VERSION}",
     )
-    args = parser.parse_args(argv)
 
+
+def _dispatch(args: Any) -> int:
+    """Route parsed args to the appropriate handler. Returns exit code."""
     if args.command == "config":
         from setup_wizard import run_config_wizard
         return run_config_wizard()
-
     if args.command == "reset":
         from setup_wizard import run_reset_wizard
         return run_reset_wizard()
-
     if args.command == "setup":
         from setup_wizard import run_setup
         return run_setup(args.repo_path, args.history_path)
-
+    # triage-adapt: link_commit dispatch — added per #124 backport without
+    # the broader _register_subparsers/_dispatch refactor or the branch-scan
+    # / --with-push-hook prerequisites
+    if args.command == "link_commit":
+        from cli.link_commit_cli import main as link_commit_main
+        return link_commit_main(args.commit_hash, quiet=args.quiet)
     if args.smoke_test:
         result = asyncio.run(run_smoke_test())
         print(f"{result['server_name']} {result['server_version']} smoke test passed")
         for tool_name in result["tool_names"]:
             print(tool_name)
         return 0
-
     asyncio.run(serve_stdio())
     return 0
 

setup_wizard.py

@@ -421,8 +421,14 @@ def _install_claude_hooks(repo_path: Path) -> bool:
 #!/bin/sh
 # Bicameral MCP — post-commit hook (installed by bicameral-mcp setup, Guided mode)
 # Syncs the decision ledger after every commit so drift status is current immediately.
-# Silent on failure; only runs when .bicameral/ exists.
-[ -d .bicameral ] && bicameral-mcp link_commit HEAD >/dev/null 2>&1 || true
+# Loud-but-non-blocking failure: any stderr from link_commit is captured to
+# ${HOME}/.bicameral/hook-errors.log and surfaced on stderr in the same commit.
+# The `>` redirection truncates the log file each run, so successful commits
+# auto-clear stale errors from prior failed runs. Always exits 0 — the commit
+# itself never blocks on a sync hook failure (#124).
+[ -d .bicameral ] && bicameral-mcp link_commit HEAD >/dev/null 2>"${HOME}/.bicameral/hook-errors.log"
+[ -s "${HOME}/.bicameral/hook-errors.log" ] && echo "Bicameral post-commit hook failed; see ${HOME}/.bicameral/hook-errors.log" >&2
+exit 0
 """
 
 

tests/test_hook_command_registration.py

@@ -0,0 +1,81 @@
+"""Issue #124 Phase 2 — hook command registration smoke tests.
+
+Walks every ``bicameral-mcp <subcommand>`` invocation in installed
+hook scripts and asserts each subcommand is registered as a subparser
+in ``server.cli_main``. Catches the original #124 bug at PR time:
+the post-commit hook called ``link_commit`` for months without
+``link_commit`` ever being a registered subcommand.
+
+These tests assume Phase 0a's ``_register_subparsers`` is the source
+of truth for registered commands — it builds the parser without
+running the dispatch.
+"""
+
+from __future__ import annotations
+
+import re
+from argparse import ArgumentParser
+
+# triage-adapt: dropped _GIT_PRE_PUSH_HOOK import + the pre-push test
+# below — _GIT_PRE_PUSH_HOOK is a #48 prerequisite (pre-push drift hook)
+# that doesn't exist on triage. Post-commit coverage is preserved (the
+# actual #124 regression).
+from server import _register_subparsers
+from setup_wizard import _GIT_POST_COMMIT_HOOK
+
+# Match `bicameral-mcp <subcommand>` where the subcommand is a
+# lower-snake-or-dash identifier. Anchors on the literal command
+# token to avoid matching e.g. comments that mention bicameral-mcp.
+_CMD_RE = re.compile(r"\bbicameral-mcp\s+([a-z][a-z0-9_-]+)\b")
+
+
+def _extract_bicameral_mcp_commands(hook_script: str) -> set[str]:
+    """Return the set of unique subcommand tokens invoked in the script."""
+    return set(_CMD_RE.findall(hook_script))
+
+
+def _registered_subcommands() -> set[str]:
+    """Build a fresh parser via _register_subparsers and return the
+    set of registered subparser names."""
+    parser = ArgumentParser()
+    subparsers = parser.add_subparsers(dest="command")
+    _register_subparsers(parser, subparsers)
+    return set(subparsers.choices.keys())
+
+
+def test_post_commit_hook_command_is_registered() -> None:
+    """The post-commit hook calls ``link_commit``; that subcommand
+    must be a registered subparser. THIS TEST WAS RED ON DEV
+    BEFORE #124 — the regression that the original bug report named."""
+    invoked = _extract_bicameral_mcp_commands(_GIT_POST_COMMIT_HOOK)
+    registered = _registered_subcommands()
+    missing = invoked - registered
+    assert not missing, (
+        f"Post-commit hook invokes {invoked} but only {registered} are "
+        f"registered. Missing: {missing}"
+    )
+
+
+# triage-adapt: dropped test_pre_push_hook_command_is_registered —
+# pre-push hook is from #48 (missing prerequisite on triage)
+
+
+def test_all_hook_commands_have_dispatch_branches() -> None:
+    """Every command referenced in any installed hook script must
+    appear in server._dispatch as an ``args.command == "..."``
+    branch — registered-but-not-dispatched would still pass the
+    register tests above but would silently no-op at runtime.
+
+    triage-adapt: scoped to _GIT_POST_COMMIT_HOOK only — pre-push
+    hook is from #48, not on triage."""
+    import inspect
+
+    from server import _dispatch
+
+    dispatch_src = inspect.getsource(_dispatch)
+    invoked = _extract_bicameral_mcp_commands(_GIT_POST_COMMIT_HOOK)
+    missing = {cmd for cmd in invoked if f'args.command == "{cmd}"' not in dispatch_src}
+    assert not missing, (
+        f"Hook scripts invoke {invoked} but _dispatch has branches for "
+        f"only {invoked - missing}. Missing: {missing}"
+    )

tests/test_link_commit_cli.py

@@ -0,0 +1,96 @@
+"""Issue #124 Phase 1 — link_commit CLI subcommand contract tests.
+
+Tests the CLI surface of ``cli.link_commit_cli.main`` in isolation:
+mocks the shared runner so no SurrealDB / no real git activity is
+required. Six tests cover argparse defaults, output shape, --quiet
+flag, and the two graceful-skip paths (no ledger, handler exception).
+"""
+
+from __future__ import annotations
+
+import json
+from unittest.mock import patch
+
+from contracts import LinkCommitResponse
+
+
+def _fake_response(commit_hash: str = "abc123") -> LinkCommitResponse:
+    """Minimal valid LinkCommitResponse for output-shape tests."""
+    return LinkCommitResponse(
+        commit_hash=commit_hash,
+        synced=True,
+        reason="new_commit",
+    )
+
+
+def test_default_commit_hash_is_HEAD() -> None:
+    """``main()`` with no positional arg passes ``HEAD`` to the runner."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = None
+        link_commit_cli.main()
+    mock.assert_called_once_with("HEAD")
+
+
+def test_explicit_commit_hash_passed_through() -> None:
+    """``main("abc1234")`` passes the explicit hash to the runner."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = None
+        link_commit_cli.main("abc1234")
+    mock.assert_called_once_with("abc1234")
+
+
+def test_json_output_on_success(capsys) -> None:
+    """A successful sync prints valid JSON with the response shape."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = _fake_response("deadbeef")
+        rc = link_commit_cli.main("deadbeef")
+    captured = capsys.readouterr()
+    assert rc == 0
+    payload = json.loads(captured.out)
+    assert payload["commit_hash"] == "deadbeef"
+    assert payload["synced"] is True
+    assert payload["reason"] == "new_commit"
+
+
+def test_quiet_flag_suppresses_output(capsys) -> None:
+    """``--quiet`` (quiet=True) emits no stdout but still exits 0."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = _fake_response()
+        rc = link_commit_cli.main("HEAD", quiet=True)
+    captured = capsys.readouterr()
+    assert rc == 0
+    assert captured.out == ""
+
+
+def test_no_ledger_returns_zero_silently(capsys) -> None:
+    """Runner returns None (no ledger) → main exits 0, no stdout."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = None
+        rc = link_commit_cli.main()
+    captured = capsys.readouterr()
+    assert rc == 0
+    assert captured.out == ""
+
+
+def test_handler_exception_returns_zero_silently(capsys) -> None:
+    """Runner swallows exceptions and returns None — main treats it
+    identically to no-ledger (exit 0, silent). The hook's
+    failure-loud semantics live in shell, not Python."""
+    from cli import link_commit_cli
+
+    with patch.object(link_commit_cli, "invoke_link_commit") as mock:
+        mock.return_value = None  # runner already converted exception → None
+        rc = link_commit_cli.main()
+    captured = capsys.readouterr()
+    assert rc == 0
+    assert captured.out == ""