feat(cli): add [auth.smtp] and [deployments] sections to config-as-code#123
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds auth.smtp and deployments.subdomain handling across validation, TOML I/O, diffing/formatting, secret resolution, export/apply commands, and tests; bumps package version. ChangesSMTP & deployments config-as-code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/config/apply.ts (1)
102-113:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreflight SMTP secret resolution before applying any change to prevent partial applies.
With mixed batches,
auth.allowed_redirect_urlscan be PUT first, then fail on SMTP secret resolution later. That leaves the run partially applied.Suggested direction
+// 1) Pre-resolve all secret refs for planned SMTP changes before any PUT. +const resolvedSmtpPasswords = new Map<DiffChange, string>(); +for (const change of result.changes) { + if (change.section === 'auth.smtp' && change.passwordEnvRef) { + const value = await resolveEnvRef(`env(${change.passwordEnvRef})`, 'auth.smtp.password'); + resolvedSmtpPasswords.set(change, value); + } +} + // 2) Apply changes only after preflight succeeds. for (const change of result.changes) { ... - await applyChange(change); + await applyChange(change, resolvedSmtpPasswords.get(change)); applied.push(change); }And update
applyChangeto accept an optional pre-resolved password and avoid resolving during mutation.Also applies to: 186-193
🤖 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/commands/config/apply.ts` around lines 102 - 113, Preflight resolution of SMTP secrets for every change before any mutation: iterate result.changes and for each change whose path may require secrets (e.g., SMTP/password fields) resolve the secret ahead of time and collect failures into skipped so no partial applies occur; then call applyChange(change, preResolvedSecret?) instead of resolving inside applyChange. Update the applyChange function signature to accept an optional preResolvedPassword (or credentials object) and ensure it uses that value if provided and does not attempt resolution during the mutation. Apply the same preflight logic to the second batch processing block that also iterates result.changes (the other loop around applyChange) so both places resolve SMTP secrets before making any PUTs and mark failing resolutions as skipped.
🧹 Nitpick comments (1)
src/lib/config-secrets.test.ts (1)
83-137: ⚡ Quick winAdd a malformed-JSON lookup test for
resolveEnvRef.You’re asserting major failure modes already; adding a
200+ invalid JSON case will lock in stable error behavior for backend/proxy anomalies.Proposed test case
describe('resolveEnvRef', () => { + it('throws SECRET_LOOKUP_FAILED when 200 response is not valid JSON', async () => { + ossFetchMock.mockResolvedValueOnce(new Response('not-json', { status: 200 })); + await expect( + resolveEnvRef('env(SMTP_PASSWORD)', 'auth.smtp.password'), + ).rejects.toMatchObject({ code: 'SECRET_LOOKUP_FAILED' }); + });🤖 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-secrets.test.ts` around lines 83 - 137, Add a test inside the describe('resolveEnvRef') block that simulates a 200 response with malformed JSON from ossFetchMock (e.g., ossFetchMock.mockResolvedValueOnce(new Response('invalid', { status: 200 })) ) when calling resolveEnvRef('env(BROKEN)', 'auth.smtp.password') and assert it rejects with the same error shape used for HTTP lookup failures (e.g., toMatchObject({ code: 'SECRET_LOOKUP_FAILED' })); reference resolveEnvRef, ossFetchMock and the existing SECRET_LOOKUP_FAILED assertions to mirror the error expectation and placement alongside the other cases.
🤖 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/commands/config/export.ts`:
- Around line 83-84: The guard using "'smtpConfig' in authSlice" can throw if
authSlice is not an object; update the conditional in export command (the block
referencing authSlice and smtpConfig) to first verify authSlice is a non-null
object (e.g., typeof authSlice === 'object' && authSlice !== null) before using
the 'in' operator and accessing authSlice.smtpConfig, so malformed metadata
yields a controlled CLI error path rather than a runtime TypeError.
In `@src/lib/config-format.ts`:
- Around line 51-53: The note rendering incorrectly nests env(...) when
c.passwordEnvRef already includes env(...) — normalize c.passwordEnvRef before
pushing the line: check and strip any surrounding "env(" and trailing ")" (or
detect if it already starts with "env(") so you always produce a single env(...)
wrapper; update the code around the lines.push call that references
c.passwordEnvRef to use the normalized value (e.g., compute a local
normalizedPasswordRef and then use lines.push(` (password force-resent from
env(${normalizedPasswordRef}))`)) so duplicate env(...) is avoided.
In `@src/lib/config-schema.ts`:
- Around line 113-118: The SMTP port validation currently only checks for
integer type in the block handling 'port' and allows out-of-range values; update
the check in the same conditional (the code that assigns out.port) to validate
that obj.port is an integer within the TCP port range 1..65535 and throw
ConfigValidationError('auth.smtp.port', '<message>') when it's not; keep the
existing type/integer validation but extend it to enforce the range before
assigning out.port so invalid values like -1 or 70000 are rejected.
In `@src/lib/config-secrets.ts`:
- Around line 130-131: The JSON parsing for the secret response is unguarded:
wrap the call to res.json() in a try/catch and validate HTTP status before
trusting the body so parsing errors or non-2xx responses produce a consistent
CLIError; specifically, in the function that handles the fetch response (the
block using res and const body = (await res.json()) as { value?: string }),
first check res.ok and throw a CLIError with context if not ok, then try to
parse res.json() inside a try/catch and on any exception or if body.value is not
a non-empty string throw a CLIError describing the failure to retrieve/parse the
secret (include res.status/res.statusText or the caught error.message for
debugging).
In `@src/lib/config-toml.ts`:
- Around line 56-61: The emitted guidance hardcodes "SMTP_PASSWORD" even though
smtp.password contains the actual env reference; update the logic that builds
the comment (where lines.push is called for smtp.password in config-toml.ts) to
extract the secret/env name from smtp.password (the env(...) string) and
interpolate that name into the comment instead of the literal SMTP_PASSWORD,
falling back to a generic message if extraction fails. Ensure you reference and
parse smtp.password (the env(...) value) to compute the secret name used in the
comment and then push the comment using that extracted name.
---
Outside diff comments:
In `@src/commands/config/apply.ts`:
- Around line 102-113: Preflight resolution of SMTP secrets for every change
before any mutation: iterate result.changes and for each change whose path may
require secrets (e.g., SMTP/password fields) resolve the secret ahead of time
and collect failures into skipped so no partial applies occur; then call
applyChange(change, preResolvedSecret?) instead of resolving inside applyChange.
Update the applyChange function signature to accept an optional
preResolvedPassword (or credentials object) and ensure it uses that value if
provided and does not attempt resolution during the mutation. Apply the same
preflight logic to the second batch processing block that also iterates
result.changes (the other loop around applyChange) so both places resolve SMTP
secrets before making any PUTs and mark failing resolutions as skipped.
---
Nitpick comments:
In `@src/lib/config-secrets.test.ts`:
- Around line 83-137: Add a test inside the describe('resolveEnvRef') block that
simulates a 200 response with malformed JSON from ossFetchMock (e.g.,
ossFetchMock.mockResolvedValueOnce(new Response('invalid', { status: 200 })) )
when calling resolveEnvRef('env(BROKEN)', 'auth.smtp.password') and assert it
rejects with the same error shape used for HTTP lookup failures (e.g.,
toMatchObject({ code: 'SECRET_LOOKUP_FAILED' })); reference resolveEnvRef,
ossFetchMock and the existing SECRET_LOOKUP_FAILED assertions to mirror the
error expectation and placement alongside the other cases.
🪄 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: c19f54ee-fe1e-4af7-91cf-bbe4d02adda8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
package.jsonsrc/commands/config/apply.test.tssrc/commands/config/apply.tssrc/commands/config/export.test.tssrc/commands/config/export.tssrc/lib/config-capabilities.tssrc/lib/config-diff.test.tssrc/lib/config-diff.tssrc/lib/config-format.tssrc/lib/config-schema.tssrc/lib/config-secrets.test.tssrc/lib/config-secrets.tssrc/lib/config-toml.test.tssrc/lib/config-toml.ts
| if (c.passwordEnvRef) { | ||
| lines.push(` (password force-resent from env(${c.passwordEnvRef}))`); | ||
| } |
There was a problem hiding this comment.
Normalize passwordEnvRef before rendering the force-resend note.
Current formatting can produce env(env(...)) depending on what passwordEnvRef carries.
Proposed fix
if (c.passwordEnvRef) {
- lines.push(` (password force-resent from env(${c.passwordEnvRef}))`);
+ const ref = c.passwordEnvRef.startsWith('env(')
+ ? c.passwordEnvRef
+ : `env(${c.passwordEnvRef})`;
+ lines.push(` (password force-resent from ${ref})`);
}🤖 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-format.ts` around lines 51 - 53, The note rendering
incorrectly nests env(...) when c.passwordEnvRef already includes env(...) —
normalize c.passwordEnvRef before pushing the line: check and strip any
surrounding "env(" and trailing ")" (or detect if it already starts with "env(")
so you always produce a single env(...) wrapper; update the code around the
lines.push call that references c.passwordEnvRef to use the normalized value
(e.g., compute a local normalizedPasswordRef and then use lines.push(`
(password force-resent from env(${normalizedPasswordRef}))`)) so duplicate
env(...) is avoided.
| const body = (await res.json()) as { value?: string }; | ||
| if (typeof body.value !== 'string' || body.value.length === 0) { |
There was a problem hiding this comment.
Guard JSON parsing for successful secret lookups.
A 2xx response with invalid/empty JSON will currently throw an untyped parse error instead of a consistent CLIError, which makes config apply failure handling brittle.
Proposed fix
- const body = (await res.json()) as { value?: string };
+ let body: { value?: string };
+ try {
+ body = (await res.json()) as { value?: string };
+ } catch {
+ throw new CLIError(
+ `failed to resolve env(${secretName}) for ${fieldPath}: invalid response body`,
+ 1,
+ 'SECRET_LOOKUP_FAILED',
+ );
+ }📝 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.
| const body = (await res.json()) as { value?: string }; | |
| if (typeof body.value !== 'string' || body.value.length === 0) { | |
| let body: { value?: string }; | |
| try { | |
| body = (await res.json()) as { value?: string }; | |
| } catch { | |
| throw new CLIError( | |
| `failed to resolve env(${secretName}) for ${fieldPath}: invalid response body`, | |
| 1, | |
| 'SECRET_LOOKUP_FAILED', | |
| ); | |
| } | |
| if (typeof body.value !== 'string' || body.value.length === 0) { |
🤖 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-secrets.ts` around lines 130 - 131, The JSON parsing for the
secret response is unguarded: wrap the call to res.json() in a try/catch and
validate HTTP status before trusting the body so parsing errors or non-2xx
responses produce a consistent CLIError; specifically, in the function that
handles the fetch response (the block using res and const body = (await
res.json()) as { value?: string }), first check res.ok and throw a CLIError with
context if not ok, then try to parse res.json() inside a try/catch and on any
exception or if body.value is not a non-empty string throw a CLIError describing
the failure to retrieve/parse the secret (include res.status/res.statusText or
the caught error.message for debugging).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/config/export.ts">
<violation number="1" location="src/commands/config/export.ts:83">
P2: Guard `authSlice` as an object before using the `in` operator to avoid runtime `TypeError` on malformed metadata responses.</violation>
</file>
<file name="src/lib/config-secrets.ts">
<violation number="1" location="src/lib/config-secrets.ts:102">
P1: `resolveEnvRef` relies on `ossFetch` returning 404 responses, but `ossFetch` throws on non-2xx, so the 404/`!res.ok` branches here are unreachable and missing secrets are misclassified as `SECRET_LOOKUP_FAILED`.</violation>
</file>
<file name="src/lib/config-toml.ts">
<violation number="1" location="src/lib/config-toml.ts:60">
P3: Avoid hardcoding `SMTP_PASSWORD` in the emitted guidance so instructions don't conflict with a custom `env(...)` secret name.</violation>
</file>
<file name="src/lib/config-schema.ts">
<violation number="1" location="src/lib/config-schema.ts:114">
P2: Validate `auth.smtp.port` against the valid TCP range (1-65535); integer-only validation currently allows invalid ports like -1 and 70000.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
ossFetch throws on any non-2xx response, swallowing the status code. The
resolveEnvRef path that previously checked res.status === 404 never fired
— missing secrets always landed in SECRET_LOOKUP_FAILED.
Recover the signal from the thrown error's message instead (the backend's
NOT_FOUND path uses "Secret not found: NAME" wording). Verified against
live local backend on the SMTP branch:
$ insforge config apply # with env(NONEXISTENT) in TOML
code: SECRET_NOT_FOUND
msg: auth.smtp.password references env(NONEXISTENT) but no such secret
exists.
fix: insforge secrets add NONEXISTENT "<value>"
Unit tests updated to mock ossFetch as throwing (matches real behavior).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/commands/config/apply.test.ts (1)
24-29: ⚡ Quick winMake missing-secret mock error shape closer to real
ossFetchfailures.The mocked throw message (
Secret not found: ...) is a bit synthetic compared to non-2xx throw paths. Aligning it with the real thrown format (including 404 signal) will better guard against regressions inresolveEnvReferror parsing.🤖 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/commands/config/apply.test.ts` around lines 24 - 29, The mock in src/commands/config/apply.test.ts should throw an error shaped like real ossFetch non-2xx failures so resolveEnvRef can parse the 404 signal; change the thrown Error in the test (currently "Secret not found: ${key}") to include the HTTP status indicator (e.g. include "404" or "status: 404" and a Not Found phrase) so error-parsing paths in resolveEnvRef and any ossFetch-related logic will behave the same as real failures.
🤖 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/commands/config/apply.test.ts`:
- Around line 93-97: The shared test state variable nextMetadataResponse is not
reset between tests; update the beforeEach block (the one that calls
vi.clearAllMocks(), secretsStore.clear(), and sets tmp) to also reset
nextMetadataResponse (e.g. set to undefined/null or its initial value) so each
test starts with a clean metadata state and cannot inherit prior test responses.
In `@src/lib/config-secrets.test.ts`:
- Around line 124-127: The test in src/lib/config-secrets.test.ts that currently
does ossFetchMock.mockResolvedValueOnce(new Response('boom', { status: 500 }))
should instead mock the fetch as a rejected promise to mirror runtime behavior;
replace that resolved Response with ossFetchMock.mockRejectedValueOnce(new
Error('boom')) (or an Error-like rejection) so the test exercising the
SECRET_LOOKUP_FAILED mapping uses a thrown error like the real ossFetch and
accurately reflects the code paths in the functions that handle ossFetch
failures.
---
Nitpick comments:
In `@src/commands/config/apply.test.ts`:
- Around line 24-29: The mock in src/commands/config/apply.test.ts should throw
an error shaped like real ossFetch non-2xx failures so resolveEnvRef can parse
the 404 signal; change the thrown Error in the test (currently "Secret not
found: ${key}") to include the HTTP status indicator (e.g. include "404" or
"status: 404" and a Not Found phrase) so error-parsing paths in resolveEnvRef
and any ossFetch-related logic will behave the same as real failures.
🪄 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: 715005df-00aa-491b-9ed3-213fc682fdd2
📒 Files selected for processing (3)
src/commands/config/apply.test.tssrc/lib/config-secrets.test.tssrc/lib/config-secrets.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/config-secrets.ts
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| secretsStore.clear(); | ||
| tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-')); | ||
| }); |
There was a problem hiding this comment.
Reset nextMetadataResponse in beforeEach to prevent cross-test coupling.
nextMetadataResponse is shared mutable state but isn’t reset per test. Add a reset in beforeEach so future tests can’t accidentally inherit previous metadata.
Suggested patch
beforeEach(() => {
vi.clearAllMocks();
secretsStore.clear();
+ nextMetadataResponse = {};
tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-'));
});📝 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.
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| secretsStore.clear(); | |
| tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-')); | |
| }); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| secretsStore.clear(); | |
| nextMetadataResponse = {}; | |
| tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-')); | |
| }); |
🤖 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/commands/config/apply.test.ts` around lines 93 - 97, The shared test
state variable nextMetadataResponse is not reset between tests; update the
beforeEach block (the one that calls vi.clearAllMocks(), secretsStore.clear(),
and sets tmp) to also reset nextMetadataResponse (e.g. set to undefined/null or
its initial value) so each test starts with a clean metadata state and cannot
inherit prior test responses.
| it('throws SECRET_LOOKUP_FAILED on a non-404 HTTP error', async () => { | ||
| ossFetchMock.mockResolvedValueOnce( | ||
| new Response('boom', { status: 500 }), | ||
| ); |
There was a problem hiding this comment.
Mock non-404 failures as rejected promises to mirror runtime behavior.
This test currently simulates HTTP 500 via a resolved Response, but runtime behavior (per this PR’s own contract) is that ossFetch throws on non-2xx. Keeping this as a rejection makes the test representative and avoids false confidence in SECRET_LOOKUP_FAILED mapping.
Suggested test adjustment
it('throws SECRET_LOOKUP_FAILED on a non-404 HTTP error', async () => {
- ossFetchMock.mockResolvedValueOnce(
- new Response('boom', { status: 500 }),
- );
+ ossFetchMock.mockRejectedValueOnce(
+ new Error('Request failed with status 500'),
+ );
await expect(
resolveEnvRef('env(WHATEVER)', 'auth.smtp.password'),
).rejects.toMatchObject({ code: 'SECRET_LOOKUP_FAILED' });
});📝 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.
| it('throws SECRET_LOOKUP_FAILED on a non-404 HTTP error', async () => { | |
| ossFetchMock.mockResolvedValueOnce( | |
| new Response('boom', { status: 500 }), | |
| ); | |
| it('throws SECRET_LOOKUP_FAILED on a non-404 HTTP error', async () => { | |
| ossFetchMock.mockRejectedValueOnce( | |
| new Error('Request failed with status 500'), | |
| ); | |
| await expect( | |
| resolveEnvRef('env(WHATEVER)', 'auth.smtp.password'), | |
| ).rejects.toMatchObject({ code: 'SECRET_LOOKUP_FAILED' }); | |
| }); |
🤖 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-secrets.test.ts` around lines 124 - 127, The test in
src/lib/config-secrets.test.ts that currently does
ossFetchMock.mockResolvedValueOnce(new Response('boom', { status: 500 })) should
instead mock the fetch as a rejected promise to mirror runtime behavior; replace
that resolved Response with ossFetchMock.mockRejectedValueOnce(new
Error('boom')) (or an Error-like rejection) so the test exercising the
SECRET_LOOKUP_FAILED mapping uses a thrown error like the real ossFetch and
accurately reflects the code paths in the functions that handle ossFetch
failures.
Mirrors the existing guard on the allowed_redirect_urls branch — without it, a malformed metadata response (e.g. auth: "x") would TypeError on 'smtpConfig' in authSlice instead of cleanly landing in skipped[]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code Review — feat(cli): add [auth.smtp] section to config-as-code with env() password resolution
Summary: Solid, well-scoped addition of SMTP config-as-code. The security model (env()-only passwords, opaque plan output, pre-flight secret resolution, no plaintext in logs) is correct and the test coverage is thorough. Four non-blocking items below.
Requirements context: No /docs/superpowers/ directory exists in this repo — assessing against the PR description (Step 2 of SMTP config-as-code initiative) and companion PR InsForge/InsForge#1258 as the stated requirements. The implementation matches the spec described in the PR body on all major points: schema, capability gate, env() resolution, diff/force-resend semantics, export placeholder, and apply upsert path.
Findings
Critical
(none)
Suggestion
[Software Engineering / Functionality] Port range not validated — src/lib/config-schema.ts:113–118
validateSmtp accepts any integer for port (including 0, negatives, or values > 65535). Compare the existing non-negative guard on min_interval_seconds at line 151–162 — the same pattern should apply here:
if (
typeof obj.port !== 'number' ||
!Number.isInteger(obj.port) ||
obj.port < 1 ||
obj.port > 65535
) {
throw new ConfigValidationError('auth.smtp.port', 'must be an integer between 1 and 65535');
}A user who writes port = 0 will get a cryptic backend 400 rather than a parse-time error. Low blast radius but inconsistent with how min_interval_seconds is treated in the same function.
[Functionality] Discovery comment in TOML renderer hardcodes SMTP_PASSWORD regardless of the actual ref — src/lib/config-toml.ts:60
lines.push('# password is managed via secrets — run `insforge secrets add SMTP_PASSWORD <value>`');
lines.push(`password = ${JSON.stringify(smtp.password)}`);stringifyConfigToml is a general renderer, but the comment always mentions SMTP_PASSWORD. If smtp.password is "env(PROD_SMTP_PASS)", the output becomes:
# password is managed via secrets — run `insforge secrets add SMTP_PASSWORD <value>`
password = "env(PROD_SMTP_PASS)"The comment is misleading — it tells the user to create a secret under the wrong name. Extract the name from the env() ref:
const ref = parseEnvRef(smtp.password);
const secretName = ref ?? 'SMTP_PASSWORD';
lines.push(`# password is managed via secrets — run \`insforge secrets add ${secretName} "<value>"\``);(Export-generated TOML always uses env(SMTP_PASSWORD) today so the comment is correct there — but this will bite the first user who names their secret differently.)
[Functionality] Secret-not-found detection relies on error message string — src/lib/config-secrets.ts:108–116
const message = (err as Error).message ?? '';
if (/not found/i.test(message)) {This creates implicit coupling to ossFetch's error message format. If the backend error wording changes (e.g. "Secret 'X' does not exist"), the regex fails to match and the error silently degrades from the actionable SECRET_NOT_FOUND (with insforge secrets add hint) to the generic SECRET_LOOKUP_FAILED. The mock in the test (new Error('Secret not found: MISSING')) hard-codes the matching string, so a backend message change wouldn't be caught.
Not urgent since it only affects error quality, not correctness — but it's worth tracking. Ideally ossFetch would surface the HTTP status so callers can branch on 404 vs 5xx without string-matching.
[Software Engineering] metadataSupports has no exhaustiveness check on the DiffChange union — src/lib/config-capabilities.ts:56
Unlike applyChange() (which has the const _exhaustive: never = change guard at line 205–206 of apply.ts), metadataSupports silently returns false for any unrecognized change.section. If a new DiffChange variant is added in a future PR, metadataSupports will silently skip it (every apply of that new section will be put into skipped[]). Adding the same exhaustiveness pattern here would catch that at compile time.
Information
Duplicated RawAuthMetadata / RawMetadataResponse interfaces — apply.ts:17–29 and export.ts:14–26
Identical interface definitions in two files. Not worth splitting into a shared module for two consumers, but flag for the next person who adds a third field — that's the right moment to consolidate.
Verdict
Approved (informational — explicit GitHub approval is a separate human action)
No Critical findings. Four non-blocking items. The secret-handling model is well-designed, test coverage is strong, and scope discipline is clean. The port-range validation and comment-rendering issues are the most concrete and worth a quick follow-up before the npm release (0.1.76 tag).
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>
…-as-code PR Combining both follow-ups (SMTP and deployment slug) into one PR. Both hang off the same config-as-code scaffolding (capability probe, diff, apply, export) so a single review is cheaper than two. Conflict resolution: each section is additive. Kept both DiffChange variants in the discriminated union, both apply handlers, both capability probes, both render paths, and both describe blocks in the tests. The existing "both sections present" / "older backend" tests now assert that the OTHER section gets skipped, since the fixtures only carry one slice. 312 tests pass; lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tonychang04
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/config/apply.test.ts (1)
93-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset shared metadata state in
beforeEach.
nextMetadataResponseis still not reset per test. On Line 93, please reset it (e.g.,{}) to prevent test-order coupling.Suggested patch
beforeEach(() => { vi.clearAllMocks(); secretsStore.clear(); + nextMetadataResponse = {}; tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-')); });🤖 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/commands/config/apply.test.ts` around lines 93 - 97, Tests share mutable state via the nextMetadataResponse variable causing test-order coupling; update the beforeEach block to reset nextMetadataResponse to an empty object (e.g., nextMetadataResponse = {}) so each test starts with a clean metadata state—look for the beforeEach that calls vi.clearAllMocks(), secretsStore.clear(), and mkdtempSync and add the reset of nextMetadataResponse there.
🧹 Nitpick comments (1)
src/commands/config/apply.test.ts (1)
13-39: ⚡ Quick winMake
ossFetchMockfail-closed for unknown routes.The permissive fallback response can hide unexpected API calls. Prefer throwing on unhandled
method + pathand explicitly allow known endpoints only.Suggested hardening
const ossFetchMock = vi.fn(async (path: string, init?: RequestInit) => { + const method = init?.method ?? 'GET'; if (path === '/api/metadata' && (!init || init.method === undefined || init.method === 'GET')) { return new Response(JSON.stringify(nextMetadataResponse), { status: 200, headers: { 'content-type': 'application/json' }, }); } @@ - return new Response(JSON.stringify({ ok: true }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }); + if ( + method === 'PUT' && + (path === '/api/auth/config' || + path === '/api/auth/smtp-config' || + path === '/api/deployments/slug') + ) { + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + + throw new Error(`Unhandled ossFetch call in test: ${method} ${path}`); });🤖 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/commands/config/apply.test.ts` around lines 13 - 39, The ossFetchMock currently returns a permissive fallback Response for any unhandled path/method which can mask unexpected API calls; update ossFetchMock to fail-closed by throwing an Error for unknown method+path combinations and only handle the explicit routes used in tests (the '/api/metadata' GET case and the '/api/secrets/:key' GET case that uses secretMatch and secretsStore); ensure any other methods or paths (including mismatched methods for known endpoints) throw a descriptive error mentioning the method and path so tests surface unintended requests.
🤖 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-toml.test.ts`:
- Around line 136-152: The test for stringifyConfigToml currently checks out
not.toContain('password') which is too broad; update the assertion in the "omits
password line entirely when password is undefined" test so it specifically
asserts the password key line is absent (e.g., ensure the output does not
contain a password key assignment or any line that begins with the password key)
rather than any occurrence of the word "password"; locate the test around the
it(...) block referencing stringifyConfigToml and replace the generic
containment check with a targeted check (e.g., negative regex match or not
containing the exact "password =" pattern) to avoid false positives from
comments.
---
Duplicate comments:
In `@src/commands/config/apply.test.ts`:
- Around line 93-97: Tests share mutable state via the nextMetadataResponse
variable causing test-order coupling; update the beforeEach block to reset
nextMetadataResponse to an empty object (e.g., nextMetadataResponse = {}) so
each test starts with a clean metadata state—look for the beforeEach that calls
vi.clearAllMocks(), secretsStore.clear(), and mkdtempSync and add the reset of
nextMetadataResponse there.
---
Nitpick comments:
In `@src/commands/config/apply.test.ts`:
- Around line 13-39: The ossFetchMock currently returns a permissive fallback
Response for any unhandled path/method which can mask unexpected API calls;
update ossFetchMock to fail-closed by throwing an Error for unknown method+path
combinations and only handle the explicit routes used in tests (the
'/api/metadata' GET case and the '/api/secrets/:key' GET case that uses
secretMatch and secretsStore); ensure any other methods or paths (including
mismatched methods for known endpoints) throw a descriptive error mentioning the
method and path so tests surface unintended requests.
🪄 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: 49cc9119-6a09-43d1-b529-9e795fc57439
📒 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
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/config-toml.ts
- src/commands/config/export.ts
- src/lib/config-capabilities.ts
- src/commands/config/apply.ts
- src/lib/config-diff.ts
- src/lib/config-schema.ts
- src/commands/config/export.test.ts
| it('omits password line entirely when password is undefined', () => { | ||
| const out = stringifyConfigToml({ | ||
| auth: { | ||
| smtp: { | ||
| enabled: false, | ||
| host: '', | ||
| port: 587, | ||
| username: '', | ||
| sender_email: '', | ||
| sender_name: '', | ||
| min_interval_seconds: 60, | ||
| }, | ||
| }, | ||
| }); | ||
| expect(out).toContain('[auth.smtp]'); | ||
| expect(out).not.toContain('password'); | ||
| }); |
There was a problem hiding this comment.
Tighten password omission assertion to target the key, not any text
At Line 151, not.toContain('password') is broader than the requirement (“omit password field”) and can cause false failures if comments include that word. Assert against the key line instead.
Suggested change
- expect(out).not.toContain('password');
+ expect(out).not.toMatch(/^\s*password\s*=/m);📝 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.
| it('omits password line entirely when password is undefined', () => { | |
| const out = stringifyConfigToml({ | |
| auth: { | |
| smtp: { | |
| enabled: false, | |
| host: '', | |
| port: 587, | |
| username: '', | |
| sender_email: '', | |
| sender_name: '', | |
| min_interval_seconds: 60, | |
| }, | |
| }, | |
| }); | |
| expect(out).toContain('[auth.smtp]'); | |
| expect(out).not.toContain('password'); | |
| }); | |
| it('omits password line entirely when password is undefined', () => { | |
| const out = stringifyConfigToml({ | |
| auth: { | |
| smtp: { | |
| enabled: false, | |
| host: '', | |
| port: 587, | |
| username: '', | |
| sender_email: '', | |
| sender_name: '', | |
| min_interval_seconds: 60, | |
| }, | |
| }, | |
| }); | |
| expect(out).toContain('[auth.smtp]'); | |
| expect(out).not.toMatch(/^\s*password\s*=/m); | |
| }); |
🤖 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-toml.test.ts` around lines 136 - 152, The test for
stringifyConfigToml currently checks out not.toContain('password') which is too
broad; update the assertion in the "omits password line entirely when password
is undefined" test so it specifically asserts the password key line is absent
(e.g., ensure the output does not contain a password key assignment or any line
that begins with the password key) rather than any occurrence of the word
"password"; locate the test around the it(...) block referencing
stringifyConfigToml and replace the generic containment check with a targeted
check (e.g., negative regex match or not containing the exact "password ="
pattern) to avoid false positives from comments.
Greptile SummaryExtends the config-as-code pipeline with two new TOML sections:
Confidence Score: 5/5Safe to merge — the new SMTP and deployments sections are well-isolated behind capability gates, all apply paths are gated by pre-flight checks, and no existing behavior is regressed. The core apply/export/diff logic is correct and thoroughly unit-tested (51 tests). Force-resend semantics are intentional and documented. Secrets never appear in plan output or TOML. Both findings are edge-case consistency issues that do not affect the main happy path or introduce data loss. src/lib/config-capabilities.ts — the smtpConfig presence check diverges from the truthy checks in apply.ts and export.ts; worth aligning before the backend is deployed broadly. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI
participant Metadata as GET /api/metadata
participant Secrets as GET /api/secrets/:name
participant SmtpAPI as PUT /api/auth/smtp-config
participant SlugAPI as PUT /api/deployments/slug
CLI->>Metadata: fetch raw metadata
Metadata-->>CLI: "{ auth: { smtpConfig, allowedRedirectUrls }, deployments }"
CLI->>CLI: liveFromMetadata() → LiveConfig
CLI->>CLI: diffConfig(live, file) → DiffResult
loop each DiffChange
CLI->>CLI: metadataSupports(raw, change)?
alt not supported
CLI->>CLI: "skipped.push({ key, reason })"
else auth.smtp change
alt passwordEnvRef present
CLI->>Secrets: GET /api/secrets/:name
Secrets-->>CLI: "{ value } or throw"
end
CLI->>SmtpAPI: "PUT { enabled, host, port, username, senderEmail, senderName, minIntervalSeconds, [password] }"
else deployments.subdomain change
CLI->>SlugAPI: "PUT { slug: string | null }"
end
end
CLI->>CLI: "emit { applied[], skipped[] }"
Reviews (2): Last reviewed commit: "fix(config): address AI reviewer's non-b..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (c.passwordEnvRef) { | ||
| lines.push(` (password force-resent from env(${c.passwordEnvRef}))`); | ||
| } | ||
| return lines.join('\n '); |
There was a problem hiding this comment.
' ' (4 extra spaces) while each inner line already carries its own 4-space indent prefix. The result is 8 spaces before each field line. Combined with formatPlan's own 4-space wrap via ` ${formatChange(c)}`, field lines end up at 12-space indent while the ~ smtp config: header sits at 4-space. Use a plain ' ' join and keep the prefix in the pushed lines.
| return lines.join('\n '); | |
| return lines.join('\n'); |
| // ossFetch throws on any non-2xx, swallowing the status. Recover the | ||
| // "missing secret" case from the error message — the backend's NOT_FOUND | ||
| // path is the most common failure here and deserves the named code + | ||
| // actionable hint, not a generic network error. | ||
| const message = (err as Error).message ?? ''; | ||
| if (/not found/i.test(message)) { | ||
| throw new CLIError( | ||
| `${fieldPath} references env(${secretName}) but no such secret exists.\n` + | ||
| ` fix: insforge secrets add ${secretName} "<value>"`, | ||
| 1, | ||
| 'SECRET_NOT_FOUND', | ||
| ); | ||
| } | ||
| // Other failures: re-wrap with the path context so users see what we | ||
| // were trying to resolve when the lookup blew up. | ||
| throw new CLIError( | ||
| `failed to resolve env(${secretName}) for ${fieldPath}: ${message}`, | ||
| 1, | ||
| 'SECRET_LOOKUP_FAILED', | ||
| ); | ||
| } | ||
|
|
||
| if (!res.ok) { | ||
| throw new CLIError( | ||
| `failed to resolve env(${secretName}) for ${fieldPath}: HTTP ${res.status}`, | ||
| 1, | ||
| 'SECRET_LOOKUP_FAILED', | ||
| ); | ||
| } |
There was a problem hiding this comment.
res.ok guard
The comment at line 104 says "ossFetch throws on any non-2xx, swallowing the status" — implying the if (!res.ok) guard at line 126 is dead code. But the unit test mocks ossFetch to resolve (not throw) with status: 500, which is the only way res.ok would ever trigger. One of these is wrong: either the comment is incorrect (ossFetch can return non-ok responses without throwing), or the test is exercising an unreachable path that never runs in production.
Three real findings from the review verdict:
1. Port range validation — auth.smtp.port now rejects values outside
1-65535 with a clear parse-time error. The validator was already
opinionated for min_interval_seconds; being inconsistent here was the
bad pattern (the actual concern from the reviewer, not duplicated
defense-in-depth).
2. Discovery comment named the actual env ref — when the user writes
password = "env(PROD_SMTP_PASS)", the rendered hint now says
"insforge secrets add PROD_SMTP_PASS …" instead of unconditionally
telling them to create SMTP_PASSWORD. Pointing at the wrong secret
name would have bitten anyone with a non-default ref.
3. Exhaustiveness check on metadataSupports — mirrors the pattern in
applyChange so a future DiffChange variant will TS-error at compile
time instead of silently routing every apply of that section into
skipped[] forever.
Skipped per the reviewer's own ranking: the "secret-not-found relies on
error string" suggestion (reviewer explicitly: "only affects error
quality, not correctness…not urgent") and the duplicated
RawMetadataResponse interfaces ("flag for the next person who adds a
third field").
314 tests pass (+2 new); lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code Review — feat(cli): add [auth.smtp] and [deployments] sections to config-as-code
Summary: Well-structured step-2 implementation of the SMTP config-as-code initiative. The capability-gate protocol, force-resend semantics, and secret-opaque plan output are all correct; test coverage is solid across the new surface area.
Requirements context
No /docs/superpowers/ directory exists in this repo — assessed against the PR description alone. The PR is self-contained with clear design rationale; all claimed behaviors were verified against the implementation.
Findings
Critical
(none)
Suggestion
[Software Engineering] Duplicate RawAuthMetadata / RawMetadataResponse types — apply.ts:17–39 and export.ts:14–36
Both files define identical RawAuthMetadata and RawMetadataResponse interfaces. They already diverge slightly in comments and will diverge further as the metadata shape grows (e.g. future [auth.providers] section). Extracting them once to src/lib/config-metadata.ts (or similar) prevents silent drift and makes both callers authoritative off the same type.
[Functionality] Fragile "not-found" detection in resolveEnvRef — config-secrets.ts:103–116
const message = (err as Error).message ?? '';
if (/not found/i.test(message)) {ossFetch throws for any non-2xx, swallowing the HTTP status. The regex /not found/i recovers the 404 case from the message string, but it will also match unrelated failures — for example, a reverse-proxy or network error whose message contains "route not found", "host not found", or a secret name that happens to contain "not found". The user then receives a misleading insforge secrets add hint when the actual problem is connectivity.
More robust: have ossFetch (or a wrapper) attach the numeric status to the thrown error so callers can branch on err.status === 404 rather than string-matching. If that's out of scope here, the fallback fix is a tighter pattern — e.g. /^secret not found:/i to match only the backend's exact error format, and leave all other throw paths in the SECRET_LOOKUP_FAILED branch.
The test at config-secrets.test.ts:100 deliberately mirrors the current throw format, which means this assumption is tested — but the test would also pass if the regex were accidentally over-broad.
[Functionality] Omitting SMTP fields from TOML silently applies renderer defaults — config-diff.ts:230–241 + apply.ts:186–194
renderFileSmtp fills absent fields with hardcoded defaults (port: 587, min_interval_seconds: 60, sender_email: '', …). Because applyChange builds the upsert body from change.to (the full rendered view), a minimal TOML like:
[auth.smtp]
enabled = true
host = "smtp.example.com"
username = "user"
password = "env(SMTP_PASSWORD)"will PUT port=587, sender_email='', etc., overwriting any non-default values the backend currently holds for those fields. This is whole-object upsert semantics and the design rationale is noted in the PR, but the plan output ("~ smtp config: …") only shows fields that change — so a user with port=25 on the backend will see port: 25 → 587 in their plan, but may not understand why. A one-line note in the formatted output (e.g., "omitted fields will be reset to defaults") would reduce surprise.
Information
Version bump mismatch in PR description: The body says "0.1.75 → 0.1.76" but package.json shows "0.1.76" → "0.1.77". The code is correct; the description text is stale.
Test coupling to ossFetch throw format — config-secrets.test.ts:100: The mock reproduces the exact string "Secret not found: MISSING" to drive the SECRET_NOT_FOUND branch. If ossFetch is ever refactored to produce a different message format, this test continues to pass (it controls both sides of the mock) while production silently falls through to SECRET_LOOKUP_FAILED. Worth a comment noting the coupling.
stringifyConfigToml — no test for fully-empty config (config-toml.ts:23–61): The TOML serializer's final replace(/\n+$/, '\n') is exercised by every test with content, but there's no test for stringifyConfigToml({}) (no sections at all). Minor gap.
Verdict
approved (informational — no Critical findings; explicit GitHub approval is a separate human action)
The three Suggestion items are clean-up / robustness improvements rather than blockers. The core capability-gate protocol, secret-opaque diff output, and force-resend semantics are all implemented correctly. No security issues found.
2 participants
Summary
Step 2 of the SMTP config-as-code initiative. Adds
[auth.smtp]to the declarative TOML surface with secret-aware password handling.Companion PR: InsForge/InsForge#1258 (backend — exposes
smtpConfigin admin/api/metadata). Required for this CLI feature to work — without it,config applycorrectly puts SMTP changes in `skipped[]` per the capability gate.What ships
Schema
[auth.smtp]section ininsforge.tomlwith 8 fields (enabled, host, port, username, password, sender_email, sender_name, min_interval_seconds)passwordfield validated asenv(NAME)ref only — literal values rejected at parse time (the validator infrastructure from PR feat(cli): config-as-code (export/plan/apply) + env() validation + compute port fix #109 finally gets its first consumer)Capability gate
metadataSupports()extended to proberaw.auth.smtpConfigpresenceskipped[]with the upgrade message; no PUT issued — same protocol as redirect URLsenv() ref resolution
resolveEnvRef(envRef, fieldPath)insrc/lib/config-secrets.ts/api/secrets/:namepre-flight before any PUT — missing/inactive secrets fail fast with actionable error (SECRET_NOT_FOUND/SECRET_EMPTY) naming the exactsecrets addcommandDiff layer
DiffChangediscriminated union:auth(redirect URLs) |auth.smtp(whole-object upsert)password = \"env(NAME)\", every `apply` resolves the current secrets-store value and re-sends. Reason: backend ciphertext can't be diffed against client-side, and re-sending matches user mental model ("I just rotated SMTP_PASSWORD, re-apply should propagate it")Export
Apply
Version bump
0.1.75 → 0.1.76
Tests
Test plan
Design notes captured in code
The trickier decisions are documented inline:
Followups (out of scope here)
🤖 Generated with Claude Code
Summary by cubic
Adds
[auth.smtp]and[deployments]subdomain toinsforge.tomlwith secureenv(NAME)password handling, export/apply wiring, and capability gates. Also tightens validation and improves export hints.New Features
[auth.smtp]withenabled,host,port(1–65535),username,password(must beenv(NAME)),sender_email,sender_name,min_interval_seconds;[deployments]withsubdomain(string or null;""clears on apply)./api/auth/smtp-config(omitpasswordto preserve;password = "env(NAME)"resolves via secrets store and force-resends); Deployments → PUT/api/deployments/slugwith{ slug: "name" | null }.password = "env(SMTP_PASSWORD)"when a password exists, with a discovery comment that names the actual secret in the file; Deployments emits only when a slug is set./api/metadataexposesauth.smtpConfiganddeployments; otherwise entries are added toskipped[].Bug Fixes
ossFetch-thrown errors so missing secrets surface asSECRET_NOT_FOUNDwith aninsforge secrets add NAME "<value>"hint.authbefore checkingsmtpConfigto avoid a crash on malformed metadata.metadataSupportsto prevent silently skipping future sections.Written for commit 6d6eee4. Summary will update on new commits.
Summary by CodeRabbit
Note
Add [auth.smtp] and [deployments] sections to config-as-code export and apply
[auth.smtp]and[deployments].[auth.smtp]fields with anenv(SMTP_PASSWORD)placeholder when a password exists server-side, and writes[deployments]when a non-empty custom subdomain is set.env()secret references viaGET /api/secrets/:namebefore issuingPUT /api/auth/smtp-config, failing fast with actionable errors if the secret is missing or empty. Deployment subdomain changes are sent viaPUT /api/deployments/slug, includingnullto clear.env()references are accepted for the SMTP password field.Changes since #123 opened
Macroscope summarized 35af5b9.