Skip to content

Add services.LookupAny + fix 6 binary-service silent failures#62

Merged
munezaclovis merged 8 commits intomainfrom
feat/services-lookupany
Apr 15, 2026
Merged

Add services.LookupAny + fix 6 binary-service silent failures#62
munezaclovis merged 8 commits intomainfrom
feat/services-lookupany

Conversation

@munezaclovis
Copy link
Copy Markdown
Contributor

Summary

  • Closes the silent-failure gaps left by the rustfs and mailpit binary-service migrations. Six callsites previously consulted only the Docker registry via services.Lookup and silently mishandled binary services (s3, mail).
  • Adds one new helper — services.LookupAny(name) (Kind, BinaryService, Service, error) — using the same binary-first lookup-order convention as the private resolveKind dispatcher.
  • Surgically fixes each callsite with a kind-branched dispatch.

Bugs fixed

Callsite Was Now
cmd/install.go:61 (parseWith) pv install --with=service[mail] errors "unknown service" accepts binary services
cmd/setup.go:70 (wizard list) mail and s3 hidden from the multi-select both visible with their DisplayName()
cmd/setup.go:251 (post-wizard provisioning) binary kinds silently skipped after selection provisioned via single-arg RunAdd
cmd/doctor.go:621 reports healthy mail/s3 as "registry may be out of date" binary services skipped (their health is covered by service:list/status/logs)
internal/commands/service/env.go:45 (loop) prints "Skipping unknown service" for each binary prints correct env vars
internal/commands/service/env.go:71 (single) pv service:env mail hard-errors prints MAIL_* keys correctly

Out of scope (deliberately deferred)

  • internal/commands/service/dispatch.go's private resolveKind enforces an extra registry-collision invariant; collapsing it into LookupAny would weaken the dispatcher contract.
  • internal/commands/service/stop.go's applyFallbacksToLinkedProjects rewrites linked .env for binary services even when supervised — different bug shape, separate follow-up PR.
  • ReadyCheck sum-type tightening, TestBuildSupervisorProcess_Mailpit, E2E .env injection assertions — separate PRs.

Test plan

  • go build ./... clean
  • go vet ./... clean
  • go test ./... — 30 packages pass, 0 failures
  • 14 new tests added: TestLookupAny_* (4), TestBuildServiceOptions_* (2), TestEnvVarsFor_* (3), TestDoctor_SkipsBinaryServices (1), TestParseWith_BinaryService* / _UnknownServiceMongodb (3), plus the Docker-less regression assertion in the doctor test
  • Doctor regression assertion empirically verified: temporarily reverting the fix causes TestDoctor_SkipsBinaryServices to FAIL with "doctor output should not suggest restarting binary service mail"
  • CI: existing E2E phases continue to pass (no E2E changes in this PR)

Design + plan docs

  • Spec: docs/superpowers/specs/2026-04-15-services-lookupany-design.md
  • Plan: docs/superpowers/plans/2026-04-15-services-lookupany.md

Closes the silent-failure gaps left over from the rustfs and mailpit
binary-service migrations. Six callsites currently consult only the
Docker registry via services.Lookup and silently mishandle binary
services (s3, mail) — the wizard hides them, post-wizard provisioning
skips them, doctor reports them as broken, install validation rejects
them, and `pv service:env` errors on them.

Spec introduces a single new helper services.LookupAny returning a
tagged-union (Kind, BinaryService, Service, error), then walks each
of the six callsites with a small kind-branched fix. Out of scope:
the dispatcher's resolveKind (different invariant), stop.go's
applyFallbacks issue (different bug shape), and the ReadyCheck
sum-type tightening (separate type-design PR).
Six tasks executing the design from
docs/superpowers/specs/2026-04-15-services-lookupany-design.md:
1. Pre-flight verification of service.RunAdd's arg shape.
2. services.LookupAny + 4 unit tests (TDD).
3. cmd/setup.go: extract buildServiceOptions + post-wizard branch.
4. internal/commands/service/env.go: extract envVarsFor dispatch helper
   so both call sites and the test exercise the same code path.
5. cmd/doctor.go: skip binary services in per-service Docker check.
6. cmd/install.go: parseWith validation accepts binary services.

Each task ends with a commit; six commits to land on the branch.
Resolves a service name across both Docker and binary registries,
returning a tagged union (Kind, BinaryService, Service, error). Binary
wins on a hypothetical collision — matching the order used by the
service:* command dispatcher's private resolveKind helper.

Closes the gap that left several callsites only consulting the Docker
registry and silently mishandling binary services (s3, mail). Per-callsite
fixes follow in subsequent commits.
Extracts buildServiceOptions() so the wizard's service multi-select
includes both Docker and binary services (mail, s3). The post-wizard
provisioning loop now branches on kind and calls service:add with the
appropriate arg form for each kind — the binary branch uses the
single-arg form which routes through resolveKind correctly.

Fixes two of the six silent failures called out in the mailpit PR review.
Extracts an envVarsFor helper that resolves the service name via
LookupAny and dispatches to the right EnvVars signature for each kind
(BinaryService.EnvVars(projectName) vs Service.EnvVars(projectName,
port)). Both call sites in envCmd now go through it, eliminating the
duplicated branching and giving us a real test surface.

Previously "pv service:env mail" hard-errored and "pv service:env"
with no args printed "Skipping unknown service" for each registered
binary service.

Fixes two of the six silent failures from the mailpit PR review.
Doctor previously reported every registered binary service (mail, s3)
as "unknown service type — registry may be out of date" because the
loop fed every name through services.Lookup, which only consults the
Docker registry. Binary services have first-class observability via
pv service:list / pv service:status / pv service:logs; doctor's job
is cross-cutting infra health, not per-binary-service supervision
health.

Fixes one of the six silent failures from the mailpit PR review.
parseWith now uses services.LookupAny, so --with=service[s3] and
--with=service[mail] no longer falsely error as 'unknown service'.
Validation behavior for genuinely unknown names is unchanged.

Closes the last of the six silent failures from the mailpit PR review.
The pre-existing absence-of-error-strings checks only fire when Docker
is reachable, because the lookup_error branch in cmd/doctor.go is gated
behind `engine != nil`. In CI environments without Docker (or local
runs without Colima), the test passed regardless of whether the fix
in 6f19458 was applied — empirically verified by reverting the fix and
seeing the test still pass.

Add a positive assertion that the output does not suggest
"pv service:start mail" — pre-fix code surfaces mail as a failing
check with that Fix message even when engine is nil. Confirmed catches
the regression: with the fix temporarily reverted, the test now FAILS.
@munezaclovis munezaclovis merged commit f5121e0 into main Apr 15, 2026
1 check failed
@munezaclovis munezaclovis deleted the feat/services-lookupany branch April 15, 2026 15:23
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.

1 participant