Skip to content

Cover the agent-callable tools, the tunnel supervisor and the operator commands - #5

Draft
garethx wants to merge 2 commits into
mainfrom
tests/tools-coverage
Draft

Cover the agent-callable tools, the tunnel supervisor and the operator commands#5
garethx wants to merge 2 commits into
mainfrom
tests/tools-coverage

Conversation

@garethx

@garethx garethx commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Closes #2.

Coverage was 67% overall, but as #2 argued, the distribution was the problem: the surface an LLM drives unsupervised against a live project was the least tested part of the codebase, and tools.py had no test file at all.

Module Before After
tools.py 29% (no test file) 100%
cli.py 34% 85%
tunnel.py 41% 91%
dashboard/plugin_api.py 64% 83%
overall 67% 86%

88 new tests. No production code changes — the only non-test edit is adding .coverage to .gitignore.

What is actually asserted

The emphasis is on what decides which resource gets mutated, rather than on return-value shape.

  • The pause ceilingMAX_PAUSE_MINUTES is the only thing between a confused agent and an indefinitely paused connection. Every way a model can produce a bad number is covered: "10", -30, [], None, absent, over-cap. Plus that the minutes the tool reports are the minutes it actually scheduled, since the model plans against that number.
  • Pause → auto-resume, end to end. The tool writes a deadline to the ledger; adapter._resume_due_connections honours it. Covered separately, both halves pass while disagreeing about the ledger path or the row shape — and a deadline written where the adapter never looks is indistinguishable from the feature silently not existing.
  • The dashboard's ownership check on its refusal path. _require_own_connection is what stops this tab pausing somebody else's production traffic, and the endpoint is reachable whether or not the tab renders a button for it. Also that every configured route is consulted, not just the first.
  • The tunnel's backoff reset rule, which Increase test coverage, starting with the agent-callable tools #2 correctly flagged as easy to get backwards, along with terminate-then-kill on shutdown and the hookdeck ci opt-in.
  • doctor, including the check for a retry rule narrower than what the adapter emits — the silent-data-loss case.

Verification

The three safety-critical assertions were mutation-checked rather than trusted to the coverage number. Each of these fails the suite:

Mutation Caught by
Remove the pause ceiling 3 tests
Disable the ownership check 2 tests
Invert the backoff reset rule 2 tests

One test initially passed for the wrong reason and is worth calling out: RunLedger.__init__ calls mkdir(parents=True), so an "unwritable path" was being silently created. It now patches RunLedger to raise, and asserts both that the connection is still paused and that the failure is logged — a pause recorded nowhere is recoverable, a pause that fails silently is not.

Merge order

This should land after #3. It is based on main and adds no new merge conflicts — test-merging this branch against #3 produces exactly the same four conflicts (README.md, adapter.py, cli.py, test_adapter.py) that #3 already has against main, because #3 is four commits behind. tests/test_cli.py and tests/test_dashboard_api.py auto-merge cleanly.

The second commit here removes the couplings to #3 that would otherwise have caused spurious failures: the doctor tests no longer pin HOOKDECK_API_KEY by name (either spelling satisfies them), no longer assert on wording that #3's API-key message changes, and clear both env-var spellings so a developer's shell cannot make a check pass for the wrong reason.

Left alone, per #2's own ordering: adapter.py (72%) and dashboard/dist/index.js.

garethx and others added 2 commits August 11, 2026 15:11
…r commands

Coverage was 67% overall, but the distribution was the problem: the modules
an LLM can drive unsupervised against a live project were the least tested,
and tools.py had no test file at all.

  tools.py             29% -> 100%
  tunnel.py            41% ->  91%
  cli.py               34% ->  85%
  dashboard/…          64% ->  83%
  overall              67% ->  86%

The emphasis is on what decides *which* resource gets mutated, rather than
on return-value shape:

* the pause ceiling, every way a model can get a number wrong
* the pause -> auto-resume contract driven end to end, tool half and adapter
  half together, because covered separately they can disagree about the
  ledger path and both still pass
* connection resolution by name, including the name that matches nothing
* the dashboard's ownership check on its refusal path — the endpoint is
  reachable whether or not the tab renders a button for it
* the tunnel's backoff reset rule, which is easy to get backwards
* doctor's diagnoses, including a retry rule too narrow for what the adapter
  emits

Each of those was mutation-checked: inverting the backoff reset, removing the
pause ceiling, and disabling the ownership check all fail the suite.

Also ignores .coverage, which is a build artifact rather than source.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three of these were coupled to wording and env-var names that #3 changes, so
they would have failed on merge for reasons that are not regressions:

* the missing-API-key assertion pinned `HOOKDECK_API_KEY` by name, and #3
  renames it to `HOOKDECK_EG_API_KEY`. It now accepts either, because the
  regression worth catching is doctor staying quiet about a missing key.
* the push-mode test asserted "Hookdeck CLI" was absent from the output to
  prove the cli-mode checks had not run. #3's API-key message mentions the
  Hookdeck CLI, so that assertion broke without anything being wrong. It now
  records calls to `shutil.which`, which is what it meant all along.
* the fixture cleared only the unprefixed env vars, so a shell with the
  namespaced ones set could make a doctor test pass for the wrong reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase test coverage, starting with the agent-callable tools

1 participant