clawmates/crates/cm-api/src/phase_config.rs

//! Which phase-config keys the platform actually reads.
//!
//! `mission_phases.config` is free-form JSONB written by workflow recipes, the
//! mission wizard and the API. Nothing connected a key to the code that reads
//! it, so a key could be accepted, validated, stored, rendered — and consumed
//! by nobody.
//!
//! `task` was exactly that. Every phase of every mission received identical
//! instructions because the runner selected only the mission description; the
//! per-phase task sat in Postgres unread. Mission `019fc42b` is what surfaced
//! it: two coding phases with different `task` values produced the same two
//! files. There was no error, because there is nothing to fail — an unread key
//! is indistinguishable from a key whose value happens not to matter.
//!
//! This module is the missing link. Every key here names the code that reads
//! it, `unknown_keys` reports anything else, and a test asserts the shipped
//! recipes only write keys that exist. It cannot make a reader appear, but it
//! makes an absent one visible.

/// A phase-config key and where it is consumed.
pub struct KnownKey {
    pub key: &'static str,
    /// The code path that reads it. Kept as prose so this survives refactors
    /// that a symbol reference would not.
    pub read_by: &'static str,
}

/// Keys with a reader in the current build.
///
/// Adding a key here without a reader defeats the purpose. The rule is: a key
/// earns its entry when something consumes it, not when something writes it.
pub const KNOWN_KEYS: &[KnownKey] = &[
    KnownKey {
        key: "done_when",
        read_by: "cm_db::repo::missions::create — promoted to the done_when column, \
                  swept by phase_runner::evaluate_finished_phases",
    },
    KnownKey {
        key: "max_iterations",
        read_by: "cm_db::repo::missions::create — promoted to the max_iterations column",
    },
    KnownKey {
        key: "task",
        read_by: "phase_runner::start_pending_phases — injected by phase_task_text",
    },
    KnownKey {
        key: "commit_policy",
        read_by: "mission_delivery::Gate::parse — selects the delivery gate",
    },
    KnownKey {
        key: "allow_empty",
        read_by: "phase_runner::empty_delivery_is_a_failure — when true, a coding \
                  phase that changes no files still completes; also vm_stop_gate::\
                  StopGate::for_phase, where it drops the in-loop delivery check",
    },
    KnownKey {
        key: "tools",
        read_by: "security_scan::run — gates which of cargo_audit / gitleaks / \
                  trivy_fs / semgrep run against the phase's checkout; absent \
                  means all four. Listed here as NOT IMPLEMENTED while wired, \
                  which understated the recipe: the key was real, what was \
                  missing was anything that FIRED the scan outside an operator \
                  button — now phase_runner::scan_finished_security_phases",
    },
    KnownKey {
        key: "harness",
        read_by: "benchmark_runner::harness_from_config — selects criterion / \
                  cargo_bench / vitest_bench / pytest_bench / shell, with \
                  `bench_name` (criterion) and `cmd` (shell) as its arguments. \
                  phase_runner's benchmark sweep runs the baseline through it. \
                  This key was listed as NOT IMPLEMENTED while being fully \
                  wired, which is worse than an unread key: the registry exists \
                  so an operator can trust what a recipe does, and it was wrong",
    },
    KnownKey {
        key: "bench_name",
        read_by: "benchmark_runner::harness_from_config — the criterion bench target",
    },
    KnownKey {
        key: "cmd",
        read_by: "benchmark_runner::harness_from_config — the shell harness command line",
    },
    KnownKey {
        key: "done_when_check",
        read_by: "vm_stop_gate::StopGate::for_phase — a shell command the agent's \
                  `Stop` hook runs, refusing the stop while it exits non-zero",
    },
];

/// Keys a recipe may carry that are deliberately not consumed *yet*.
///
/// Distinguished from unknown keys so the report stays useful: these are known
/// gaps with an owner, not typos. Every one is a feature described in a shipped
/// workflow recipe whose implementation does not exist — which is worth seeing
/// listed, because a recipe promising `loop = "until_done"` reads to an
/// operator like something that loops.
pub const DECLARED_BUT_UNREAD: &[KnownKey] = &[
    KnownKey {
        key: "loop",
        read_by: "NOT IMPLEMENTED — phase iteration uses max_iterations + done_when",
    },
    KnownKey {
        key: "produces",
        read_by: "NOT IMPLEMENTED — artifact rendering is not driven by this",
    },
    KnownKey {
        key: "input_from_phase",
        read_by: "NOT IMPLEMENTED — phases share a checkout, not declared inputs",
    },
    KnownKey {
        key: "mode",
        read_by: "NOT IMPLEMENTED — benchmark/refactor mode selection",
    },
    KnownKey {
        key: "benchmark",
        read_by: "NOT IMPLEMENTED — nested benchmark settings",
    },
    KnownKey {
        key: "mcp_bundles",
        read_by: "NOT IMPLEMENTED at phase level — bundles come from the TEAM \
                  template (mission_orchestrator binds template.mcp_bundles) and \
                  runtime_provision writes agents.<alias>.mcp_bundles. A recipe \
                  setting this per phase changes nothing: security_hardening.toml \
                  asks for gitea_forge + security_scan and its phase gets neither",
    },
];

fn is_listed(key: &str, list: &[KnownKey]) -> bool {
    list.iter().any(|k| k.key == key)
}

