mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-08 22:21:45 +00:00
accd91a0a6
# Demote non-category leak fields to plain `String` Second slice of the interpolation unification — the first **reducing** PR stacked on the foundation (#472), per the reduce-first sequencing: narrowing changes land before capability additions. (The other reducing slice, the DOT de-templating, already landed independently as #474.) ## Why The target model gives `InterpString` to fields in five categories — `command` / `script` / `headers` / `env` / `url` — wherever they appear. A handful of fields were typed `InterpString` but are *identifiers or commit content*, not in any category: - `run.model.provider` / `run.model.name` - `cli.exec.model.provider` / `cli.exec.model.name` - `run.git.author.name` / `run.git.author.email` - `run.scm.owner` / `run.scm.repository` Their consumers never resolved them — they leaked raw source text via `as_source()`. This PR demotes them to plain `String` (layer and resolved structs) with **no interpolation**. ## The principle: only `InterpString` fields access variables These fields are dropped from the variable substitute pass entirely, so both `{{ vars.* }}` and `{{ env.* }}` are now literal text. This **removes an incidental behavior**: run-scoped plain-`String` fields used to get `{{ vars.* }}` substituted via the String pass (a lucky accident), while `env` always leaked literally. Variable access becomes deliberate and typed rather than accidental; if any of these fields should support variables later, that's a controlled promotion back to `InterpString`. ## Behavior changes (honest list) - **The incidental run-scoped `{{ vars.* }}` substitution on these eight fields stops working.** To keep the removal visible rather than silent, a `tracing::warn!` fires at resolve time when a demoted field still contains claimed template tokens (`warn_if_demoted_template`). Unclaimed `{{ ... }}` text (jq programs, Go templates) never interpolated and does not warn. - `{{ env.* }}` / `{{ secrets.* }}` / `{{ inputs.* }}` never resolved on these fields, so nothing else changes. ## Added in review: D11 demotions (separate commit, revertable) The rule got refined during review: a field is `InterpString` iff it is in one of the five categories **and resolved at the run boundary** (the only point where `vars`/`secrets`/`inputs` exist — they're server state, so connect-time and startup-time fields can't reach them even in principle). A separate commit applies the clean subset so it can be cherry-picked out if we change course: - `cli.target.http.url` / `cli.target.unix.path` — consumed at CLI connect time; consumers only ever leaked raw source, so nothing working is removed. - `run.working_dir` — **the outlier; see the PR comment.** Its `{{ vars.* }}` substitution worked; demoted on the category test alone. ## What's deliberately NOT here - The **control-plane fields** (`server.storage.root` / `listen.unix.path` / S3 fields / `github.app_id/client_id/slug` / `server.api.url` / `server.web.url`) — untouched here, demoted in a follow-up PR. **Resolved during review** (see the resolution comment): `InterpString` was conflating the user-facing workflow language with the internal control plane. Control-plane fields never interpolate; the few deployment knobs that need late binding (e.g. `FABRO_WEB_URL`, whose only real usage is the split-web PoC ferrying a compose env var across a file mount) become explicit native `EnvVars` reads, and `fabro-server/src/interp.rs` shrinks to deletion. `slack.default_channel` stays `InterpString` (consumed with run context). ## Implementation notes - Consumers move from `as_source()` to direct `String` access; the foundation's `#[expect(disallowed_methods, ... demotion pending ...)]` annotations for these fields are removed (no longer `InterpString`). - `fabro-checkpoint`'s author plumbing and `fabro-manifest`'s scm fields simplify accordingly. ## Verification - `cargo build --workspace` - `cargo nextest run --workspace` → 6684 passed, 181 skipped - `cargo +nightly fmt --check --all` - `cargo +nightly clippy --workspace --all-targets -- -D warnings` → clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| crates | ||
| packages/fabro-api-client | ||