CONCERNS

Concerns & Technical Debt

Analysis Date: 2026-08-15

High Priority

launcher.py is a 4931-line module holding 100 functions and both backends

• Location: src/harnessed/launcher.py
• Impact: The largest module in the tree by a factor of 1.5 over schema.py (3231) and 4× over mounts.py (1212); src/harnessed/ totals ~20k lines across 37 modules, so a quarter of the application sits in one file. It carries the Typer CLI, the build path, auth wiring, the aoe bridge, and both ExecutionBackend implementations (HostBackend:2167, ContainerBackend:2783). The size is what makes the CWD and bare-except findings below easy to introduce and hard to see, and it forces most launch-path tests to stub at the module boundary rather than the unit.
• Fix approach: The seam already exists — backend.py defines the contract and tests/test_module_boundaries.py enforces the one-way import. Extracting the two backend classes into their own modules is mechanical and does not change behavior; the co-location was a deliberate choice to avoid an import cycle (see backend.py's docstring), so any split must keep the dependency pointing into backend.py.
• Note: This concern predates the 2026-08-15 regeneration and was dropped from it without being fixed. Re-stated here at the current measured size.

svc command always resolves from CWD — no --path flag

• Location: src/harnessed/launcher.py:4657
• Impact: The svc command unconditionally calls Path.cwd().resolve() with no path parameter accepted. Every other run-path (host_run, container_run, setupenv) accepts a --path flag and falls back to CWD only if unset. The CLAUDE.md constraint "Never key build/assembly off the CWD" is violated for svc — invoking it from a wrong directory silently uses the wrong project key, producing confusing "service not found" errors.
• Fix approach: Add a --path optional argument mirroring the pattern at launcher.py:2320 and launcher.py:2619.

launchenv.py exception handlers swallow error context on cleanup paths

• Location: src/harnessed/launchenv.py:160, src/harnessed/launchenv.py:216
• Impact: Both _write_op_env_file and _normalize_plain_env_file use bare except Exception during temp-file cleanup before re-raising. If the secondary os.unlink itself raises an OSError, the original exception is lost in Python's exception chaining only if the cleanup OSError propagates — but there is no raise ... from exc to preserve the original. In practice the cleanup is wrapped in its own except OSError: pass, so the primary exception does re-raise. The real risk is diagnostic: if os.chmod or os.fdopen fails in an unusual way, the stack trace entry pointing at the real cause may be obscured.
• Fix approach: Use except Exception as exc: ... raise instead of bare except Exception to ensure the cause stays in __context__.

No dedicated test file for the svcstate.py service lifecycle

• Location: src/harnessed/svcstate.py
• Impact: Scoped claim — svcstate.py is not untested. _service_refs (svcstate.py:274) is exercised from seven test files (test_service_refs.py, test_launcher_services.py, test_project_scoped_services.py, test_stable_port.py, test_backend_seam.py, test_launch_host.py, test_capmatrix.py), several through the launcher re-export. The real gap is narrower: the lifecycle dispatch (up, down, recreate, sync) and the three subprocess.run callsites for podman inspect/pod/exec have no dedicated coverage, so a regression there is invisible unless another test incidentally crosses it.
• Fix approach: Add tests/test_svcstate.py with monkeypatched subprocess.run covering the up/down/recreate dispatch and the timeout path.

Medium Priority

capability.py mkdtemp ownership contract is implicit and untested

• Location: src/harnessed/capability.py:510, src/harnessed/capability.py:899
• Impact: _launch_stack_headless and a second callsite allocate a temp project dir with tempfile.mkdtemp and document "CALLER owns its lifetime". The comment at line 507 warns that deleting the dir while the pod runs breaks podman exec. If a caller forgets cleanup or raises before teardown, the scratch dir leaks under /tmp. The test suite never exercises the pod layer (CLAUDE.md: "suite runs no podman build and no harnessed container-run"), so no test can catch a missing cleanup.
• Fix approach: Return the temp path alongside the launch result so a try/finally at the call site is structurally required, or convert to a context manager.

launcher.py:3266 bare except Exception on build failure cleanup

• Location: src/harnessed/launcher.py:3266
• Impact: The container_run cleanup block (cleans up an orphan minted manifest on build failure) uses except Exception with no comment explaining what exceptions are expected or what would be missed. Unlike the annotated noqa: BLE001 handlers in aoe.py, this one carries no rationale. A SystemExit or KeyboardInterrupt would pass through (not swallowed), but typer.Exit — which _build_stack raises for recoverable CLI errors — is caught, preventing the manifest orphan cleanup from running when _build_stack exits with code 0.
• Fix approach: Narrow to except (Exception, typer.Exit) with explicit handling, or document intent with a noqa comment.

subprocess.run calls without timeout in svcstate.py

• Location: src/harnessed/svcstate.py:213, src/harnessed/svcstate.py:353, src/harnessed/svcstate.py:404
• Impact: All three subprocess.run calls in svcstate.py omit timeout=. A hung or unreachable podman daemon blocks the calling process indefinitely. launcher.py wraps podman calls in _bounded() which enforces _PODMAN_QUERY_TIMEOUT; svcstate.py calls subprocess.run directly and has no such guard.
• Fix approach: Replace direct subprocess.run with _bounded() using _PODMAN_QUERY_TIMEOUT, matching the pattern used in launcher.py.

_SHARED_IMAGES_BUILT process-level set is never cleared

• Location: src/harnessed/launcher.py:513
• Impact: The module-level set _SHARED_IMAGES_BUILT tracks which shared images have been built this process. Under --jobs > 1, images are correctly built once. But if a build fails partway and the image is left in a broken state, re-running in the same process (e.g., via test harness) would skip the rebuild because the set already contains the image tag. In production CLI use (one process per invocation) this is harmless; in tests that call into _build_shared_once directly, it can mask test isolation failures.
• Fix approach: Accept for production use; add a reset fixture or monkeypatch in tests that exercise the shared-image path.

Low Priority / Improvement Opportunities

credmounts.py bare except Exception without noqa rationale

• Location: src/harnessed/credmounts.py:274, src/harnessed/credmounts.py:293
• Note: Both handlers carry # noqa: BLE001 but no inline explanation of what is expected to be caught. The equivalent handlers in aoe.py explain the "never fail a launch" rationale. Without a comment, a reader cannot tell whether the silent suppression is intentional for all exceptions or is missing an as exc for logging.

Path.cwd() fallback pattern repeated across five call sites

• Location: src/harnessed/launcher.py:2320, :2619, :3275, :4657, :4792
• Note: The pattern Path(path).resolve() if path else Path.cwd() appears four times as a correct, guarded fallback. Line 4657 omits the guard entirely (no path parameter). Extracting the pattern to a helper would make future violations easier to spot.

launchenv.py env-file quote-stripping is single-level only

• Location: src/harnessed/launchenv.py:179
• Note: _parse_plain_env_line strips exactly one pair of surrounding quotes. Nested or escaped quotes (e.g., KEY="val with \"inner\"") are not handled. This is unlikely in practice but is an undocumented limitation that could confuse users who author complex .env files.

No integration test for update.py against a real registry

• Location: src/harnessed/update.py, tests/test_update_*.py
• Note: Update tests (test_update_pins.py, test_update_cooldown.py, test_update_release_selection.py) mock urllib.request calls. The suite never makes a live HTTPS request to the ghcr.io or PyPI registries, so a registry API change or auth regression would not be caught until a user hits it.

TODOs and FIXMEs Found

No TODO, FIXME, HACK, XXX, or WORKAROUND markers were found in src/, catalog/, tools/, or tests/. The deprecated markers found are all documentation notes about SSE transport being superseded by Streamable HTTP — enforced at build time in schema.py:1306 — not open work items.

Missing or Weak Areas

• svcstate.py's lifecycle dispatch lacks dedicated coverage — _service_refs is well covered via seven test files, but the up/down/recreate/sync paths and their three subprocess.run callsites are not directly tested.
• podman build and harnessed container-run paths are structurally unreachable by the test suite (documented in CLAUDE.md); capability.py, volumes.py, and the build side of launcher.py have coverage only for schema/assembly logic, not for live container behavior.
• Corporate proxy CA injection (_service_dockerfile_with_ca) has no test — the function is only exercised if a cert file is present on the test host, which is never true in CI.
• aoe.py's live-binary path needs aoe on PATH, so test_aoe_real.py (123 lines) skips in most CI environments. The module itself is not thinly tested: tests/test_aoe.py is 1188 hermetic lines covering identity derivation, grouping, dedup and the detached write path. Only behavior that depends on a real aoe binary goes unverified in CI.