Batch 1.5: realign EnvConnector with synthing architecture

.chief/milestone-2/_contract/base-connector-contract.md

@@ -0,0 +1,49 @@
+# Contract: BaseConnector
+
+Service boundary between `@synthing/core` and any connector plugin (e.g. `@synthing/plugin-env`). Locked by the grilling session recorded in `.chief/milestone-2/_goal/design-spec.md` §14 (decisions 94–99). Must not be violated without a new grilling session updating both this file and the design-spec.
+
+## Interface shape
+
+```ts
+// @synthing/core
+interface BaseConnector<Config extends object = object> {
+  config: Config;
+  logger?: Logger;
+
+  load(keys: string[]): Promise<void>;
+  get(key: string): unknown | undefined;
+
+  setWorkingDir?(dir: string | undefined): void;
+  getWorkingDir?(): string | undefined;
+}
+```
+
+- **`interface`, not `abstract class`.** Structural typing — a connector plugin's installed `@synthing/core` copy does not need to be the same class instance as the host's (avoids the dual-package hazard).
+- `config`, `logger`, `setWorkingDir?`, `getWorkingDir?` are part of the contract, not connector-specific extras. Engine code may call `connector.setWorkingDir?.(dir)` on any `BaseConnector` without narrowing to a concrete class.
+
+## Coercion boundary (hard rule)
+
+**Connectors never coerce, parse, or transform values.** `get()` returns the raw value exactly as read from the source.
+
+Rationale: `load(keys: string[])` only receives key *names* — a connector has no way to know a key's declared `VariableType`. Any connector-side type inference is necessarily a blind guess based on value shape, not the user's declared intent, and can double-coerce a value the engine's `coerceFromString()` then tries to coerce again (e.g. a connector that JSON-parses a flat-object-looking string before the engine's `coerceFromString(value, "object")` — which expects a raw string — ever sees it).
+
+`@synthing/toolkit.coerceFromString(rawValue, declaredType)` is the **only** coercion path, called exclusively by the engine, after `get()` returns.
+
+## No `SecretValue` type
+
+Kubricate's `SecretValue` (`string | number | boolean | null | undefined | Record<string, Primitive>`) existed to satisfy `BaseProvider.prepare(name, value: SecretValue)` — Kubernetes Secrets are flat, string-serializable key-value maps, so the type was shaped by that downstream serialization constraint.
+
+Synthing has no `BaseProvider` equivalent. Nothing downstream constrains connector output shape. `get()` stays `unknown | undefined` at the interface level; concrete connectors narrow it to whatever their source actually produces (`EnvConnector` → `string | undefined`, since env vars are always strings).
+
+## Logging (connector-specific, not part of the core interface)
+
+A connector MAY log values it handles, but must not assume it knows which keys are secrets — `secret: true` is a `VariableManager`-level declaration the connector never sees. `EnvConnector` masks every logged value indiscriminately by default (`maskValues: true`) rather than guessing.
+
+## Concrete instance: EnvConnector
+
+| Aspect | Contract |
+|---|---|
+| `get()` return type | `string \| undefined` (no `SecretValue`, no object coercion) |
+| Key matching | `expectedKey = prefix + key`, matched against `process.env` case-insensitively by default (`caseInsensitive: true`) |
+| Value transformation | None — raw string only, no JSON-sniffing |
+| Logging | `maskValues: true` by default — all logged values masked via `maskingValue()` |

.chief/milestone-2/_goal/design-spec.md

@@ -121,7 +121,9 @@ For a given key, the value precedence is:
 
 ### 2.6 Type Coercion
 
