feat(cli): add [deployments] subdomain to config-as-code#125
feat(cli): add [deployments] subdomain to config-as-code#125tonychang04 wants to merge 1 commit into
Conversation
Mirrors the SMTP/auth pattern: per-section capability probe gates the
write so old/self-host backends never receive a PUT they can't honor.
Wire:
- [deployments] subdomain = "my-app" → PUT /api/deployments/slug { slug: "my-app" }
- [deployments] subdomain = "" → PUT { slug: null } (clear)
- absent section → default-keep (live state preserved)
Version skew handled in metadataSupports() via presence-only probe of the
new metadata slice exposed in InsForge#1259:
- pre-#1259 backend (any topology) → no `deployments` key → skip with named warning
- post-#1259 self-host → getConfigMetadata returns undefined → same skip
- post-#1259 cloud (slug null/set) → slice present → proceed
Empty-string in TOML normalizes to null in the diff/apply layer (TOML
lacks a null literal). Export emits [deployments] only when a slug is
actually set, to avoid implying "clear on apply" on a fresh export.
CLI bumped 0.1.76 → 0.1.77.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR extends the CLI's config-as-code system to support the ChangesDeployments Subdomain Config Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/config-capabilities.ts`:
- Around line 53-56: The current capability check that marks deployments
supported uses the expression "raw?.deployments !== undefined && raw.deployments
!== null && typeof raw.deployments === 'object'", which treats arrays as valid;
update this to explicitly exclude arrays (e.g., replace the typeof check with a
plain-object guard such as "!Array.isArray(raw.deployments) && typeof
raw.deployments === 'object'" or call a small helper
isPlainObject(raw.deployments)) so that raw.deployments (referenced in the
boolean expression) must be a non-null plain object before reporting the
capability as supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcb1dfa8-7393-4877-b0c1-8ffad3f78b53
📒 Files selected for processing (12)
package.jsonsrc/commands/config/apply.test.tssrc/commands/config/apply.tssrc/commands/config/export.test.tssrc/commands/config/export.tssrc/lib/config-capabilities.test.tssrc/lib/config-capabilities.tssrc/lib/config-diff.tssrc/lib/config-format.tssrc/lib/config-schema.tssrc/lib/config-toml.test.tssrc/lib/config-toml.ts
| raw?.deployments !== undefined && | ||
| raw.deployments !== null && | ||
| typeof raw.deployments === 'object' | ||
| ); |
There was a problem hiding this comment.
Treat non-plain deployments values as unsupported.
On Line 55, typeof raw.deployments === 'object' also accepts arrays, which can incorrectly mark capability as supported on malformed metadata.
Suggested patch
return (
raw?.deployments !== undefined &&
raw.deployments !== null &&
- typeof raw.deployments === 'object'
+ typeof raw.deployments === 'object' &&
+ !Array.isArray(raw.deployments)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raw?.deployments !== undefined && | |
| raw.deployments !== null && | |
| typeof raw.deployments === 'object' | |
| ); | |
| raw?.deployments !== undefined && | |
| raw.deployments !== null && | |
| typeof raw.deployments === 'object' && | |
| !Array.isArray(raw.deployments) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/config-capabilities.ts` around lines 53 - 56, The current capability
check that marks deployments supported uses the expression "raw?.deployments !==
undefined && raw.deployments !== null && typeof raw.deployments === 'object'",
which treats arrays as valid; update this to explicitly exclude arrays (e.g.,
replace the typeof check with a plain-object guard such as
"!Array.isArray(raw.deployments) && typeof raw.deployments === 'object'" or call
a small helper isPlainObject(raw.deployments)) so that raw.deployments
(referenced in the boolean expression) must be a non-null plain object before
reporting the capability as supported.
Greptile SummaryThis PR wires
Confidence Score: 4/5Safe to merge; all four version-skew permutations are covered by tests and the implementation logic is sound. The core apply/export logic, capability gating, and empty-string-to-null normalization are all correct and well-tested. The only findings are three doc comments that were not updated to reflect that deployments is now a wired section rather than a future one. src/lib/config-diff.ts, src/lib/config-schema.ts, and src/lib/config-toml.ts each carry a stale comment worth cleaning up before the next contributor adds a new config section. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Backend as Backend /api
Note over User,Backend: config export
User->>CLI: insforge config export
CLI->>Backend: GET /api/metadata
Backend-->>CLI: "{ auth: {...}, deployments?: { customSlug } }"
alt deployments slice present AND customSlug is non-null string
CLI->>CLI: "config.deployments = { subdomain: customSlug }"
CLI-->>User: "writes [deployments] subdomain = "..." to TOML"
else deployments slice present, customSlug is null
CLI-->>User: "omits [deployments] section (no-slug = default URL)"
else "deployments slice absent (self-host / pre-#1259)"
CLI-->>User: omits [deployments], adds deployments.subdomain to skipped[]
end
Note over User,Backend: config apply
User->>CLI: insforge config apply
CLI->>Backend: GET /api/metadata
Backend-->>CLI: "{ auth: {...}, deployments?: { customSlug } }"
CLI->>CLI: build live config, run diffConfig(live, file)
loop for each DiffChange
alt "metadataSupports(raw, change) == false"
CLI-->>User: skip with named warning
else deployments.subdomain change
CLI->>Backend: "PUT /api/deployments/slug { slug: value | null }"
Backend-->>CLI: 200 OK or 409 Slug taken
end
end
CLI-->>User: "{ applied: [...], skipped: [...] }"
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code Review — feat(cli): add [deployments] subdomain to config-as-code
Summary. Solid, well-scoped addition that wires insforge config export/apply to manage custom deployment slugs, with correct capability gating for version-skew safety and comprehensive test coverage across all documented rollout permutations.
Requirements context
No /docs/superpowers/ directory exists in this repo — assessing against the PR description and the linked PR InsForge#1259 as the source of intent. The implementation matches the stated spec on all four version-skew cases (pre-#1259 / post-#1259 self-host / cloud+no-slug / cloud+slug-set).
Findings
Critical
(none)
Suggestion
[Functionality] Missing test for 409 "slug already taken" error path — apply.ts:144-153, apply.test.ts
The PR description explicitly calls out: "A 409 from a taken slug surfaces as CLIError("Slug is already taken") via ossFetch, and aborts the apply mid-batch." This is a distinct, user-visible failure mode that users will hit in practice (slug collisions are common), yet there's no test covering it. The existing ossFetchMock infrastructure in apply.test.ts should make it straightforward to mock a 409 response and assert the error message propagates. Worth adding before the manual E2E pass.
Information
[Software engineering] Stale JSDoc in config-diff.ts:36
v1 scope: auth.allowed_redirect_urls only. Default-keep for absent fields
Now covers deployments.subdomain too. Minor doc drift.
[Software engineering] Stale comment in config-toml.ts:17
Section ordering is deterministic (project_id → auth)
Three sections are now serialized. Should read project_id → auth → deployments.
[Functionality] liveDeployments = live.deployments ?? {} double-gate pattern — config-diff.ts:66-68
When the backend doesn't expose deployments, live.deployments is undefined, so liveDeployments becomes {} and fromV becomes null. This means diffConfig still emits a change (null → TOML value) even when the backend lacks the slice. The capability gate in apply.ts:97 catches it correctly — but the path is subtle. The existing comment partially explains it; a one-line note linking the two gates (// capability gate in apply.ts filters this out before it reaches applyChange) would help the next reader avoid the same double-take.
This is purely informational — the logic is correct and safe.
Verdict
Approved (informational; explicit GitHub approval is a separate human action). No blocking issues found. The one Suggestion (409 test) is worth addressing before or after the manual E2E, but it doesn't block merge given the rest of the coverage is thorough.
2 participants
Summary
Adds
[deployments] subdomain = \"...\"toinsforge.toml, completing the config-as-code surface for the deployment slug feature (InsForge#1259, already merged).insforge config exportpulls the live custom slug from/api/metadataand emits[deployments]when one is setinsforge config applydiffs the TOML against live state andPUTs/api/deployments/slugfor changesVersion-skew handling
The capability probe is presence-only on the new
metadata.deploymentsslice from #1259, so all three rollout permutations land in the right place:deploymentskeyskipped[]with named warning, no PUTgetConfigMetadatareturnsundefined→ key omitteddeployments: { customSlug: null }deployments: { customSlug: \"foo\" }A 409 from a taken slug surfaces as
CLIError(\"Slug is already taken\")viaossFetch, and aborts the apply mid-batch — earlier successful changes in the same run stay applied.TOML semantics
subdomain = \"my-app\"→ set the slugsubdomain = \"\"→ clear the slug (TOML has no null; the diff layer normalizes "" →nullfor the PUT body)Export deliberately omits
[deployments]when no slug is set so a fresh export doesn't imply "clear on apply."Test plan
npm test— 287 pass, 0 failnpm run lint— cleanVersion bumped 0.1.76 → 0.1.77.
🤖 Generated with Claude Code
Summary by cubic
Add
[deployments] subdomaintoinsforge.tomland wireinsforge config export/applyto manage deployment slugs. Lets users set or clear custom subdomains from code, with safe gating on older or self-hosted backends./api/metadata.deploymentsand emits[deployments]only when a slug exists./api/deployments/slugwith{ slug: string | null }.subdomain = ""means clear (normalized tonull); absent section keeps live state.metadata.deploymentsis missing; change is reported as skipped.Written for commit 54ea7ee. Summary will update on new commits.
Note
Add
[deployments].subdomainfield to config-as-code export and apply[deployments]section to the config schema, TOML serialization, diff, and format utilities, withsubdomainas the only field.subdomainwhen the backend exposes the deployments slice and a custom slug is set; it recordsdeployments.subdomainas skipped when the slice is absent.PUT /api/deployments/slugwith{ slug: string | null }to apply changes, treating an empty string in TOML as a null (clear) operation.metadataSupportsgates this field on the presence of thedeploymentsobject in raw metadata, so older backends silently skip it.Macroscope summarized 54ea7ee.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores