mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-20 00:11:34 +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> |
||
|---|---|---|
| .. | ||
| approve.rs | ||
| archive.rs | ||
| artifact_cp.rs | ||
| artifact_list.rs | ||
| attach.rs | ||
| auth.rs | ||
| cli_reference.rs | ||
| config.rs | ||
| create.rs | ||
| deny.rs | ||
| diff.rs | ||
| discord.rs | ||
| docs.rs | ||
| doctor.rs | ||
| dump.rs | ||
| events.rs | ||
| exec.rs | ||
| fabro.rs | ||
| fork.rs | ||
| graph.rs | ||
| inspect.rs | ||
| install.rs | ||
| json_global.rs | ||
| logs.rs | ||
| mcp.rs | ||
| mod.rs | ||
| model.rs | ||
| model_list.rs | ||
| model_test.rs | ||
| parent.rs | ||
| parse.rs | ||
| pr.rs | ||
| pr_close.rs | ||
| pr_create.rs | ||
| pr_link.rs | ||
| pr_merge.rs | ||
| pr_unlink.rs | ||
| pr_view.rs | ||
| preflight.rs | ||
| provider.rs | ||
| provider_login.rs | ||
| ps.rs | ||
| render_graph.rs | ||
| repo.rs | ||
| repo_deinit.rs | ||
| repo_init.rs | ||
| resume.rs | ||
| rewind.rs | ||
| rm.rs | ||
| run.rs | ||
| runner.rs | ||
| sandbox_cp.rs | ||
| sandbox_preview.rs | ||
| sandbox_ssh.rs | ||
| secret.rs | ||
| secret_list.rs | ||
| secret_rm.rs | ||
| secret_set.rs | ||
| send_analytics.rs | ||
| send_panic.rs | ||
| server_start.rs | ||
| server_status.rs | ||
| server_stop.rs | ||
| start.rs | ||
| support.rs | ||
| system.rs | ||
| system_df.rs | ||
| system_events.rs | ||
| system_info.rs | ||
| system_prune.rs | ||
| system_repair.rs | ||
| test_panic.rs | ||
| top_level.rs | ||
| unarchive.rs | ||
| uninstall.rs | ||
| upgrade.rs | ||
| validate.rs | ||
| variable.rs | ||
| variable_get.rs | ||
| variable_list.rs | ||
| variable_rm.rs | ||
| variable_set.rs | ||
| version.rs | ||
| wait.rs | ||
| worker_auth.rs | ||
| workflow.rs | ||
| workflow_create.rs | ||
| workflow_list.rs | ||