-**Shared coercion utilities (from `@synthing/toolkit`)** — toolkit exports `coerceFromString()` helpers. Connectors and the engine may use them. The engine performs the final type-check to ensure basic type matching (a variable declared as `number` returns a number or errors).
+**Shared coercion utilities (from `@synthing/toolkit`)** — toolkit exports `coerceFromString()` helpers. **Coercion is engine-owned only.** Connectors return raw values exactly as received from their source (e.g. `EnvConnector` returns the raw `process.env` string, untouched) — they never parse, transform, or guess types. The engine is the sole caller of `coerceFromString()`, using the `type` declared via `.addVariable()`, and performs the final type-check (a variable declared as `number` returns a number or errors).
+
+> A connector never knows a key's declared `type` — `load(keys: string[])` only receives key names. Any connector-side coercion would be a blind guess based on value shape, not the user's declared intent, and risks double-coercing values the engine then tries to coerce again. See `.chief/milestone-2/_contract/base-connector-contract.md` for the full rationale.
 
 ```ts
 // @synthing/toolkit exports
@@ -239,26 +241,36 @@ Users declare keys via `.addVariable()`. Connectors do not contribute or discove
 
 ### 4.1 BaseConnector (in `@synthing/core`)
 
-`BaseConnector` is **domain-agnostic**. The same connector class serves both variables and secrets. It lives in `@synthing/core` as a pure abstract interface.
+`BaseConnector` is **domain-agnostic**. The same connector implementation serves both variables and secrets. It lives in `@synthing/core` as a TypeScript **`interface`** (not an `abstract class`) — structural typing avoids the dual-package hazard for third-party connector plugins (a plugin's installed `@synthing/core` copy doesn't need to be the exact same class instance as the host's; it only needs to match the shape).
 
 Naming: `BaseConnector`, not `BaseVariableConnector` or `BaseVariableResolver`. Aligns with kubricate's `BaseConnector` term.
 
+`BaseConnector` has no `SecretValue`-style return type. Kubricate's `SecretValue` union existed to satisfy `BaseProvider.prepare()`'s requirement that secrets be flat and string-serializable for Kubernetes Secret encoding — synthing has no `BaseProvider` equivalent, so there's nothing downstream constraining connector output shape. `get()` returns `unknown | undefined`; concrete connectors narrow it (e.g. `EnvConnector` narrows to `string | undefined`, since env vars are always strings).
+
 ### 4.2 Two-Phase Interface
 
-Matches kubricate's `BaseConnector` interface:
+Matches kubricate's `BaseConnector` interface shape, including the optional working-dir and logger members:
 
 ```ts
