I’m building Withmere, a personal AI environment. A long-running daemon on my Mac connects agents, each working inside its own conversation session, to my projects. One of its features is Preview: an agent can serve a directory, an npm dev server or a server already running on the machine, and publish it to my devices over Tailscale Serve, which maps a local port to an HTTPS address on my private network. I open the URL on my phone and see the page the agent just built.
Starting a Preview launches a worker process, waits for it to answer a health check, publishes a route, and records all of it as a row in a manifest file. The row is written as a candidate before the route goes live, so that a restarted daemon can find and clean up anything an earlier one left half-built.
On 1 October a coding agent and I added a small tool that ranks functions by CRAP score, a metric that combines cyclomatic complexity with test coverage. Its first repository-wide run put one function far ahead of everything else:
CRAP CC cov location
2429.09 53 7/129 kernel/preview/controller.py:270 PreviewController.start
897.47 30 1/82 interfaces/cli.py:30 CLIInterface.start
693.19 150 123/173 kernel/event_types.py:2887 validate_night_watch_event
The formula is CC² × (1 − coverage)³ + CC. With complexity 53 and 7 of 129 statements executed by the default test suite, the cubic term dominates: 2809 × 0.846 + 53 ≈ 2429. The score is saying that a complex function has almost no tests. The ordinary response is to write them.
That response turned out to be unavailable. The reason is the subject of this essay: whether code can be tested is decided by where its boundaries are drawn, and a coverage number reports the result without the cause.
Two ceilings on coverage
The seven covered statements took a lock, resolved the source, compared the owner’s login and read the manifest. Every test that called start expected it to refuse: five bad sources stopped at line 274, and a corrupt manifest at line 278.
PreviewController.start · abridged
Where the one test stopped
Every test that called start expected a refusal; the last of them feeds it a corrupt manifest and stops at line 278. Nothing below the line runs in the default suite.
- 270def start(self, invocation, kind, source, *, …, admit=None):
- 271with self._locked():
- 272if admit is not None:
- 273admit()
- 274canonical, package = self._source(invocation, kind, source, …)
- 275current_login, dns_name = self.serve.self_identity()
- 276if current_login.casefold() != self.owner_login.casefold():
- 277raise PermissionError(…)
- 278rows = self._read()
- JSONDecodeError: the test ends here, as intended
- ⋮19 lines: retry lookup, HTTPS port, tokens, worker config
- file298fd = os.open(config_path, os.O_CREAT | os.O_EXCL | …, 0o600)
- spawn304process = subprocess.Popen([sys.executable, …])
- seam309identity = self.host.process_identity(process.pid)
- signal312os.killpg(process.pid, signal.SIGTERM)
- ⋮32 lines: build and write the manifest row
- process345if process.poll() is not None:
- http ×80352with urllib.request.urlopen(request, timeout=0.25) as …:
- sleep356time.sleep(0.05)
- socket362with socket.create_connection((host, port), timeout=1):
- seam367self.serve.on(https_port, target)
- ⋮27 lines: route read-back, then the failure block begins
- signal395os.killpg(process.pid, signal.SIGTERM)
- process399process.wait(timeout=3)
- seam405if not self.host.group_alive(process.pid):
- sleep407time.sleep(0.05)
- lsof411if listener_state(upstream_port, process.pid) not in …:
- file417config_path.unlink(missing_ok=True)
Part of what lay beyond was plain neglect. The retry lookup just after line 278 reads the manifest and asks the host interface whether a worker is alive, and the existing fakes could have driven it. From line 304 on, it is different: start launches the worker with subprocess.Popen and from then on talks to the operating system directly. A test that wanted to go further had two options. It could start a real worker and a real Tailscale route, which is slow, depends on the network and is exactly the kind of test the project keeps out of the default suite. Or it could monkeypatch subprocess.Popen and urllib, which the project’s test rules forbid.
The rule against monkeypatching is sound. Michael Feathers defines a seam as a place where you can alter behaviour without editing the code at that place. self.host.process_identity(pid) is a seam the design names: pass a different host and the behaviour changes. Patching subprocess.Popen also finds a place to alter behaviour, but one the design never names. It lives in the test file, tied to whatever the implementation looks like that day, and the next person who needs it has to build it again.
So coverage has two ceilings. One is effort: nobody wrote the tests. The other is structure: the tests cannot be written without changing the code under test. CRAP assigns both the same number. Their prescriptions are opposite. For the first, write tests. For the second, the structural change comes first, and the tests become its acceptance check.
Seams grow where tests already are
The function did have a host interface. PreviewHost abstracted three facts: free_port, process_identity and group_alive. It was drawn on 28 September, when the tests that ran start against real worker processes moved out of the default suite into a named host check, because they depended on the real host. The move was reasonable. What stayed behind were fast tests for stopping, listing and reconciling Previews, and the interface took the shape of what they needed. Nobody decided what a host boundary for Preview should be; the tests that remained decided it.
That is a feedback loop running the wrong way. Code that can only be tested with real processes leaves the fast suite; code outside the fast suite creates no demand for seams; code without seams can only be tested with real processes. The code with the heaviest side effects is the code that most needs seams and is least likely to get them.
The function’s history shows how it got there.
Five days of commits
How start grew
Length of the function at each commit that touched its file. Hollow: the commit left start unchanged.
656b3486 feat: add session-bound tailnet previews11511770399 fix: verify file and npm listener ownership121322927d3 fix: tighten authorization and recovery boundaries155dc9e7815 fix: reclaim dead workers and report URL availability1555a49ddfa fix: allow loopback attach and preserve HMR protocol15595f9afe3 fix: remove shell turn binding155Each of those fixes came with tests, most of them run against real workers. When the tests moved to the host check on the last day, the branches they covered stopped counting in the default suite, and a function that had been tested became one that coverage could not see into.
The first fix drew more windows in the wrong wall
The agent’s first proposal was to widen PreviewHost with seven more methods, spawn_worker, wait_ready, terminate_group, drain_group among them, so that a fake host could script each failure. It would have worked, in the sense of making start testable.
My objection was that the boundary itself was strange. The file went from controller logic straight to operating system calls, skipping a great deal in between. Its imports made the point: fcntl, os, signal, socket, subprocess, sys, time and urllib.request, in a module whose job was authorisation, idempotent retry, uncertainty semantics and audit. Laid out by altitude, start stood on six layers in a single stack frame.
Six altitudes, three boundaries
Who holds each layer of start
Each row is one altitude the function works at. Each column is a module. Switch the boundary and see where the layers land.
As foundPolicy reaches straight into the operating system
- Controller imports fcntl, os, signal, socket, subprocess, sys, time, urllib.request.
- A fake host can script three facts: free port, process identity, group alive.
- From line 304 on, nothing in start can run without a real worker.
Widen the hostMore methods, still at the bottom layer
- start becomes testable: a fake host scripts spawn, wait, drain and probes.
- The fake speaks in pids and return codes, not in the terms the controller reasons in.
- Readiness and cleanup are still decided inside start, written twice.
Add a worker moduleThe missing layer gets an owner
- signal, sys and urllib.request leave the controller; every call that acts on a worker moves behind the port.
- The fake speaks in Outcome(status, reason); the adapter is tested on real processes.
- The abort path and stop share one cleanup.
Ingress already had its own adapter, and it was the one boundary in the file that was drawn correctly. The worker layer had code but no owner, so policy reached directly into the operating system. Widening the host would have added more methods at the wrong altitude, and a fake that spoke in return codes and pids rather than in the terms the controller reasons about.
What was missing was a module that owns a worker’s lifecycle: launching it, knowing whether it is still the same process, deciding when it is ready, stopping it and confirming its process group is gone. The controller would talk to that module, and only that module would talk to the operating system.
A worker’s identity is a value
The first design constraint came from a fact about the controller itself: it restarts. The daemon that started a Preview may not be the one that stops it. A restarted daemon reads the manifest row and must decide whether the process at that pid is still the worker it recorded, or an unrelated process that inherited a reused pid. A Popen handle cannot answer that, because it does not survive the restart.
The existing code already answered it with a pair of values stored in the row: the pid and the process’s start time as printed by ps -o lstart. The pair was the identity in practice; it just had no name. A cleaner alternative was on the table: let each worker hold a lock on its own config file for life, and ask the kernel who holds it.
Constructed example
A pid is a number the system hands out again
The manifest row remembers pid 4242. A restarted daemon has to decide whether 4242 is still its worker. Two schemes read different evidence.
1 Choose when reconcile runs
2 Read where the cursor crosses each track
3 What each scheme concludes
ps reading vs manifest row
12:03:40 ≠ 12:01:07
Not ours: no signal
who holds the config lock
no holder
Gone: nothing to stop
All four moments
A is already how the code worked and needs only ps and a string comparison. It is wrong only in the last row.
B is right in every row and needs no ps, but changes the protocol between controller and worker, with a release boundary while workers without the lock still run.
The new module named the pair as the worker’s identity and declared the start time opaque: the macOS adapter produces it and compares it, and nothing else interprets the string, so a later adapter can switch to a numeric kernel timestamp without changing the interface. The lock-holder design stays in the record as the alternative, with the reason it was not built.
To choose the interface itself, the agent ran three design passes as separate agent sessions with no shared history, each under a different constraint. They agreed on more than they disagreed about, and one row of the comparison settled it.
Three drafts, separate contexts
The same module, designed three ways
Each draft read the same code under a different constraint. The two marked rows decided it: the first ruled out handles, the second chose between the other two.
Smallest interface
- Entry points
- 3: launch(spec, publish), observe, stop
- After a restart
- Default path: every call takes a value ref
- Holds a Popen
- No
- Callback
- Yes: publish(ref) lets the controller write its row mid-launch
- Kept from it
- observe: identity, group, listener and health in one judgement
Most flexible
- Entry points
- 8: a host with 4 methods issuing handles with 4, plus two policy value types
- After a restart
- Special path: attach() builds a handle for a process it did not start
- Holds a Popen
- Yes, wrapping Popen
- Callback
- No
- Kept from it
- spawn and ready kept apart, so the row is written between them
Simplest call sites
- Entry points
- 5: spawn, ready, observe, stop, loopback_host
- After a restart
- Default path: WorkerRef.from_row(row)
- Holds a Popen
- No
- Callback
- No
- Kept from it
- start rewritten: its abort becomes one stop call
Value refs, two-step launch, one observe
spawn, ready, observe, stop, identity_of. The restart path is the default because the controller restarts. No callback, because it would have been the interface’s only inversion of control and existed to save one method. No handle on the interface.
class WorkerPort(Protocol):
def spawn(self, spec: WorkerSpec) -> WorkerRef: ... # raises if no identified worker results
def ready(self, ref: WorkerRef) -> Outcome: ...
def observe(self, ref: WorkerRef, *, probe_health: bool = False) -> Observation: ...
def stop(self, ref: WorkerRef, *, confirm_exit: bool = False) -> Outcome: ...
def identity_of(self, pid: int) -> str | None: ... # daemon and request-owner identity
@dataclass(frozen=True)
class Outcome:
status: Literal["ok", "failed", "uncertain"]
reason: str = ""
The three-valued outcome was not invented for the port. The old stop path already returned stopped or uncertain with a reason, and treated “the host could not tell me” as different from “the process is gone”. All three drafts converged on naming that, with one reason vocabulary shared by every path: process_identity_changed, npm_group_leader_missing, npm_group_still_alive, npm_listener_unowned, worker_unobservable, worker_lingering.
One apparent cost survived the comparison. Stopping a routed Preview reads process identity twice: once in observe, before the controller re-checks authorisation and removes the route, and again inside stop, before the signal. All three drafts listed this as a weakness, and so did the agent’s summary. It is the safety property: between the two reads the controller does slow external work, and the second read means no signal goes to a pid whose identity contradicts the record.
The failure path was already a stop
Reading start beside the method that stops a running Preview showed what its fifty-line failure block really was.
The failure path of start
Fifty lines that were already a stop
Steps of the old abort block. Marked steps also existed, written separately, in the normal stop path.
Abort, written inline
- Read the route back; remove it if it is ours; read againalso in stop
- Route still there: keep the row, audit, raiseabort only
os.killpg(pid, SIGTERM)also in stopprocess.wait(timeout=3); timeout: keep the row, raiseabort only- npm: drain the group, 30 × 50 ms; still alive: keep the row, raisealso in stop
- npm: listener outside the group: audit, raisealso in stop
- Unlink the config filealso in stop
- Remove the row from the manifestalso in stop
- Audit the outcomealso in stop
Abort, through the port
- Route not cleared: keep the row, raise
workers.stop(ref, confirm_exit=True)- Uncertain: keep the row, raise
- Remove the row; audit the failure
With a port in place, the block became a call:
def _abort(self, invocation, row, ref, *, recorded, route_attempted):
if route_attempted and not self._route_cleared(row["https_port"], ref.target):
self._upsert_row(row) # keep the candidate; the route may be live
raise PreviewCleanupUncertain(...)
done = self.workers.stop(ref, confirm_exit=True) # no route, no browser yet
if done.status == "uncertain":
self._upsert_row(row)
raise PreviewCleanupUncertain(...)
if recorded:
self._remove_row(row["id"])
There is now one cleanup to fix instead of two copies evolving separately. start went from 155 lines to about 75.
What three reviews found
Drawing a boundary once does not keep things on the right side of it. The implementation, written by the coding agent and read by both of us, passed the old preview tests unchanged, added tests for the new module, and looked finished. Three reviews then ran as separate agent sessions with no shared history, each with a different brief: old against new line by line; adversarial scenarios against the state machine and concurrency; and the quality of the tests themselves. Each found something the change’s author could not see, and each was something carried across a line the design had just drawn.
Three reviews, fresh contexts
Three things that crossed a line
Each finding is something the new design carried across a boundary it had just drawn.
The handle came back
Keeping Popen objects for the workers this process launched is harmless.
Dropped Popen objects had been reaped by accident. Kept ones would have become zombies, and ps still reports a zombie’s start time.
Reap owned processes before every identity read.
- worker crashes
- Popen already dropped
- next subprocess call reaps it
- ps: no process
- absent
- worker crashes
- Popen kept
- zombie
- ps prints its start time
- observe: alive
Abort’s condition does not hold for stop
Stop should wait for the worker to exit, as abort did.
Abort runs before any route or browser exists. Stop runs with a page open, and the worker may take 60 s to close it.
Wait only when the abort path asks: stop(confirm_exit=True).
- no route
- no browser
- wait ≤ 3 s
- exits
- page open
- websocket held ≤ 60 s
- lock waiters give up at 5 s
- uncertain: worker_lingering
The fake agreed with itself
Scenario tests through the fake prove how the controller handles each outcome.
The fake copied the adapter’s ownership rules. Two tests checked the copy against itself.
Empty the fake; test the real adapter on real processes.
- fake mirrors adapter rules
- controller test passes
- proves the copy
- fake returns scripted outcomes
- adapter tested on real processes
- proves the contract
The handle came back. The new adapter keeps a Popen object for every worker it launches, because stopping one of its own needs it. The old code had dropped those objects, and CPython quietly reaps a dropped Popen the next time any subprocess starts, so a crashed worker used to vanish at the next ps call. Kept, it would have lingered as a zombie, and macOS ps still prints a zombie’s start time: observe would have reported a dead worker as alive and unchanged. The first review caught this before the commit, and the adapter now reaps its own processes before every identity read. The design had just decided that identity is a value, not a handle. The handle, brought back for one job, removed a cleanup the old code had never known it relied on.
Abort’s condition does not hold for stop. I had made stop wait up to 3 seconds for the worker to exit, as abort did, and recorded it as an improvement. But a Preview open in a browser holds a websocket for hot reload, and the worker’s web framework (aiohttp) allows up to 60 seconds to close open connections on shutdown, so an ordinary stop would report uncertain: worker_lingering and leave the row behind. Automatic reconciliation also runs stops inside the manifest lock, where another process waiting for the lock gives up after 5 seconds. Abort can wait because no route and no browser exist yet. The precondition had been carried from one call site to another where it was false.
The fake agreed with itself. The controller’s scenario tests used an in-memory fake whose ownership rules were a copy of the real adapter’s, with a comment that said Mirrors HostWorkers. Two tests about orphaned process groups passed through that copy, so they proved that the copy agreed with itself. Production logic had crossed into the test double. The fake now holds no logic: it returns scripted outcomes and records whether each stop call set confirm_exit, so controller tests assert which part of the port contract the controller used.
Whether the real adapter honours that contract is tested on its own side of the line, without Tailscale or a real Preview. Its tests launch throwaway Python processes that sleep, ignore SIGTERM, exit before becoming ready, or leave an orphaned process group; a second adapter instance stops a worker the first one launched, which is the restart path. The third review ran that file ten times without a failure.
What the change established
The CRAP score of start fell from 2429 to 16, with 48 of 50 statements covered and complexity down from 53 to 16; much of that complexity now lives in the adapter’s stop. The score is the least informative result of the change. The tests were written for the new seams, so coverage had to rise; the number shows that the function can now be tested, not that it is correct.
The evidence for correctness is the scenarios, and the three findings above show that the ones that mattered were found by readers who had not written the change. The preview tests went from 22 to 54 and run in about 2 seconds. The 22 original tests were not edited and still pass; sixteen new ones drive the real adapter with real subprocesses, and sixteen drive the controller through the logic-free fake. The full suite of 3,737 passed. Eight changes in visible behaviour, such as authorisation being re-checked before the first side effect, are recorded as intended rather than presented as preserved.
CRAP score · CC² × (1 − coverage)³ + CC
From the top of the list to the bottom of the scale
start before and after the change, beside the next three functions in the repository ranking of 1 October.
PreviewController.startbefore 2,429 CLIInterface.start 897 validate_night_watch_event 693 SchedulerRunner._inject_body 582 PreviewController.startafter 16 Table
| Function | CRAP | CC | Statements covered |
|---|---|---|---|
PreviewController.start (before) | 2429.09 | 53 | 7/129 |
CLIInterface.start | 897.47 | 30 | 1/82 |
validate_night_watch_event | 693.19 | 150 | 123/173 |
SchedulerRunner._inject_body | 581.69 | 127 | 208/299 |
PreviewController.start (after) | 16.02 | 16 | 48/50 |
Preview after the change
Where a Preview request goes
Each tier talks only to the one below it. The marked box is the module this change added.
preview.sh start npm devPreviewBrokerchecks the caller's SessionPreviewControllerauthorisation, retry key, owner login, candidate rows, auditServeAdapterroute on, off, read backtailscale serveHTTPS on the private networkWorkerPort · HostWorkersspawn, ready, observe, stop; every OS call for a workerps · Popen · killpg · lsofthe operating systemThe boundary is drawn around the worker, not around everything the controller touches. The controller still imports os, socket, subprocess, time and fcntl, for the manifest lock, a loopback probe and the Serve adapter. What left it are signal, sys, urllib.request and every call that acts on a worker.
Telling the two apart
A coverage number cannot answer the title’s question; a reader of the code can, by asking one thing. Can a test reach the uncovered lines through an interface the design already has, without a real side effect? If so, the function is untested, and the work is writing tests. If not, it is untestable, and the work is the boundary, with the tests as its acceptance check.
That morning the project’s refactoring rule had gained a line saying that a thinly covered function gets tests first. The first function the tool ranked was a counterexample. Coverage also arrives late: it reports the problem long after the design made it. A static signal might flag it sooner, such as heavy branching in a function that calls the operating system directly, which start had in one frame. I have not built that check, and one function cannot show whether it works.
The rule against patching subprocess was right. It is half of a testing policy; the other half is putting seams where the rule will need them.
Untested is a gap in the tests. Untestable is a gap in the design, and only the design can close it.
$ preview.sh --agent <agent> start npm dev
{"status": "started", "url": "https://<host>:8443/?preview_token=<redacted>"}
# This essay was revised while being read through Previews like this one,
# served by the worker module described above.
# Ad space available.