/// Keys in this config that no code reads and that are not known gaps.
///
/// Almost always a typo or a setting invented for a feature that was never
/// built. Returned rather than rejected: a mission whose config carries an
/// unread key is not *wrong*, it is just doing less than its author believes,
/// and failing the request would break recipes that already ship these.
pub fn unknown_keys(config: &serde_json::Value) -> Vec<String> {
    let Some(obj) = config.as_object() else {
        return Vec::new();
    };
    obj.keys()
        .filter(|k| !is_listed(k, KNOWN_KEYS) && !is_listed(k, DECLARED_BUT_UNREAD))
        .cloned()
        .collect()
}

/// Keys that are recognised but that nothing consumes.
pub fn inert_keys(config: &serde_json::Value) -> Vec<String> {
    let Some(obj) = config.as_object() else {
        return Vec::new();
    };
    obj.keys()
        .filter(|k| is_listed(k, DECLARED_BUT_UNREAD))
        .cloned()
        .collect()
}

/// Log what a phase's config asked for that will not happen.
///
/// Called once per phase at mission creation. Deliberately not an error: the
/// point is that the author's intent and the platform's behaviour have
/// diverged, and the author should be able to see that without being blocked.
pub fn report(kind: &str, order_idx: i32, config: &serde_json::Value) {
    let unknown = unknown_keys(config);
    if !unknown.is_empty() {
        eprintln!(
            "phase_config: phase {order_idx} ({kind}) sets unrecognised key(s) {} — \
             nothing reads them; check for a typo",
            unknown.join(", ")
        );
    }
    let inert = inert_keys(config);
    if !inert.is_empty() {
        eprintln!(
            "phase_config: phase {order_idx} ({kind}) sets {} — recognised but NOT \
             IMPLEMENTED, so it will have no effect on this run",
            inert.join(", ")
        );
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn a_key_cannot_be_both_read_and_unread() {
        for k in KNOWN_KEYS {
            assert!(
                !is_listed(k.key, DECLARED_BUT_UNREAD),
                "{} is listed as both read and unread",
                k.key
            );
        }
    }

    #[test]
    fn every_known_key_names_its_reader() {
        for k in KNOWN_KEYS {
            assert!(
                !k.read_by.is_empty() && !k.read_by.starts_with("NOT IMPLEMENTED"),
                "{} claims to be read but names no reader",
                k.key
            );
        }
        for k in DECLARED_BUT_UNREAD {
            assert!(
                k.read_by.starts_with("NOT IMPLEMENTED"),
                "{} is listed as unread but names a reader — promote it to KNOWN_KEYS",
                k.key
            );
        }
    }

    /// The regression that motivated the module: `task` must stay claimed.
    #[test]
    fn the_per_phase_task_key_has_a_reader() {
        assert!(
            is_listed("task", KNOWN_KEYS),
            "task lost its reader again — every phase will get identical instructions"
        );
    }

    #[test]
    fn unknown_and_inert_keys_are_reported_separately() {
        let cfg = serde_json::json!({
            "done_when": "tests pass",
            "loop": "until_done",
            "typpo": true,
        });
        assert_eq!(unknown_keys(&cfg), vec!["typpo".to_string()]);
        assert_eq!(inert_keys(&cfg), vec!["loop".to_string()]);
    }

    /// Every key the shipped workflow recipes write must be accounted for.
    ///
    /// This is the CI-time half: a recipe that invents `comit_policy` should
    /// fail here rather than run a mission whose gate silently defaults.
    #[test]
    fn shipped_recipes_only_write_accounted_keys() {
        let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../templates/workflows");
        let Ok(entries) = std::fs::read_dir(dir) else {
            return; // templates not present in this build context
        };
        // Keys that belong to the recipe/phase envelope rather than to the
        // phase config blob itself.
        const ENVELOPE: &[&str] = &[
            "key",
            "name",
            "title",
            "blurb",
            "kind",
            "order_idx",
            "requires_repo",
            "default_team_template",
            "default_phase_teams",
            "default_topology",
            "phases",
            "description",
        ];
        // `[default_phase_teams]` maps a phase PURPOSE to a team template key,
        // so its keys are not config keys and must not be checked as such.
        // They are checked against the purposes `phase_runner::purposes_for`
        // can actually emit instead — a typo'd purpose matches no phase and
        // that phase silently falls back to the mission-wide team, which is
        // exactly the kind of quiet wrong staffing this table exists to end.
        const PURPOSES: &[&str] = &["research", "coding", "security", "mission"];
        for entry in entries.flatten() {
            let path = entry.path();
            if path.extension().and_then(|e| e.to_str()) != Some("toml") {
                continue;
            }
            let body = std::fs::read_to_string(&path).unwrap();
            let mut table = String::new();
            for line in body.lines() {
                let line = line.trim();
                if line.starts_with('[') {
                    table = line.trim_matches(['[', ']'].as_slice()).to_string();
                    continue;
                }
                if line.starts_with('#') || !line.contains('=') {
                    continue;
                }
                let key = line.split('=').next().unwrap().trim();
                if key.is_empty() || key.contains(' ') || key.contains('[') {
                    continue;
                }
                if table == "default_phase_teams" {
                    assert!(
                        PURPOSES.contains(&key),
                        "{} staffs purpose `{key}`, which `purposes_for` never emits — \
                         that phase would fall back to the mission-wide team with \
                         nothing reporting it",
                        path.display()
                    );
                    continue;
                }
                let accounted = ENVELOPE.contains(&key)
                    || is_listed(key, KNOWN_KEYS)
                    || is_listed(key, DECLARED_BUT_UNREAD);
                assert!(
                    accounted,
                    "{} writes `{key}`, which no reader claims and no gap declares",
                    path.display()
                );
            }
        }
    }
}