diff --git a/run.json b/run.json index f3f0c50a2..424efeb0d 100644 --- a/run.json +++ b/run.json @@ -508,24 +508,25 @@ "kind": "running" }, "status_updated_at": "2026-05-07T21:31:26.204215Z", - "last_event_at": "2026-05-07T21:31:40.365287Z", + "last_event_at": "2026-05-07T21:33:53.013295Z", "pending_control": null, "checkpoint": { - "timestamp": "2026-05-07T21:33:43.304468Z", - "current_node": "preflight_compile", + "timestamp": "2026-05-07T21:36:06.737029Z", + "current_node": "preflight_lint", "completed_nodes": [ "start", "toolchain", - "preflight_compile" + "preflight_compile", + "preflight_lint" ], "node_retries": {}, "context_values": { "internal.work_dir": "/home/daytona/workspace", "command.output": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", "failure_class": "", - "current_node": "preflight_compile", + "current_node": "preflight_lint", "graph.rankdir": "LR", - "internal.thread_id": "toolchain", + "internal.thread_id": "preflight_compile", "graph.model_stylesheet": "\n * { model: claude-opus-4-7; }\n ", "internal.fidelity": "compact", "graph.goal": "# Move GitHub token permissions from `[server.integrations.github]` to `[run.integrations.github]`\n\n## Context\n\nToday `[server.integrations.github.permissions]` controls what scopes Fabro requests on the Installation Access Token it injects into the sandbox as `GITHUB_TOKEN`. This is conceptually wrong:\n\n- Token permissions describe what *this run* is authorized to do, not the server's identity.\n- The current key is server-only. `workflow.toml` and `project.toml` cannot override it (`builders.rs:394` strips `server.*` from per-workflow layers), so projects/workflows can't tighten or relax permissions.\n- The public docs (`integrations/github.mdx:211-220`) already describe a per-run config, which never existed in code. The intent has always been run-level.\n\nGreenfield app — no migration, no backwards compat. Moving permissions to `[run.integrations.github.permissions]` makes the natural layer-merge (workflow > project > user > defaults) Just Work: server admins set defaults in `~/.fabro/settings.toml`, projects and workflows override.\n\n## Security model\n\nRun config is trusted policy input for sandbox token scopes. The upper bound on what Fabro can mint is the GitHub App installation's granted permissions; Fabro does **not** impose a separate server-side cap. Operators must not run untrusted workflow/project/user TOML against a broadly-scoped App installation. Preflight prints requested permissions so reviewers can see them; no enforcement layer beyond GitHub's own.\n\n## Design\n\nNew TOML path: `[run.integrations.github.permissions]`. Creates a fresh `[run.integrations]` namespace for future run-level integration knobs.\n\n**Merge semantics** (presence-aware, hand-rolled). `ReplaceMap` does NOT work here: `maps.rs:76-80` is `if self.0.is_empty() { other } else { self }`, i.e. an empty higher layer falls back to the inherited map. We need empty-wins-as-clear, so we don't reuse `ReplaceMap` (and don't change global semantics for other users).\n\nLayer field: `pub permissions: Option>`.\n\n| Higher layer | Lower layer | Result |\n|---|---|---|\n| `None` | anything | lower (inherit) |\n| `Some(map)` | anything | `Some(map)` (full replace, including `Some({})` = clear) |\n\nHand-roll `Combine` on `RunIntegrationsGithubLayer`:\n```rust\nimpl Combine for RunIntegrationsGithubLayer {\n fn combine(self, other: Self) -> Self {\n Self { permissions: self.permissions.or(other.permissions) }\n }\n}\n```\nDon't derive — the blanket `Option` impl recurses into the inner type and would reintroduce the empty-fallback bug.\n\nResolved type collapses the Option: `pub permissions: HashMap` where empty = no token requested. The presence distinction only matters during merge.\n\n**Interpolation**: keep `InterpString` in the resolved type. Resolve to `String` at the start-services construction boundary, matching the existing pattern at `server.rs:2763-2772`. No early resolution in `resolve_run`.\n\n## Changes\n\n### 1. Config schema — `lib/crates/fabro-config/src/layers/run.rs`\n\nAdd new layer types. `RunIntegrationsLayer` derives `Combine` normally; `RunIntegrationsGithubLayer` does NOT — hand-roll `Combine` (see Design section) so `Some({})` is honored as a clear sentinel.\n\n```rust\n/// `[run.integrations]` — run-level integration knobs.\n#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)]\n#[serde(deny_unknown_fields)]\npub struct RunIntegrationsLayer {\n #[serde(default, skip_serializing_if = \"Option::is_none\")]\n pub github: Option,\n}\n\n/// `[run.integrations.github]` — runtime GitHub token shape.\n#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]\n#[serde(deny_unknown_fields)]\npub struct RunIntegrationsGithubLayer {\n #[serde(default, skip_serializing_if = \"Option::is_none\")]\n pub permissions: Option>,\n}\n\nimpl Combine for RunIntegrationsGithubLayer {\n fn combine(self, other: Self) -> Self {\n Self { permissions: self.permissions.or(other.permissions) }\n }\n}\n```\n\nAdd `pub integrations: Option` to `RunLayer`.\n\n### 2. Config schema — server side\n\n`lib/crates/fabro-config/src/layers/server.rs:226`: remove `permissions: StickyMap` from `GithubIntegrationLayer`. Server struct keeps only identity/auth/webhook fields.\n\n### 3. Resolved types — `lib/crates/fabro-types/src/settings/run.rs`\n\nResolved types collapse the layer-time `Option` (presence is only meaningful during merge):\n\n```rust\npub struct RunIntegrationsSettings {\n pub github: RunIntegrationsGithubSettings,\n}\n\npub struct RunIntegrationsGithubSettings {\n pub permissions: HashMap, // empty = no token\n}\n```\n\nAdd `integrations: RunIntegrationsSettings` to `RunNamespace`. Drop `permissions` from the resolved `GithubIntegrationSettings` in fabro-types.\n\n### 4. Resolver — `lib/crates/fabro-config/src/resolve/run.rs`\n\nAdd `fn resolve_integrations(layer: Option<&RunIntegrationsLayer>) -> RunIntegrationsSettings`. Pass through `InterpString`s untouched — do NOT resolve env vars here. Collapse `Option>` → `HashMap<...>` (None and Some({}) both become empty).\n\n`lib/crates/fabro-config/src/resolve/server.rs:465`: drop the line that copies `permissions` into the resolved github settings.\n\n### 5. Server consumer — `lib/crates/fabro-server`\n\nFour read sites swap from `server_settings.server.integrations.github.permissions` to `run_spec.settings.run.integrations.github.permissions` (a flat `HashMap`; empty = no token):\n\n- `run_manifest.rs:488` — clone-credential gate inside `prepare_manifest`.\n- `run_manifest.rs:1184-1235` — `run_github_token_check`. Change signature to take resolved run permissions; resolve `InterpString`s inside the function for both minting and the report.\n- `server.rs:2727` — forced-credential gate in run launch path.\n- `server.rs:2763-2772` — InterpString → String resolution for `StartServices.github_permissions`. Read from run settings instead of server settings; same resolution logic.\n\n### 5b. Bundled-workflow TOML parsing — `lib/crates/fabro-server/src/run_manifest.rs:290-307`\n\n`root_workflow_run_layer` currently parses the workflow TOML as a raw `toml::Table`, lifts out `run`, and silently discards every other top-level key. That means stale `[server.integrations.github.permissions]` is not caught by `deny_unknown_fields`.\n\nNaive \"reject any non-`run` key\" would break valid workflow TOML (every workflow.toml in the repo has `_version = 1`; `hello/workflow.toml` has `[workflow]`). `SettingsLayer` (`layers/settings.rs:21-37`) is the schema for a settings file: `_version`, `project`, `workflow`, `run`, `cli`, `server`, `features`.\n\nFix: parse the source via `source.parse::()` and take `layer.run.unwrap_or_default()`. That gives:\n- valid top-level domains parse without error;\n- stale `[server.integrations.github.permissions]` fails because `permissions` is no longer a known field on `GithubIntegrationLayer` (section 2) and the layer has `serde(deny_unknown_fields)`;\n- shape-valid `[server.*]` in a workflow.toml is silently ignored downstream by `builders.rs:394`, preserving today's behavior (workflow.toml shouldn't set server config, but doesn't blow up either).\n\n`resolve_manifest_dockerfile(&mut run, &config.path, &workflow.files)` (line 305) still runs on the extracted `RunLayer`.\n\n### 6. CLI worker — `lib/crates/fabro-cli/src/commands/run/runner.rs` (P0, was missing)\n\nTwo reads on the CLI launch path that today still point at the server-side field:\n\n- `runner.rs:120` — `github_permissions: HashMap::new()` is hardcoded. Source from `run_spec.settings.run.integrations.github.permissions`, applying the same `InterpString` → `String` resolution as the server side.\n- `runner.rs:514-555` — `maybe_build_github_credentials`. Line 524 reads `settings.server.integrations.github.permissions.is_empty()` to decide whether credentials are required. Swap to read the run-level permissions. Identity fields (`strategy`, `app_id`, `slug`) at lines 527-538 stay on the server side.\n\nTo make this testable, factor two private helpers in `runner.rs`:\n- `fn resolve_run_github_permissions(run: &RunNamespace) -> HashMap` — InterpString resolution loop, same as the server side. Reuse server-side helper if one exists; otherwise extract a shared one in fabro-config or fabro-server.\n- `fn requires_github_credentials(run: &RunNamespace, server: Option<&ServerNamespace>) -> bool` — folds the existing clone/PR/permissions gates.\n\nBoth are pure functions of resolved settings; unit-test them in `#[cfg(test)] mod tests` inside `runner.rs`. Don't try to reach private items from an integration test in `tests/it/`.\n\nWithout this fix, CLI-launched runs silently get no `GITHUB_TOKEN` regardless of TOML.\n\nDownstream is unchanged: `StartServices.github_permissions` → `SandboxEnvSpec.github_permissions` (`fabro-workflow/src/operations/start.rs:99`, `pipeline/types.rs:225`) → `mint_github_token` + `GITHUB_TOKEN` injection at `pipeline/initialize.rs:240`.\n\n### 7. Parse hints — `lib/crates/fabro-config/src/parse.rs:117-118`\n\nUpdate the legacy-`[github]` migration hint to distinguish identity vs. permissions. Suggested wording:\n- `\"github\"` → `\"split into [server.integrations.github] (App identity/auth) and [run.integrations.github.permissions] (sandbox token scopes)\"`.\n\nAlso: ensure `[server.integrations.github]` with a `permissions` subkey produces a `deny_unknown_fields` error pointing at the new path. If automatic, no extra code; if not, add a targeted parse-time check.\n\n### 8. OpenAPI — `docs/public/api-reference/fabro-api.yaml`\n\nWire shape mirrors the resolved Rust types (no `Option`s, no nullability):\n\n- **Remove** `permissions` from `GithubIntegrationSettings` (line 7212): drop the property and remove from `required`.\n- **Add** new schemas:\n - `RunIntegrationsSettings`: `properties: { github: { $ref: \"#/components/schemas/RunIntegrationsGithubSettings\" } }`, `required: [\"github\"]`.\n - `RunIntegrationsGithubSettings`: `properties: { permissions: { type: object, additionalProperties: { type: string } } }`, `required: [\"permissions\"]`.\n- **Add** `integrations: { $ref: \"#/components/schemas/RunIntegrationsSettings\" }` to `RunNamespace.properties` and to `RunNamespace.required`.\n\nThe collapsed-Option resolved types (section 3) make this clean: `github` is always present, `permissions` is always an object (possibly empty). No `nullable`, no `oneOf`, no `skip_serializing_if` to debate.\n\nType ownership: progenitor auto-generates new run DTOs (Explore confirmed no `with_replacement` for run types today). No new `with_replacement` entries; do not split into hand-rolled DTOs unless a real semantic divergence emerges. Add a fabro-api JSON parity test asserting OpenAPI's `RunIntegrationsGithubSettings` round-trips through the resolved Rust type, including the empty-permissions case.\n\nRegenerate TS client: `cd lib/packages/fabro-api-client && bun run generate`.\n\n### 9. Repo TOMLs — rewrite\n\n- `.fabro/workflows/gh-triage/workflow.toml`\n- `.fabro/workflows/implement-issue/workflow.toml`\n- `.fabro/workflows/gh-list/workflow.toml`\n\nEach: `[server.integrations.github.permissions]` → `[run.integrations.github.permissions]`.\n\n### 10. User settings (advisory)\n\n`~/.fabro/settings.toml` should be rewritten to put a default at `[run.integrations.github.permissions]`. Don't auto-edit; cover in verification.\n\n### 11. Docs — `docs/public/integrations/github.mdx`\n\n- Line 35 (overview table): rewrite the row to reference `[run.integrations.github.permissions]`.\n- Lines 209-220 (\"GITHUB_TOKEN injection\"): rewrite. Show canonical run-level path. Note `~/.fabro/settings.toml` is the natural place for server defaults because it's the user layer of the same merge stack. Drop the `[github]` shorthand from line 211 (never existed in code).\n- Add the security-model note (boundary = installation grants, no Fabro-side cap).\n\n### 12. Tests\n\n**Layer parsing + merge** (`lib/crates/fabro-config/src/tests/`, parse real TOML at each layer; not just resolver-level fixtures):\n- Parse a workflow.toml with `[run.integrations.github.permissions]` — assert the layer round-trips.\n- Merge user `{ contents = \"read\" }` + workflow `{ issues = \"write\" }` — assert workflow fully replaces: result is `{ issues = \"write\" }`.\n- Merge user `{ contents = \"read\" }` + workflow absent (no `[run.integrations]` block) — assert inheritance: result is `{ contents = \"read\" }`.\n- Merge user `{ contents = \"read\" }` + workflow `permissions = {}` — assert clear: resolved permissions is empty.\n- Negative: `[server.integrations.github.permissions]` in user/project/workflow TOML must error via `deny_unknown_fields`.\n- Negative (bundled workflow path, P2): targeted test exercising `root_workflow_run_layer` (`run_manifest.rs:290`) with a workflow.toml containing a stale `[server.integrations.github.permissions]` block — assert it errors via `deny_unknown_fields` after the rewrite to parse through `SettingsLayer`. Pair with a positive test asserting `_version = 1` and a `[workflow]` block still parse cleanly through this code path.\n\n**Resolver** (`resolve/run.rs` tests): assert `InterpString` is preserved in resolved settings, not flattened to `String`. Assert resolved `permissions` is `HashMap` (Option collapsed).\n\n**Preflight** (`run_manifest.rs:1822` and surrounds):\n- Add a case where the run config sets `permissions = { issues = \"read\" }` and the GitHub App is configured — assert `run_github_token_check` reports `Pass`.\n- Update `server_settings_fixture` (`run_manifest.rs:1390`) usage in any test that previously set permissions through it; rewrite to set via run layer.\n\n**CLI worker path** (P0): unit-test the new private helpers in `runner.rs`'s `#[cfg(test)] mod tests`:\n- `resolve_run_github_permissions` — given a `RunNamespace` with `InterpString` permissions referencing env vars, returns the resolved `HashMap`.\n- `requires_github_credentials` — exercises each truth-table case (clone needed, PR enabled, permissions non-empty, all-absent).\n- Do not try to assert `StartServices.github_permissions` from an integration test — those internals are private. End-to-end behavior is covered by the smoke runs in Verification.\n\n## Verification\n\n1. `cargo build --workspace` clean.\n2. `cargo nextest run -p fabro-config` — layer/resolve tests pass, including the new override + replace-semantics + deny-unknown tests.\n3. `cargo nextest run -p fabro-server -p fabro-cli` — preflight + run-manifest + worker tests pass.\n4. `cargo nextest run -p fabro-api` — JSON parity test for the new run integrations schema passes.\n5. Smoke run via server (manual):\n - Update `~/.fabro/settings.toml` to put `[run.integrations.github.permissions]` defaults (`pull_requests = \"read\"`, `issues = \"read\"`).\n - Restart `fabro server`.\n - `fabro run gh-list --no-retro`; both stages exit 0 with PR/issue listings on stdout.\n - `fabro logs ` shows `gh pr list` returning data; no \"populate the GH_TOKEN\" error.\n6. Smoke run via CLI worker path (manual): same as above but with a `fabro run` invocation that uses the local-CLI worker path (not HTTP). Confirms P0 fix.\n7. Override test: in `gh-list/workflow.toml`, set `permissions = { issues = \"write\" }` over a server default of `read`. Confirm the minted-token preflight summary shows `issues: write` and not `read`.\n8. Tightening test: in another workflow, set `permissions = {}`. Confirm preflight reports no token requested and the sandbox env has no `GITHUB_TOKEN`.\n9. `cd apps/fabro-web && bun run typecheck && bun test` — generated TS client compiles against the new schema.\n\n## Open questions\n\n1. Anything else worth promoting to `[run.integrations.*]` now (Slack, Discord run-time config)? Recommendation: leave empty until a concrete need lands; don't speculate.\n2. Should preflight surface the *resolved* permission strings in its report, or the raw `InterpString` source? Recommendation: resolved, so reviewers see what the App will actually be asked for; treat unresolved env-var fallbacks as a preflight warning.\n3. `resolve_run_github_permissions` location — should the InterpString-resolution loop live in fabro-config (shared by server + CLI worker), or in each consumer? Recommendation: extract to fabro-config / fabro-types alongside the resolved type, so server (`server.rs:2763-2772`) and CLI (`runner.rs`) both call the same helper. Avoids two copies drifting.\n", @@ -537,7 +538,9 @@ "internal.node_visit_count": 1, "internal.run_id": "01KR25M0Q1VARG70MW2Y88MK0K", "internal.retry_count.preflight_compile": 0, + "thread.preflight_compile.current_node": "preflight_lint", "internal.retry_count.toolchain": 0, + "internal.retry_count.preflight_lint": 0, "command.stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126" }, "node_outcomes": { @@ -554,6 +557,15 @@ "status": "succeeded", "usage": null }, + "preflight_lint": { + "status": "succeeded", + "context_updates": { + "command.stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "command.output": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126" + }, + "notes": "Script completed: cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", + "usage": null + }, "toolchain": { "status": "succeeded", "context_updates": { @@ -564,8 +576,9 @@ "usage": null } }, - "next_node_id": "preflight_lint", + "next_node_id": "implement", "node_visits": { + "preflight_lint": 1, "toolchain": 1, "start": 1, "preflight_compile": 1 @@ -659,6 +672,71 @@ "start": 1 } } + ], + [ + 36, + { + "timestamp": "2026-05-07T21:33:53.007362Z", + "current_node": "preflight_compile", + "completed_nodes": [ + "start", + "toolchain", + "preflight_compile" + ], + "node_retries": {}, + "context_values": { + "command.output": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "failure_class": "", + "failure_signature": "", + "outcome": "succeeded", + "command.stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "current_node": "preflight_compile", + "graph.goal": "# Move GitHub token permissions from `[server.integrations.github]` to `[run.integrations.github]`\n\n## Context\n\nToday `[server.integrations.github.permissions]` controls what scopes Fabro requests on the Installation Access Token it injects into the sandbox as `GITHUB_TOKEN`. This is conceptually wrong:\n\n- Token permissions describe what *this run* is authorized to do, not the server's identity.\n- The current key is server-only. `workflow.toml` and `project.toml` cannot override it (`builders.rs:394` strips `server.*` from per-workflow layers), so projects/workflows can't tighten or relax permissions.\n- The public docs (`integrations/github.mdx:211-220`) already describe a per-run config, which never existed in code. The intent has always been run-level.\n\nGreenfield app — no migration, no backwards compat. Moving permissions to `[run.integrations.github.permissions]` makes the natural layer-merge (workflow > project > user > defaults) Just Work: server admins set defaults in `~/.fabro/settings.toml`, projects and workflows override.\n\n## Security model\n\nRun config is trusted policy input for sandbox token scopes. The upper bound on what Fabro can mint is the GitHub App installation's granted permissions; Fabro does **not** impose a separate server-side cap. Operators must not run untrusted workflow/project/user TOML against a broadly-scoped App installation. Preflight prints requested permissions so reviewers can see them; no enforcement layer beyond GitHub's own.\n\n## Design\n\nNew TOML path: `[run.integrations.github.permissions]`. Creates a fresh `[run.integrations]` namespace for future run-level integration knobs.\n\n**Merge semantics** (presence-aware, hand-rolled). `ReplaceMap` does NOT work here: `maps.rs:76-80` is `if self.0.is_empty() { other } else { self }`, i.e. an empty higher layer falls back to the inherited map. We need empty-wins-as-clear, so we don't reuse `ReplaceMap` (and don't change global semantics for other users).\n\nLayer field: `pub permissions: Option>`.\n\n| Higher layer | Lower layer | Result |\n|---|---|---|\n| `None` | anything | lower (inherit) |\n| `Some(map)` | anything | `Some(map)` (full replace, including `Some({})` = clear) |\n\nHand-roll `Combine` on `RunIntegrationsGithubLayer`:\n```rust\nimpl Combine for RunIntegrationsGithubLayer {\n fn combine(self, other: Self) -> Self {\n Self { permissions: self.permissions.or(other.permissions) }\n }\n}\n```\nDon't derive — the blanket `Option` impl recurses into the inner type and would reintroduce the empty-fallback bug.\n\nResolved type collapses the Option: `pub permissions: HashMap` where empty = no token requested. The presence distinction only matters during merge.\n\n**Interpolation**: keep `InterpString` in the resolved type. Resolve to `String` at the start-services construction boundary, matching the existing pattern at `server.rs:2763-2772`. No early resolution in `resolve_run`.\n\n## Changes\n\n### 1. Config schema — `lib/crates/fabro-config/src/layers/run.rs`\n\nAdd new layer types. `RunIntegrationsLayer` derives `Combine` normally; `RunIntegrationsGithubLayer` does NOT — hand-roll `Combine` (see Design section) so `Some({})` is honored as a clear sentinel.\n\n```rust\n/// `[run.integrations]` — run-level integration knobs.\n#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)]\n#[serde(deny_unknown_fields)]\npub struct RunIntegrationsLayer {\n #[serde(default, skip_serializing_if = \"Option::is_none\")]\n pub github: Option,\n}\n\n/// `[run.integrations.github]` — runtime GitHub token shape.\n#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]\n#[serde(deny_unknown_fields)]\npub struct RunIntegrationsGithubLayer {\n #[serde(default, skip_serializing_if = \"Option::is_none\")]\n pub permissions: Option>,\n}\n\nimpl Combine for RunIntegrationsGithubLayer {\n fn combine(self, other: Self) -> Self {\n Self { permissions: self.permissions.or(other.permissions) }\n }\n}\n```\n\nAdd `pub integrations: Option` to `RunLayer`.\n\n### 2. Config schema — server side\n\n`lib/crates/fabro-config/src/layers/server.rs:226`: remove `permissions: StickyMap` from `GithubIntegrationLayer`. Server struct keeps only identity/auth/webhook fields.\n\n### 3. Resolved types — `lib/crates/fabro-types/src/settings/run.rs`\n\nResolved types collapse the layer-time `Option` (presence is only meaningful during merge):\n\n```rust\npub struct RunIntegrationsSettings {\n pub github: RunIntegrationsGithubSettings,\n}\n\npub struct RunIntegrationsGithubSettings {\n pub permissions: HashMap, // empty = no token\n}\n```\n\nAdd `integrations: RunIntegrationsSettings` to `RunNamespace`. Drop `permissions` from the resolved `GithubIntegrationSettings` in fabro-types.\n\n### 4. Resolver — `lib/crates/fabro-config/src/resolve/run.rs`\n\nAdd `fn resolve_integrations(layer: Option<&RunIntegrationsLayer>) -> RunIntegrationsSettings`. Pass through `InterpString`s untouched — do NOT resolve env vars here. Collapse `Option>` → `HashMap<...>` (None and Some({}) both become empty).\n\n`lib/crates/fabro-config/src/resolve/server.rs:465`: drop the line that copies `permissions` into the resolved github settings.\n\n### 5. Server consumer — `lib/crates/fabro-server`\n\nFour read sites swap from `server_settings.server.integrations.github.permissions` to `run_spec.settings.run.integrations.github.permissions` (a flat `HashMap`; empty = no token):\n\n- `run_manifest.rs:488` — clone-credential gate inside `prepare_manifest`.\n- `run_manifest.rs:1184-1235` — `run_github_token_check`. Change signature to take resolved run permissions; resolve `InterpString`s inside the function for both minting and the report.\n- `server.rs:2727` — forced-credential gate in run launch path.\n- `server.rs:2763-2772` — InterpString → String resolution for `StartServices.github_permissions`. Read from run settings instead of server settings; same resolution logic.\n\n### 5b. Bundled-workflow TOML parsing — `lib/crates/fabro-server/src/run_manifest.rs:290-307`\n\n`root_workflow_run_layer` currently parses the workflow TOML as a raw `toml::Table`, lifts out `run`, and silently discards every other top-level key. That means stale `[server.integrations.github.permissions]` is not caught by `deny_unknown_fields`.\n\nNaive \"reject any non-`run` key\" would break valid workflow TOML (every workflow.toml in the repo has `_version = 1`; `hello/workflow.toml` has `[workflow]`). `SettingsLayer` (`layers/settings.rs:21-37`) is the schema for a settings file: `_version`, `project`, `workflow`, `run`, `cli`, `server`, `features`.\n\nFix: parse the source via `source.parse::()` and take `layer.run.unwrap_or_default()`. That gives:\n- valid top-level domains parse without error;\n- stale `[server.integrations.github.permissions]` fails because `permissions` is no longer a known field on `GithubIntegrationLayer` (section 2) and the layer has `serde(deny_unknown_fields)`;\n- shape-valid `[server.*]` in a workflow.toml is silently ignored downstream by `builders.rs:394`, preserving today's behavior (workflow.toml shouldn't set server config, but doesn't blow up either).\n\n`resolve_manifest_dockerfile(&mut run, &config.path, &workflow.files)` (line 305) still runs on the extracted `RunLayer`.\n\n### 6. CLI worker — `lib/crates/fabro-cli/src/commands/run/runner.rs` (P0, was missing)\n\nTwo reads on the CLI launch path that today still point at the server-side field:\n\n- `runner.rs:120` — `github_permissions: HashMap::new()` is hardcoded. Source from `run_spec.settings.run.integrations.github.permissions`, applying the same `InterpString` → `String` resolution as the server side.\n- `runner.rs:514-555` — `maybe_build_github_credentials`. Line 524 reads `settings.server.integrations.github.permissions.is_empty()` to decide whether credentials are required. Swap to read the run-level permissions. Identity fields (`strategy`, `app_id`, `slug`) at lines 527-538 stay on the server side.\n\nTo make this testable, factor two private helpers in `runner.rs`:\n- `fn resolve_run_github_permissions(run: &RunNamespace) -> HashMap` — InterpString resolution loop, same as the server side. Reuse server-side helper if one exists; otherwise extract a shared one in fabro-config or fabro-server.\n- `fn requires_github_credentials(run: &RunNamespace, server: Option<&ServerNamespace>) -> bool` — folds the existing clone/PR/permissions gates.\n\nBoth are pure functions of resolved settings; unit-test them in `#[cfg(test)] mod tests` inside `runner.rs`. Don't try to reach private items from an integration test in `tests/it/`.\n\nWithout this fix, CLI-launched runs silently get no `GITHUB_TOKEN` regardless of TOML.\n\nDownstream is unchanged: `StartServices.github_permissions` → `SandboxEnvSpec.github_permissions` (`fabro-workflow/src/operations/start.rs:99`, `pipeline/types.rs:225`) → `mint_github_token` + `GITHUB_TOKEN` injection at `pipeline/initialize.rs:240`.\n\n### 7. Parse hints — `lib/crates/fabro-config/src/parse.rs:117-118`\n\nUpdate the legacy-`[github]` migration hint to distinguish identity vs. permissions. Suggested wording:\n- `\"github\"` → `\"split into [server.integrations.github] (App identity/auth) and [run.integrations.github.permissions] (sandbox token scopes)\"`.\n\nAlso: ensure `[server.integrations.github]` with a `permissions` subkey produces a `deny_unknown_fields` error pointing at the new path. If automatic, no extra code; if not, add a targeted parse-time check.\n\n### 8. OpenAPI — `docs/public/api-reference/fabro-api.yaml`\n\nWire shape mirrors the resolved Rust types (no `Option`s, no nullability):\n\n- **Remove** `permissions` from `GithubIntegrationSettings` (line 7212): drop the property and remove from `required`.\n- **Add** new schemas:\n - `RunIntegrationsSettings`: `properties: { github: { $ref: \"#/components/schemas/RunIntegrationsGithubSettings\" } }`, `required: [\"github\"]`.\n - `RunIntegrationsGithubSettings`: `properties: { permissions: { type: object, additionalProperties: { type: string } } }`, `required: [\"permissions\"]`.\n- **Add** `integrations: { $ref: \"#/components/schemas/RunIntegrationsSettings\" }` to `RunNamespace.properties` and to `RunNamespace.required`.\n\nThe collapsed-Option resolved types (section 3) make this clean: `github` is always present, `permissions` is always an object (possibly empty). No `nullable`, no `oneOf`, no `skip_serializing_if` to debate.\n\nType ownership: progenitor auto-generates new run DTOs (Explore confirmed no `with_replacement` for run types today). No new `with_replacement` entries; do not split into hand-rolled DTOs unless a real semantic divergence emerges. Add a fabro-api JSON parity test asserting OpenAPI's `RunIntegrationsGithubSettings` round-trips through the resolved Rust type, including the empty-permissions case.\n\nRegenerate TS client: `cd lib/packages/fabro-api-client && bun run generate`.\n\n### 9. Repo TOMLs — rewrite\n\n- `.fabro/workflows/gh-triage/workflow.toml`\n- `.fabro/workflows/implement-issue/workflow.toml`\n- `.fabro/workflows/gh-list/workflow.toml`\n\nEach: `[server.integrations.github.permissions]` → `[run.integrations.github.permissions]`.\n\n### 10. User settings (advisory)\n\n`~/.fabro/settings.toml` should be rewritten to put a default at `[run.integrations.github.permissions]`. Don't auto-edit; cover in verification.\n\n### 11. Docs — `docs/public/integrations/github.mdx`\n\n- Line 35 (overview table): rewrite the row to reference `[run.integrations.github.permissions]`.\n- Lines 209-220 (\"GITHUB_TOKEN injection\"): rewrite. Show canonical run-level path. Note `~/.fabro/settings.toml` is the natural place for server defaults because it's the user layer of the same merge stack. Drop the `[github]` shorthand from line 211 (never existed in code).\n- Add the security-model note (boundary = installation grants, no Fabro-side cap).\n\n### 12. Tests\n\n**Layer parsing + merge** (`lib/crates/fabro-config/src/tests/`, parse real TOML at each layer; not just resolver-level fixtures):\n- Parse a workflow.toml with `[run.integrations.github.permissions]` — assert the layer round-trips.\n- Merge user `{ contents = \"read\" }` + workflow `{ issues = \"write\" }` — assert workflow fully replaces: result is `{ issues = \"write\" }`.\n- Merge user `{ contents = \"read\" }` + workflow absent (no `[run.integrations]` block) — assert inheritance: result is `{ contents = \"read\" }`.\n- Merge user `{ contents = \"read\" }` + workflow `permissions = {}` — assert clear: resolved permissions is empty.\n- Negative: `[server.integrations.github.permissions]` in user/project/workflow TOML must error via `deny_unknown_fields`.\n- Negative (bundled workflow path, P2): targeted test exercising `root_workflow_run_layer` (`run_manifest.rs:290`) with a workflow.toml containing a stale `[server.integrations.github.permissions]` block — assert it errors via `deny_unknown_fields` after the rewrite to parse through `SettingsLayer`. Pair with a positive test asserting `_version = 1` and a `[workflow]` block still parse cleanly through this code path.\n\n**Resolver** (`resolve/run.rs` tests): assert `InterpString` is preserved in resolved settings, not flattened to `String`. Assert resolved `permissions` is `HashMap` (Option collapsed).\n\n**Preflight** (`run_manifest.rs:1822` and surrounds):\n- Add a case where the run config sets `permissions = { issues = \"read\" }` and the GitHub App is configured — assert `run_github_token_check` reports `Pass`.\n- Update `server_settings_fixture` (`run_manifest.rs:1390`) usage in any test that previously set permissions through it; rewrite to set via run layer.\n\n**CLI worker path** (P0): unit-test the new private helpers in `runner.rs`'s `#[cfg(test)] mod tests`:\n- `resolve_run_github_permissions` — given a `RunNamespace` with `InterpString` permissions referencing env vars, returns the resolved `HashMap`.\n- `requires_github_credentials` — exercises each truth-table case (clone needed, PR enabled, permissions non-empty, all-absent).\n- Do not try to assert `StartServices.github_permissions` from an integration test — those internals are private. End-to-end behavior is covered by the smoke runs in Verification.\n\n## Verification\n\n1. `cargo build --workspace` clean.\n2. `cargo nextest run -p fabro-config` — layer/resolve tests pass, including the new override + replace-semantics + deny-unknown tests.\n3. `cargo nextest run -p fabro-server -p fabro-cli` — preflight + run-manifest + worker tests pass.\n4. `cargo nextest run -p fabro-api` — JSON parity test for the new run integrations schema passes.\n5. Smoke run via server (manual):\n - Update `~/.fabro/settings.toml` to put `[run.integrations.github.permissions]` defaults (`pull_requests = \"read\"`, `issues = \"read\"`).\n - Restart `fabro server`.\n - `fabro run gh-list --no-retro`; both stages exit 0 with PR/issue listings on stdout.\n - `fabro logs ` shows `gh pr list` returning data; no \"populate the GH_TOKEN\" error.\n6. Smoke run via CLI worker path (manual): same as above but with a `fabro run` invocation that uses the local-CLI worker path (not HTTP). Confirms P0 fix.\n7. Override test: in `gh-list/workflow.toml`, set `permissions = { issues = \"write\" }` over a server default of `read`. Confirm the minted-token preflight summary shows `issues: write` and not `read`.\n8. Tightening test: in another workflow, set `permissions = {}`. Confirm preflight reports no token requested and the sandbox env has no `GITHUB_TOKEN`.\n9. `cd apps/fabro-web && bun run typecheck && bun test` — generated TS client compiles against the new schema.\n\n## Open questions\n\n1. Anything else worth promoting to `[run.integrations.*]` now (Slack, Discord run-time config)? Recommendation: leave empty until a concrete need lands; don't speculate.\n2. Should preflight surface the *resolved* permission strings in its report, or the raw `InterpString` source? Recommendation: resolved, so reviewers see what the App will actually be asked for; treat unresolved env-var fallbacks as a preflight warning.\n3. `resolve_run_github_permissions` location — should the InterpString-resolution loop live in fabro-config (shared by server + CLI worker), or in each consumer? Recommendation: extract to fabro-config / fabro-types alongside the resolved type, so server (`server.rs:2763-2772`) and CLI (`runner.rs`) both call the same helper. Avoids two copies drifting.\n", + "internal.work_dir": "/home/daytona/workspace", + "internal.thread_id": "toolchain", + "internal.retry_count.start": 0, + "internal.retry_count.toolchain": 0, + "graph.rankdir": "LR", + "graph.model_stylesheet": "\n * { model: claude-opus-4-7; }\n ", + "internal.node_visit_count": 1, + "internal.run_id": "01KR25M0Q1VARG70MW2Y88MK0K", + "thread.toolchain.current_node": "preflight_compile", + "thread.start.current_node": "toolchain", + "internal.fidelity": "compact", + "internal.retry_count.preflight_compile": 0 + }, + "node_outcomes": { + "toolchain": { + "status": "succeeded", + "context_updates": { + "command.output": "blob://sha256/fc14b2ba2d770e5cd3169df7a29525c962adfc4cfa3097b9098c63ebd61a748c", + "command.stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126" + }, + "notes": "Script completed: command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", + "usage": null + }, + "preflight_compile": { + "status": "succeeded", + "context_updates": { + "command.stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "command.output": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126" + }, + "notes": "Script completed: cargo check -q --workspace 2>&1", + "usage": null + }, + "start": { + "status": "succeeded", + "usage": null + } + }, + "next_node_id": "preflight_lint", + "git_commit_sha": "157fd1d631e2ec88495ef2a86130d3d93f7faf3a", + "node_visits": { + "preflight_compile": 1, + "start": 1, + "toolchain": 1 + } + } ] ], "conclusion": null, @@ -718,6 +796,65 @@ "duration_ms": 2471, "state": "succeeded" }, + "preflight_compile@1": { + "first_event_seq": 29, + "prompt": null, + "response": null, + "completion": { + "outcome": "succeeded", + "notes": "Script completed: cargo check -q --workspace 2>&1", + "failure_reason": null, + "timestamp": "2026-05-07T21:33:43.302780Z" + }, + "provider_used": null, + "diff": null, + "script_invocation": { + "script": "cargo check -q --workspace 2>&1", + "command": "cargo check -q --workspace 2>&1", + "language": "shell" + }, + "script_timing": { + "stdout": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "exit_code": 0, + "duration_ms": 122920, + "termination": "exited", + "stdout_bytes": 0, + "stderr_bytes": 0, + "streams_separated": true, + "live_streaming": false + }, + "parallel_results": null, + "stdout": null, + "stderr": null, + "stdout_bytes": 0, + "stderr_bytes": 0, + "streams_separated": true, + "live_streaming": false, + "termination": "exited", + "started_at": "2026-05-07T21:31:40.364101Z", + "duration_ms": 122938, + "state": "succeeded" + }, + "preflight_lint@1": { + "first_event_seq": 39, + "prompt": null, + "response": null, + "completion": null, + "provider_used": null, + "diff": null, + "script_invocation": { + "script": "cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", + "command": "cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", + "language": "shell" + }, + "script_timing": null, + "parallel_results": null, + "stdout": null, + "stderr": null, + "started_at": "2026-05-07T21:33:53.012397Z", + "state": "running" + }, "start@1": { "first_event_seq": 15, "prompt": null, @@ -738,25 +875,6 @@ "started_at": "2026-05-07T21:31:30.861067Z", "duration_ms": 0, "state": "succeeded" - }, - "preflight_compile@1": { - "first_event_seq": 29, - "prompt": null, - "response": null, - "completion": null, - "provider_used": null, - "diff": null, - "script_invocation": { - "script": "cargo check -q --workspace 2>&1", - "command": "cargo check -q --workspace 2>&1", - "language": "shell" - }, - "script_timing": null, - "parallel_results": null, - "stdout": null, - "stderr": null, - "started_at": "2026-05-07T21:31:40.364101Z", - "state": "running" } } } \ No newline at end of file diff --git a/stages/003-preflight_compile@1/script_timing.json b/stages/003-preflight_compile@1/script_timing.json new file mode 100644 index 000000000..4b58292b2 --- /dev/null +++ b/stages/003-preflight_compile@1/script_timing.json @@ -0,0 +1,11 @@ +{ + "stdout": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126", + "exit_code": 0, + "duration_ms": 122920, + "termination": "exited", + "stdout_bytes": 0, + "stderr_bytes": 0, + "streams_separated": true, + "live_streaming": false +} \ No newline at end of file diff --git a/stages/003-preflight_compile@1/status.json b/stages/003-preflight_compile@1/status.json new file mode 100644 index 000000000..1e96fa99d --- /dev/null +++ b/stages/003-preflight_compile@1/status.json @@ -0,0 +1,6 @@ +{ + "outcome": "succeeded", + "notes": "Script completed: cargo check -q --workspace 2>&1", + "failure_reason": null, + "timestamp": "2026-05-07T21:33:43.302780Z" +} \ No newline at end of file diff --git a/stages/003-preflight_compile@1/stderr.log b/stages/003-preflight_compile@1/stderr.log new file mode 100644 index 000000000..d87ba9545 --- /dev/null +++ b/stages/003-preflight_compile@1/stderr.log @@ -0,0 +1 @@ +blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126 \ No newline at end of file diff --git a/stages/003-preflight_compile@1/stdout.log b/stages/003-preflight_compile@1/stdout.log new file mode 100644 index 000000000..d87ba9545 --- /dev/null +++ b/stages/003-preflight_compile@1/stdout.log @@ -0,0 +1 @@ +blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126 \ No newline at end of file diff --git a/stages/004-preflight_lint@1/script_invocation.json b/stages/004-preflight_lint@1/script_invocation.json new file mode 100644 index 000000000..a6d5e8337 --- /dev/null +++ b/stages/004-preflight_lint@1/script_invocation.json @@ -0,0 +1,5 @@ +{ + "script": "cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", + "command": "cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", + "language": "shell" +} \ No newline at end of file