-abstract class BaseConnector {
+interface BaseConnector<Config extends object = object> {
+  config: Config;
+  logger?: Logger;
+
   /** Load/prepare values for the given keys (async, called once) */
-  abstract load(keys: string[]): Promise<void>;
+  load(keys: string[]): Promise<void>;
 
   /** Get a single value (sync, called per-key after load) */
-  abstract get(key: string): unknown | undefined;
+  get(key: string): unknown | undefined;
+
+  /** Optional — no-op if a connector doesn't need a working directory */
+  setWorkingDir?(dir: string | undefined): void;
+  getWorkingDir?(): string | undefined;
 }
 ```
 
 - `load(keys[])` — called once with all keys this connector might need. Allows batch fetching.
 - `get(key)` — called per-key after load. Returns the raw value or `undefined`.
+- `config`, `logger`, `setWorkingDir?`, `getWorkingDir?` are part of the shared contract (not connector-specific extras), so engine code can address any connector generically without narrowing to a concrete class.
 
 ### 4.3 EnvConnector (in `@synthing/plugin-env`)
 
@@ -275,6 +287,12 @@ new EnvConnector({ prefix: "APP_" })
 
 Users can separate variable vs secret env vars via prefix convention (e.g., `VAR_`, `SECRET_`).
 
+**Env var name matching:** `expectedKey = prefix + key`, matched against `process.env` **case-insensitively by default** (`caseInsensitive` defaults to `true`). A declared key `port` with prefix `APP_` matches `APP_PORT`, `APP_port`, or any other casing — there is no uppercase transform of the key itself, matching is case-agnostic instead. This is why the example above (`APP_PORT`) and a lowercase-declared key both work without any extra configuration.
+
+**Coercion boundary:** `get()` returns the raw `process.env` string exactly as read — never JSON-parsed, never transformed. Kubricate's `tryParseSecretValue()` (best-effort JSON-sniffing) is intentionally **not** carried over; per 2.6, coercion is engine-owned only.
+
+**Logging:** `maskValues` config, defaults to `true`. Every value the connector logs is masked via a `maskingValue()` utility, regardless of whether the key is a declared secret — the connector has no visibility into `secret: true` (that's a `VariableManager`-level fact), so it masks indiscriminately rather than guessing. Set `maskValues: false` to log raw values (local debugging only).
+
 ---
 
 ## 5. Generator System
@@ -1118,3 +1136,14 @@ For traceability, each major decision is numbered. These numbers correspond to t
 91. `BaseGenerator.format` is `string` (not literal union) — extensible, engine does map lookup
 92. CLI uses `yargs` (same as kubricate)
 93. Config loading uses `unconfig` (same as kubricate)
+
+---
+
+### Grilling session 2 — EnvConnector realignment (post-Batch-1)
+
+94. `BaseConnector` is a TypeScript `interface`, not an `abstract class` — structural typing avoids the dual-package hazard for third-party connector plugins
+95. `BaseConnector` interface includes `config`, `logger`, `setWorkingDir?`, `getWorkingDir?` as contract members, not just `load`/`get` — engine code can address any connector generically
+96. No connector-side coercion — `load(keys: string[])` only receives key names, never declared types, so connectors cannot do type-aware coercion. `@synthing/toolkit.coerceFromString` at the engine layer is the sole coercion path
+97. `SecretValue` type dropped entirely — it was a kubricate artifact tied to `BaseProvider`'s Kubernetes-Secret string-serialization constraint, which synthing has no equivalent of. `EnvConnector.get()` narrows to `string | undefined`
+98. `EnvConnector.caseInsensitive` defaults to `true` — env var name matching is case-agnostic by default, so declared lowercase keys match uppercase env vars (and vice versa) without extra config
+99. `EnvConnector` adds `maskValues` config, default `true` — every logged value is masked via `maskingValue()` regardless of secret-ness, since the connector has no visibility into which keys are declared `secret: true`

.chief/milestone-2/_plan/_todo.md

@@ -8,17 +8,26 @@
 - [x] task-4: Implement VariableManager with builder pattern, $var branded refs, and $spread helper
 - [x] task-5: Implement EnvConnector in @synthing/plugin-env
 
+## Batch 1.5: EnvConnector Realignment (blocks Batch 2 — resolution engine depends on the coercion boundary this batch locks in)
+
+Handoff from the grilling session recorded in `.chief/milestone-2/_goal/design-spec.md` §14 (decisions 94–99) and `.chief/milestone-2/_contract/base-connector-contract.md`. Batch 1 shipped `EnvConnector` with behavior that drifted from its own task spec (task-5.md said "uppercase transform"; the shipped code does exact-case matching) and carried over kubricate mechanisms (`SecretValue`, `tryParseSecretValue`) that don't fit synthing's architecture. This batch reconciles code, contract, and docs before Batch 2 builds on top of it.
+
+- [x] task-6: Convert `BaseConnector` from `abstract class` to `interface`; add `config`/`logger`/`setWorkingDir?`/`getWorkingDir?` to the contract
+- [x] task-7: Remove `SecretValue` and `tryParseSecretValue()` from `EnvConnector`; narrow `get()` to `string | undefined`
+- [x] task-8: Default `caseInsensitive` to `true` in `EnvConnectorConfig`; add a test locking in default uppercase-env-var matching
+- [x] task-9: Add `maskValues` config (default `true`); restore a `maskingValue()` utility; mask all logged values by default
+
 ## Batch 2: Engine, Generators, and Pipeline (planned, not yet detailed)
 
-- [ ] task-6: Implement resolution engine (tag scanning, resolve, resolveAll, strict/loose mode)
-- [ ] task-7: Implement YamlGenerator and GeneratorContext
-- [ ] task-8: Implement generator pipeline runner
-- [ ] task-9: Implement text pipeline (plain text and structural modes)
-- [ ] task-10: Implement $spread resolution in structural mode
+- [ ] task-10: Implement resolution engine (tag scanning, resolve, resolveAll, strict/loose mode)
+- [ ] task-11: Implement YamlGenerator and GeneratorContext
+- [ ] task-12: Implement generator pipeline runner
+- [ ] task-13: Implement text pipeline (plain text and structural modes)
+- [ ] task-14: Implement $spread resolution in structural mode
 
 ## Batch 3: CLI, Config, Integration, and Acceptance (planned, not yet detailed)
 
-- [ ] task-11: Implement defineConfig() and config loading
-- [ ] task-12: Implement CLI (synthing generate, synthing variable export-schema)
-- [ ] task-13: Write integration tests and kubricate workflow tests
-- [ ] task-14: Full acceptance verification against all criteria
+- [ ] task-15: Implement defineConfig() and config loading
+- [ ] task-16: Implement CLI (synthing generate, synthing variable export-schema)
+- [ ] task-17: Write integration tests and kubricate workflow tests
+- [ ] task-18: Full acceptance verification against all criteria

.chief/milestone-2/_plan/task-6.md

@@ -0,0 +1,51 @@
+# Task 6: Convert BaseConnector from abstract class to interface
+
+## Objective
+
+Convert `BaseConnector` in `@synthing/core` from an `abstract class` to a TypeScript `interface`, and add `config`, `logger`, `setWorkingDir?`, `getWorkingDir?` as part of the contract. Update `EnvConnector` to `implements` the interface instead of `extends` the class.
+
+## Scope
+
+### Included
+- `packages/core/src/connector.ts`: replace `abstract class BaseConnector` with `interface BaseConnector<Config extends object = object>`
+- Add `config: Config`, `logger?: Logger`, `setWorkingDir?(dir: string | undefined): void`, `getWorkingDir?(): string | undefined` to the interface
+- `packages/plugin-env/src/env-connector.ts`: change `class EnvConnector extends BaseConnector` to `class EnvConnector implements BaseConnector<EnvConnectorConfig>`
+- Remove `super()` call and `override` keyword from `EnvConnector` (no longer meaningful against an interface)
+
+### Excluded
+- `SecretValue` / `tryParseSecretValue()` removal (task-7)
+- `caseInsensitive` default change (task-8)
+- `maskValues` / logging changes (task-9)
+
+## Rules & Contracts to Follow
+- `.chief/_rules/_standard/coding-standards.md`
+- `.chief/milestone-2/_goal/design-spec.md` Sections 4.1, 4.2
+- `.chief/milestone-2/_contract/base-connector-contract.md`
+
+## Steps
+
+1. Edit `packages/core/src/connector.ts`: interface definition per the contract doc
+2. Edit `packages/plugin-env/src/env-connector.ts`: `implements` instead of `extends`, drop `super()`/`override`
+3. Run `pnpm check-types` and fix any fallout
+4. Run `pnpm test` to confirm no behavioral regressions
+
+## Acceptance Criteria
+
+- `BaseConnector` is declared as an `interface`, not a `class`, in `@synthing/core`
+- `EnvConnector implements BaseConnector<EnvConnectorConfig>`
+- `pnpm build` exits 0
+- `pnpm check-types` exits 0
+- `pnpm test` exits 0
+
+## Verification
+
+```bash
+pnpm build
+pnpm check-types
+pnpm test --filter @synthing/core --filter @synthing/plugin-env
+```
+
+## Deliverables
+
+- `packages/core/src/connector.ts` (updated)
+- `packages/plugin-env/src/env-connector.ts` (updated)

.chief/milestone-2/_plan/task-7.md

@@ -0,0 +1,57 @@
+# Task 7: Remove SecretValue and tryParseSecretValue from EnvConnector
+
+## Objective
+
+Remove the `SecretValue` type and `tryParseSecretValue()` method from `EnvConnector`. The connector must return raw `process.env` string values only — no JSON-sniffing, no coercion. This closes the coercion-boundary gap: `tryParseSecretValue()` could hand the engine an already-parsed object where `coerceFromString()` expects a raw string, breaking exactly the `type: "object"` case it's meant to handle.
+
+## Scope
+
+### Included
+- Delete `tryParseSecretValue()` method from `packages/plugin-env/src/env-connector.ts`
+- Delete the `SecretValue` type definition and its export from `packages/plugin-env/src/index.ts`
+- `get(key: string)` return type narrows to `string | undefined`
+- `load()` stores the raw string directly (no `tryParseSecretValue()` call)
+- Remove the `tryParseSecretValue()` describe block from `env-connector.test.ts`; remove any test asserting object-parsing behavior
+
+### Excluded
+- `BaseConnector` interface conversion (task-6, should land first)
+- `caseInsensitive` / `maskValues` changes (tasks 8, 9)
+
+## Rules & Contracts to Follow
+- `.chief/_rules/_standard/coding-standards.md`
+- `.chief/milestone-2/_goal/design-spec.md` Section 2.6, Section 4.3 ("Coercion boundary")
+- `.chief/milestone-2/_contract/base-connector-contract.md`
+
+## Steps
+
+1. Remove `tryParseSecretValue()` from `env-connector.ts`
+2. Remove `SecretValue` type and its export from `index.ts`
+3. Update `get()` signature and `this.secrets` map type to `Map<string, string>`
+4. Update `load()` to store `process.env[matchKey]` directly, no parsing
+5. Remove/update affected tests in `env-connector.test.ts`
+6. Confirm no other package imports `SecretValue` from `@synthing/plugin-env`
+
+## Acceptance Criteria
+
+- No `SecretValue` type or export anywhere in `@synthing/plugin-env`
+- No `tryParseSecretValue` method anywhere in the codebase
+- `EnvConnector.get()` is typed `string | undefined`
+- A flat-JSON-looking env var value (e.g. `'{"a":1}'`) is returned as the raw string `'{"a":1}'`, unparsed
+- `pnpm build` exits 0
+- `pnpm check-types` exits 0
+- `pnpm test` exits 0
+
+## Verification
+
+```bash
+pnpm build
+pnpm check-types
+pnpm test --filter @synthing/plugin-env
+grep -rn "SecretValue\|tryParseSecretValue" packages/ # should return nothing
+```
+
+## Deliverables
+
+- `packages/plugin-env/src/env-connector.ts` (updated)
+- `packages/plugin-env/src/index.ts` (updated)
+- `packages/plugin-env/src/env-connector.test.ts` (updated)

.chief/milestone-2/_plan/task-8.md

@@ -0,0 +1,45 @@
+# Task 8: Default caseInsensitive to true
+
+## Objective
+
+Change `EnvConnectorConfig.caseInsensitive` default from `false` to `true`. This resolves the mismatch between the design-spec's CLI examples (which use uppercase env vars like `APP_PORT`) and the connector's exact-case-key lookup (`prefix + key`, e.g. `APP_port` for key `port`) — with case-insensitive matching on by default, both forms resolve to the same variable without any doc or code disagreement.
+
+## Scope
+
+### Included
+- `packages/plugin-env/src/env-connector.ts`: change `this.caseInsensitive = config.caseInsensitive ?? false` to `?? true`
+- Add a test confirming default (no explicit `caseInsensitive` flag) matches an uppercase env var against a lowercase-declared key
+- Confirm existing exact-case tests still pass (case-insensitive matching is a superset of exact-case matching)
+
+### Excluded
+- `BaseConnector` interface conversion (task-6)
+- `SecretValue` / coercion removal (task-7)
+- `maskValues` logging (task-9)
+
+## Rules & Contracts to Follow
+- `.chief/_rules/_standard/coding-standards.md`
+- `.chief/milestone-2/_goal/design-spec.md` Section 4.3 ("Env var name matching")
+- `.chief/milestone-2/_contract/base-connector-contract.md`
+
+## Steps
+
+1. Change the default in the `EnvConnector` constructor
+2. Add test: `new EnvConnector({ prefix: "APP_" })` (no `caseInsensitive` specified), `process.env.APP_PORT = "8080"`, `load(["port"])`, `get("port")` returns `"8080"`
+3. Re-run the full `env-connector.test.ts` suite to confirm no existing test relied on strict-by-default behavior
+
+## Acceptance Criteria
+
+- `new EnvConnector({ prefix: "APP_" })` with `process.env.APP_PORT` set matches key `port` without `caseInsensitive: true` being passed explicitly
+- Existing exact-case tests (e.g. `APP_port` matching key `port`) still pass unchanged
+- `pnpm test` exits 0
+
+## Verification
+
+```bash
+pnpm test --filter @synthing/plugin-env
+```
+
+## Deliverables
+
+- `packages/plugin-env/src/env-connector.ts` (updated)
+- `packages/plugin-env/src/env-connector.test.ts` (updated)