Skip to content

Commit ec69c21

Browse files
waleedlatif1claude
andcommitted
fix(trigger): use the shared logger and correct the clearing docs
Review follow-ups. The required-sync failure path used `console.error`, which the repo's logging rule forbids outside the CLI package; it now goes through `createLogger`. The architecture rule and `normalizeSecretValue` both still described the preserve-on-absence behavior that the previous commit replaced, so they told a maintainer the opposite of what the resolver does when a key leaves the secret. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND
1 parent 5453ba0 commit ec69c21

4 files changed

Lines changed: 12 additions & 12 deletions

File tree

.claude/rules/sim-architecture.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ that list and set it in the secret. The repo still cannot see what else the dash
6969

7070
Two constraints on that list. `syncEnvVars` strips every `TRIGGER_`-prefixed key before it
7171
publishes, so such a variable can only be set in the dashboard (`assertSyncableKeys` fails the
72-
build rather than letting one look synced). And a key absent from the secret is left untouched
73-
rather than blanked, so removing it from the secret does not remove it from a worker.
72+
build rather than letting one look synced). And the secret is authoritative: after a successful
73+
read, a key it does not carry — absent, `null`, or blank — is published as `''`, so deleting a
74+
credential from the secret revokes it in the worker too. Nothing is cleared when the read failed
75+
or the environment is unmapped, since there is then no authoritative view to clear against.
7476

7577
So before replacing a worker's HTTP call to our own API with an in-process call, ask what env
7678
that work reads *on the app side*. Anything gated by a `require*Capability` helper is the sharp

apps/sim/lib/core/config/trigger-env-sync.test.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

66
const { mockFetchSecretMap } = vi.hoisted(() => ({ mockFetchSecretMap: vi.fn() }))
77

@@ -24,11 +24,6 @@ function byName(vars: { name: string; value: string; isSecret: boolean }[]) {
2424
describe('resolveTriggerEnvVars', () => {
2525
beforeEach(() => {
2626
vi.clearAllMocks()
27-
process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined
28-
})
29-
30-
afterEach(() => {
31-
process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined
3227
})
3328

3429
it('reads the secret mapped to the environment', async () => {
@@ -100,7 +95,7 @@ describe('resolveTriggerEnvVars', () => {
10095
})
10196

10297
it('fails the resolve instead of publishing a partial env when sync is required', async () => {
103-
process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = '1'
98+
vi.stubEnv('SIM_TRIGGER_ENV_SYNC_REQUIRED', '1')
10499

105100
await expect(
106101
resolveTriggerEnvVars('prod', async () => {

apps/sim/lib/core/config/trigger-env-sync.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,10 @@ function unavailable(reason: string): SyncedEnvVar[] {
165165

166166
/**
167167
* Coerces a secret entry to an env var value, matching how container boot
168-
* hydrates `process.env`. An absent or empty value is treated as unset so a
169-
* blank secret entry cannot overwrite a working dashboard value with `''`.
168+
* hydrates `process.env`. `undefined` means the secret does not configure this
169+
* key, which the caller publishes as `''` to clear any value a previous deploy
170+
* left in the worker. A blank entry is reported the same way as an absent one,
171+
* so blanking a key in the secret revokes it exactly like deleting it.
170172
*/
171173
function normalizeSecretValue(value: unknown): string | undefined {
172174
if (value === undefined || value === null) return undefined

apps/sim/trigger.config.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-http'
22
import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http'
33
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http'
44
import { resourceFromAttributes } from '@opentelemetry/resources'
5+
import { createLogger } from '@sim/logger'
56
import { getErrorMessage } from '@sim/utils/errors'
67
import {
78
additionalFiles,
@@ -110,7 +111,7 @@ export default defineConfig({
110111
// deploy continue having published nothing — so rejecting is not a
111112
// way to fail. Only an explicit non-zero exit is, and this path is
112113
// reached only when SIM_TRIGGER_ENV_SYNC_REQUIRED asked for it.
113-
console.error(getErrorMessage(error))
114+
createLogger('TriggerConfig').error(getErrorMessage(error))
114115
process.exit(1)
115116
}
116117
}),

0 commit comments

Comments
 (0)