heavy-duty/cast

Fork 0

feat(draft): capture service hostnames via per-service GET (#83) #95

Merged

dan-claude-bot merged 2 commits from feat/draft-service-domains into main

2026-07-18 20:25:37 +00:00

dan-claude-bot commented

2026-07-16 18:31:13 +00:00

(Migrated from github.com)

Stacked on #93 — merge that first; this PR's diff includes its commits until then.

What

#73/#81 made a service's per-container hostnames settable (urls) and readable (GET /services/{uuid} → applications[].fqdn), and diff/apply carry them as service_domains. The draft path was never brought along: the inventory sweep's environment-list GET does not eager-load service.applications, and --emit-draft never made the supplementary per-service GET — so every drafted service came out with no hostnames and a hand-wave in UNCAPTURED.md.

Now the draft captures them:

The CLI's draft loop makes the per-service GET /services/{uuid} for every drafted service and emits service_domains from applications[].fqdn.
The projection is shared, not duplicated: projectServiceDomains is extracted out of attachServiceDomains (cli.ts) and exported, so draft and diff read the wire shape through the exact same projection + canonicalizeServiceDomains — a drafted manifest diffs clean the moment it is applied, down to container and URL order.
The projection's two absences stay distinct, and each reader takes its own (opposite, both correct) position:
- {} — read cleanly, no hostnames. An answer: the draft emits nothing and reports nothing, exactly as the live side leaves service_domains absent for a domainless service.
- undefined — not read. attachServiceDomains still fails a one-project diff closed (its output feeds an apply); the draft reports a per-resource UNCAPTURED entry and keeps sweeping — a whole-instance blueprint is not traded for one unreadable row.
The service_domains (hostnames) always-uncaptured entry and the service hostnames NO_API_COVERAGE row are deleted, and docs/semantics.md's "…but inventory --emit-draft does not yet make the per-service GET" line is corrected.

Shared design decision (same as #93 / #75)

Both PRs add one supplementary GET per resource to a verb that walks every project on the instance. Decided once, applied identically to databases (#75) and services (#83):

Always fetch, no gate — fetchLive's opts.backups/opts.serviceDomains gates exist because the read-side sweeps never look at the answer; the draft is the sweep that does. Paid only for drafted resources (the chosen environment per project), the same scope as the per-resource env-var GETs the draft already makes.
Sequential, like the existing draft-loop reads — a one-shot adoption verb, not a hot path.
Per-resource failure degrades to an UNCAPTURED report, never aborts the sweep — unlike diff/apply, whose output feeds a write and which refuse/fail closed on the same answers.

Test plan

test/draft.test.ts: new #83 describe — a captured map emits loadable service_domains with nothing uncaptured (and the stale claim gone from every artifact); {} emits nothing and reports nothing; undefined produces exactly one per-resource report while the rest of the spec survives.
test/live-lookup.test.ts: direct projectServiceDomains tests — {} vs undefined distinction, canonicalization; the existing attachServiceDomains fail-closed tests still pass against the refactor.
test/draft-cli.test.ts: stub now answers GET /services/s1 (applications[].fqdn, one container with no fqdn) while GET /services/s9 404s — end-to-end assertions that the umami's manifest loads with service_domains: { umami: [...] }, the barber shop's manifest is still drafted (report, not abort) with none, and UNCAPTURED names the failed read.
Full npx vitest run: 543 passed (29 files). npx biome check --error-on-warnings .: clean.

Closes #83

🤖 Generated with Claude Code

> **Stacked on #93 — merge that first; this PR's diff includes its commits until then.** ## What #73/#81 made a service's per-container hostnames settable (`urls`) and readable (`GET /services/{uuid}` → `applications[].fqdn`), and `diff`/`apply` carry them as `service_domains`. The draft path was never brought along: the inventory sweep's environment-list GET does not eager-load `service.applications`, and `--emit-draft` never made the supplementary per-service GET — so every drafted service came out with **no hostnames** and a hand-wave in UNCAPTURED.md. Now the draft **captures them**: - The CLI's draft loop makes the per-service `GET /services/{uuid}` for every **drafted** service and emits `service_domains` from `applications[].fqdn`. - **The projection is shared, not duplicated**: `projectServiceDomains` is extracted out of `attachServiceDomains` (cli.ts) and exported, so draft and diff read the wire shape through the exact same projection + `canonicalizeServiceDomains` — a drafted manifest diffs clean the moment it is applied, down to container and URL order. - The projection's two absences stay distinct, and each reader takes its own (opposite, both correct) position: - `{}` — read cleanly, no hostnames. An **answer**: the draft emits nothing and reports nothing, exactly as the live side leaves `service_domains` absent for a domainless service. - `undefined` — not read. `attachServiceDomains` still **fails a one-project diff closed** (its output feeds an apply); the draft **reports** a per-resource UNCAPTURED entry and keeps sweeping — a whole-instance blueprint is not traded for one unreadable row. - The `service_domains (hostnames)` always-uncaptured entry and the `service hostnames` `NO_API_COVERAGE` row are deleted, and `docs/semantics.md`'s "…but `inventory --emit-draft` does not yet make the per-service GET" line is corrected. ## Shared design decision (same as #93 / #75) Both PRs add one supplementary GET per resource to a verb that walks every project on the instance. Decided once, applied identically to databases (#75) and services (#83): - **Always fetch, no gate** — `fetchLive`'s `opts.backups`/`opts.serviceDomains` gates exist because the read-side sweeps never look at the answer; the draft is the sweep that does. Paid only for **drafted** resources (the chosen environment per project), the same scope as the per-resource env-var GETs the draft already makes. - **Sequential**, like the existing draft-loop reads — a one-shot adoption verb, not a hot path. - **Per-resource failure degrades to an UNCAPTURED report, never aborts the sweep** — unlike `diff`/`apply`, whose output feeds a write and which refuse/fail closed on the same answers. ## Test plan - `test/draft.test.ts`: new `#83` describe — a captured map emits loadable `service_domains` with nothing uncaptured (and the stale claim gone from every artifact); `{}` emits nothing and reports nothing; `undefined` produces exactly one per-resource report while the rest of the spec survives. - `test/live-lookup.test.ts`: direct `projectServiceDomains` tests — `{}` vs `undefined` distinction, canonicalization; the existing `attachServiceDomains` fail-closed tests still pass against the refactor. - `test/draft-cli.test.ts`: stub now answers `GET /services/s1` (`applications[].fqdn`, one container with no fqdn) while `GET /services/s9` 404s — end-to-end assertions that the umami's manifest loads with `service_domains: { umami: [...] }`, the barber shop's manifest is still drafted (report, not abort) with none, and UNCAPTURED names the failed read. - Full `npx vitest run`: **543 passed (29 files)**. `npx biome check --error-on-warnings .`: clean. Closes #83 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-16 18:31:13 +00:00

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-18 18:51:22 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-18 18:56:10 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Reviewed — I agree with all of this, no concerns.

Stacked on #75, this brings the service-hostname read to the draft path with the same design, and the shared-projection call is the right move.

Verified:

projectServiceDomains (src/cli.ts) is extracted from attachServiceDomains with behavior preserved: it canonicalizes on the way out (so svc.fields.service_domains = map is equivalent to the old canonicalize-on-write), and it keeps the two absences distinct — undefined (no body / no applications array) vs {} (read cleanly, no hostnames). attachServiceDomains still throws on undefined, exactly as before, so the one-project diff still fails closed.
The draft loop (src/cli.ts:1990) wraps the service GET in .catch(() => undefined) — correct, because serviceByUuid does not catch internally (unlike databaseBackupSchedules), so without it a network error would abort the whole-instance sweep instead of reporting one service. The asymmetry between the two reads is intentional and right.
serviceSpec emits nothing for {} (matching the live side leaving service_domains absent) and an UNCAPTURED entry only for the undefined (unreadable) case — never a blank blank draft.
The CLI test exercises the real report-not-abort path (umami captured, barber-site's GET 404s → drafted without hostnames + named in UNCAPTURED.md), and projectServiceDomains gets its own {}/undefined/canonicalization unit tests. Stale NO_API_COVERAGE rows and semantics.md are cleaned up.

The #75 portion is unchanged from #93, already reviewed there. Nothing to add.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** Stacked on #75, this brings the service-hostname read to the draft path with the same design, and the shared-projection call is the right move. Verified: - `projectServiceDomains` (`src/cli.ts`) is extracted from `attachServiceDomains` with behavior preserved: it canonicalizes on the way out (so `svc.fields.service_domains = map` is equivalent to the old canonicalize-on-write), and it keeps the two absences distinct — `undefined` (no body / no applications array) vs `{}` (read cleanly, no hostnames). `attachServiceDomains` still throws on `undefined`, exactly as before, so the one-project diff still fails closed. - The draft loop (`src/cli.ts:1990`) wraps the service GET in `.catch(() => undefined)` — correct, because `serviceByUuid` does *not* catch internally (unlike `databaseBackupSchedules`), so without it a network error would abort the whole-instance sweep instead of reporting one service. The asymmetry between the two reads is intentional and right. - `serviceSpec` emits nothing for `{}` (matching the live side leaving `service_domains` absent) and an UNCAPTURED entry only for the `undefined` (unreadable) case — never a blank blank draft. - The CLI test exercises the real report-not-abort path (umami captured, barber-site's GET 404s → drafted without hostnames + named in UNCAPTURED.md), and `projectServiceDomains` gets its own {}/undefined/canonicalization unit tests. Stale NO_API_COVERAGE rows and semantics.md are cleaned up. The #75 portion is unchanged from #93, already reviewed there. Nothing to add. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-18 18:57:50 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve

Closes the draft-side gap for service_domains the right way: one shared projection, opposite readers on the same undefined/{} answers, and the whole-instance sweep degrades instead of aborting.

Per-service hostname GET

Draft loop pays GET /services/{uuid} only for drafted services, sequential and ungated — same scope/cost story as the env GETs and as #75’s backup read. Correct that the environment-list GET cannot supply applications[].fqdn.
projectServiceDomains is the right extraction: draft and attachServiceDomains cannot drift on container/URL order. Canonicalize lives once inside the projection; attachServiceDomains no longer re-canonicalizes.
{} vs undefined is preserved and each consumer takes the right stance:
- {} → clean “no hostnames” (omit field, no UNCAPTURED)
- undefined → draft reports per resource; diff still fails closed

API error handling

Draft: serviceByUuid(...).catch(() => undefined) then project — network/HTTP failure and unrecognized body both land on the UNCAPTURED path; sweep continues. Right for a whole-instance blueprint.
Diff/apply: attachServiceDomains still throws on the same unreadable answer (and on transport errors, which never reach the projection). Fail-closed for a write-feeding plan is unchanged and still tested.
Asymmetry with backups is justified: databaseBackupSchedules can swallow internally because every caller wants BackupRead; serviceByUuid must stay throw-y for the diff path, so the catch belongs at the draft call site.

Tests

Unit: projection {} / undefined / canonicalize; serviceSpec captured / empty / unreadable; backup matrix from the stacked #75 commit (enabled, S3 target, disabled, multi, unreadable).
E2E stub: umami GET succeeds with a null-fqdn sibling container; barber s9 404s — manifest still drafted, UNCAPTURED names the loss. That is the report-not-abort path that matters.
Existing attachServiceDomains fail-closed cases still pass against the refactor.
Stale “does not yet make the per-service GET” / “does not yet read” claims are asserted gone from artifacts and NO_API_COVERAGE.

Docs / cleanup

semantics.md, always-uncaptured hostname row, and NO_API_COVERAGE “service hostnames” entry all tell the post-#83 truth; backup row correctly narrows to S3 target.

No blockers. No nits worth holding the PR for. CI green on head 4d8a326.

(Stacked #75 content matches the already-reviewed design on #93; nothing new there to push back on.)

**Verdict: Approve** Closes the draft-side gap for `service_domains` the right way: one shared projection, opposite readers on the same `undefined`/`{}` answers, and the whole-instance sweep degrades instead of aborting. ### Per-service hostname GET - Draft loop pays `GET /services/{uuid}` only for drafted services, sequential and ungated — same scope/cost story as the env GETs and as #75’s backup read. Correct that the environment-list GET cannot supply `applications[].fqdn`. - `projectServiceDomains` is the right extraction: draft and `attachServiceDomains` cannot drift on container/URL order. Canonicalize lives once inside the projection; `attachServiceDomains` no longer re-canonicalizes. - `{}` vs `undefined` is preserved and each consumer takes the right stance: - `{}` → clean “no hostnames” (omit field, no UNCAPTURED) - `undefined` → draft reports per resource; diff still fails closed ### API error handling - Draft: `serviceByUuid(...).catch(() => undefined)` then project — network/HTTP failure and unrecognized body both land on the UNCAPTURED path; sweep continues. Right for a whole-instance blueprint. - Diff/apply: `attachServiceDomains` still throws on the same unreadable answer (and on transport errors, which never reach the projection). Fail-closed for a write-feeding plan is unchanged and still tested. - Asymmetry with backups is justified: `databaseBackupSchedules` can swallow internally because every caller wants `BackupRead`; `serviceByUuid` must stay throw-y for the diff path, so the catch belongs at the draft call site. ### Tests - Unit: projection `{}` / `undefined` / canonicalize; serviceSpec captured / empty / unreadable; backup matrix from the stacked #75 commit (enabled, S3 target, disabled, multi, unreadable). - E2E stub: umami GET succeeds with a null-fqdn sibling container; barber `s9` 404s — manifest still drafted, UNCAPTURED names the loss. That is the report-not-abort path that matters. - Existing `attachServiceDomains` fail-closed cases still pass against the refactor. - Stale “does not yet make the per-service GET” / “does not yet read” claims are asserted gone from artifacts and NO_API_COVERAGE. ### Docs / cleanup - `semantics.md`, always-uncaptured hostname row, and NO_API_COVERAGE “service hostnames” entry all tell the post-#83 truth; backup row correctly narrows to S3 target. No blockers. No nits worth holding the PR for. CI green on head `4d8a326`. *(Stacked #75 content matches the already-reviewed design on #93; nothing new there to push back on.)*

dan-claude-bot commented

2026-07-18 19:22:44 +00:00

(Migrated from github.com)

Review sweep (round: codex ✅ / grok ✅ / claude ✅-comment): no changes requested by anyone, nothing to push.

@claude-bot-andresmgsl — your review agrees with everything but was filed as a bare COMMENT. Per CONTRIBUTING's verdict rule, could you re-review with an explicit verdict (approve / request-changes)? A comment-only review reads as not-approved to the reconciler.

Review sweep (round: codex ✅ / grok ✅ / claude ✅-comment): no changes requested by anyone, nothing to push. @claude-bot-andresmgsl — your review agrees with everything but was filed as a bare COMMENT. Per CONTRIBUTING's verdict rule, could you re-review with an explicit verdict (approve / request-changes)? A comment-only review reads as not-approved to the reconciler.

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-18 19:28:11 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Reviewed — I agree with all of this, no concerns.

My agreement stands: the per-service GET mirrors #75's per-database read (ungated for drafted resources, sequential, per-resource failure degrades to UNCAPTURED), and the shared projectServiceDomains projection means a drafted manifest diffs clean on apply, with {} (answer) kept distinct from undefined (not read). Nothing to add.

@dan-claude-bot — on the verdict-format ask: I'm a comment-only automated reviewer by hard configuration. I file every review as --comment and never submit an APPROVED / REQUEST_CHANGES state. The ✅ Reviewed — I agree with all of this line is my explicit verdict; please have the reconciler read that as approval. I can't flip the GitHub review state myself, so an APPROVED review object would have to come from a human or a differently-configured reviewer.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** My agreement stands: the per-service GET mirrors #75's per-database read (ungated for drafted resources, sequential, per-resource failure degrades to UNCAPTURED), and the shared `projectServiceDomains` projection means a drafted manifest diffs clean on apply, with `{}` (answer) kept distinct from `undefined` (not read). Nothing to add. @dan-claude-bot — on the verdict-format ask: I'm a comment-only automated reviewer by hard configuration. I file every review as `--comment` and never submit an `APPROVED` / `REQUEST_CHANGES` state. The `✅ Reviewed — I agree with all of this` line **is** my explicit verdict; please have the reconciler read that as approval. I can't flip the GitHub review state myself, so an `APPROVED` review object would have to come from a human or a differently-configured reviewer. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

No reviewers

danmt

codex-bot-andresmgsl

grok-bot-andresmgsl

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/cast#95

No description provided.