node-app-build 7.0.22

//! `node-app audit` — run the embedded blueprint's auditable patterns
//! against a project on disk. Soft mode (default) only warns; --strict
//! flips warnings to hard errors.
//!
//! Phase 1 of econ-v1/node#868 introduced the audit harness with three of
//! four patterns implemented (the other two stubbed with OK notes).
//! Phase 5a of #868 (this PR) tightens packaging:
//!   - `NoWholesaleNodeModulesWhenBundled` becomes an ERROR when the app
//!     pins `manifest.nodeApp.blueprint: ">=2"`. Apps pinned at v1 keep
//!     the warning for one release cycle.
//!   - `PrivateNativeDepsInPrivateModulesDir` (new) cross-checks that
//!     packages declared in `nodeApp.privateRuntime` are actually resolvable
//!     for staging under `private_modules/`.

use crate::blueprint::{Pattern, APP_SDK_MEMORY_REPORTING_FLOOR, CURRENT};
use anyhow::Result;
use serde::Serialize;
use std::path::Path;

#[derive(Debug, Serialize)]
struct Finding {
    pattern: String,
    severity: Severity,
    message: String,
    fix: Option<String>,
}

#[derive(Debug, Serialize, PartialEq)]
enum Severity {
    Ok,
    Warning,
    Error,
}

pub fn run(path: &Path, strict: bool, json: bool) -> Result<()> {
    let mut findings = Vec::new();

    let manifest_path = path.join("manifest.json");
    let manifest = if manifest_path.exists() {
        let raw = std::fs::read_to_string(&manifest_path)?;
        Some(serde_json::from_str::<serde_json::Value>(&raw)?)
    } else {
        findings.push(Finding {
            pattern: "manifest_present".into(),
            severity: Severity::Error,
            message: "manifest.json missing".into(),
            fix: Some("run `node-app new` to scaffold, or hand-write a manifest.json".into()),
        });
        None
    };

    // app_type — surface shared-runtime fitness up-front
    if let Some(m) = &manifest {
        match m.get("app_type").and_then(|v| v.as_str()) {
            Some("bun") => findings.push(Finding {
                pattern: "app_type_check".into(),
                severity: Severity::Ok,
                message: "app_type=bun (eligible for shared runtime — set shared_runtime_enabled per-app at install time)".into(),
                fix: None,
            }),
            Some("native") => findings.push(Finding {
                pattern: "app_type_check".into(),
                severity: Severity::Ok,
                message: "app_type=native (cdylib, loaded in-process; shared runtime does not apply)".into(),
                fix: None,
            }),
            Some("standalone") => findings.push(Finding {
                pattern: "app_type_check".into(),
                severity: Severity::Warning,
                message: "app_type=standalone — own systemd unit; shared runtime memory savings do NOT apply".into(),
                fix: Some(
                    "if you don't strictly need own-systemd-unit semantics, consider `app_type: bun` instead".into(),
                ),
            }),
            Some("managed-v1") => findings.push(Finding {
                pattern: "app_type_check".into(),
                severity: Severity::Ok,
                message: "app_type=managed-v1 (LLMC-generated executable; host verification required)".into(),
                fix: None,
            }),
            other => findings.push(Finding {
                pattern: "app_type_check".into(),
                severity: Severity::Error,
                message: format!("manifest.app_type unrecognized: {:?}", other),
                fix: Some("use one of: bun, native, standalone, managed-v1".into()),
            }),
        }
    }

    // Resolve the app's declared blueprint pin (manifest.nodeApp.blueprint).
    // Defaults to ">=1" — apps that haven't opted into v2 yet keep the
    // v1 warning behavior for one release cycle.
    let pinned_min = pinned_min_blueprint(manifest.as_ref());

    // Implement each Pattern from CURRENT.patterns:
    for pattern in CURRENT.patterns {
        let f = match pattern {
            Pattern::NoWholesaleNodeModulesWhenBundled => {
                check_no_wholesale_node_modules(path, pinned_min)
            }
            Pattern::SharedExternalsMatchPin => {
                check_shared_externals_match_pin(path, manifest.as_ref())
            }
            Pattern::PrivateNativeDepsDeclared => {
                check_private_native_deps_declared(path, manifest.as_ref())
            }
            Pattern::NoBunBuildCompileForBunApps => {
                check_no_bun_compile_for_bun_apps(path, manifest.as_ref())
            }
            Pattern::PrivateNativeDepsInPrivateModulesDir => {
                check_private_native_deps_in_private_modules(path, manifest.as_ref())
            }
            Pattern::BunAppSdkReportsMemory => {
                check_bun_app_sdk_reports_memory(path, manifest.as_ref())
            }
            Pattern::LazyAppCronCadenceFloor => {
                check_lazy_app_cron_cadence_floor(path, manifest.as_ref())
            }
            Pattern::StageDeclaresDataContract => {
                check_stage_declares_data_contract(path, manifest.as_ref())
            }
            Pattern::StageStreamListsAreDistinct => {
                check_stage_stream_lists_are_distinct(path, manifest.as_ref())
            }
        };
        if let Some(f) = f {
            findings.push(f);
        }
    }

    if json {
        println!("{}", serde_json::to_string_pretty(&findings)?);
    } else {
        for f in &findings {
            let icon = match f.severity {
                Severity::Ok => "✓",
                Severity::Warning => "⚠",
                Severity::Error => "✗",
            };
            println!("{} {} — {}", icon, f.pattern, f.message);
            if let Some(fix) = &f.fix {
                println!("    fix: {}", fix);
            }
        }
    }

    let has_errors = findings.iter().any(|f| f.severity == Severity::Error);
    let has_warnings = findings.iter().any(|f| f.severity == Severity::Warning);
    if has_errors || (strict && has_warnings) {
        std::process::exit(1);
    }
    Ok(())
}

/// Severity for `NoWholesaleNodeModulesWhenBundled` depends on the
/// blueprint version the app pins (manifest.nodeApp.blueprint):
///   - `>=1` (or unset): warning. Backward-compat for one release cycle.
///   - `>=2` (or higher): hard error. Apps that have opted into v2 MUST
///     keep the wholesale node_modules/ out of the .deb (use
///     `nodeApp.privateRuntime` to declare native deps instead).
fn check_no_wholesale_node_modules(path: &Path, pinned_min: u32) -> Option<Finding> {
    let dist = path.join("dist/index.js");
    let nm = path.join("node_modules");
    if !(dist.exists() && nm.exists()) {
        return None;
    }
    let severity = if pinned_min >= 2 {
        Severity::Error
    } else {
        Severity::Warning
    };
    let message = if pinned_min >= 2 {
        "dist/index.js present alongside `node_modules/`; blueprint v2 forbids staging wholesale node_modules/ in the .deb".to_string()
    } else {
        "dist/index.js present but `node_modules/` would be staged in the .deb (allowed under blueprint v1; tightens to error at v2)".to_string()
    };
    Some(Finding {
        pattern: "NoWholesaleNodeModulesWhenBundled".into(),
        severity,
        message,
        fix: Some(
            "list native runtime deps under manifest.nodeApp.privateRuntime and pin manifest.nodeApp.blueprint: \">=2\" — `node-app package` then stages private_modules/<pkg> only".into(),
        ),
    })
}

fn check_shared_externals_match_pin(
    path: &Path,
    _manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    // Stub for phase 1 — defer the actual version-match logic to phase 6.
    // For now just print informational reminder that the shared-deps lint
    // (infra/scripts/lint-shared-deps.mjs) is the source of truth.
    let pkg = path.join("package.json");
    if !pkg.exists() {
        return None;
    }
    Some(Finding {
        pattern: "SharedExternalsMatchPin".into(),
        severity: Severity::Ok,
        message: "shared-deps version-pin checked via infra/scripts/lint-shared-deps.mjs (deferred to phase 6)".into(),
        fix: None,
    })
}

fn check_private_native_deps_declared(
    path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    // Stub for phase 1 — checking which deps have native bindings is non-trivial
    // (need to walk node_modules and look for .node files / binding.gyp). Defer
    // the real check to phase 4. For now, an OK note.
    let _ = (path, manifest);
    Some(Finding {
        pattern: "PrivateNativeDepsDeclared".into(),
        severity: Severity::Ok,
        message: "private-native-deps audit deferred to phase 4 — declare them manually in manifest.json#nodeApp.privateRuntime for now".into(),
        fix: None,
    })
}

fn check_no_bun_compile_for_bun_apps(
    path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    let app_type = manifest?.get("app_type")?.as_str()?;
    if app_type != "bun" {
        return None;
    }
    let pkg_path = path.join("package.json");
    if !pkg_path.exists() {
        return None;
    }
    let raw = std::fs::read_to_string(&pkg_path).ok()?;
    let pkg: serde_json::Value = serde_json::from_str(&raw).ok()?;
    let scripts = pkg.get("scripts")?.as_object()?;
    for (name, cmd) in scripts {
        if let Some(s) = cmd.as_str() {
            if s.contains("bun build") && s.contains("--compile") {
                return Some(Finding {
                    pattern: "NoBunBuildCompileForBunApps".into(),
                    severity: Severity::Warning,
                    message: format!(
                        "package.json script `{}` uses `bun build --compile` — incompatible with shared runtime",
                        name
                    ),
                    fix: Some(
                        "drop --compile from the build script; ship dist/index.js so the supervisor can spawn it as a Worker".into(),
                    ),
                });
            }
        }
    }
    None
}

/// Cross-check that every package listed in `manifest.nodeApp.privateRuntime`
/// is resolvable at staging time — either present under `node_modules/<pkg>`
/// (so `node-app package` can copy it into `private_modules/<pkg>`) or
/// already pre-staged under `private_modules/<pkg>` in the source tree.
/// No-op when `nodeApp.privateRuntime` is empty/absent.
fn check_private_native_deps_in_private_modules(
    path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    let private_runtime = manifest
        .and_then(|m| m.get("nodeApp"))
        .and_then(|n| n.get("privateRuntime"))
        .and_then(|v| v.as_array());
    let pkgs: Vec<&str> = {
        let arr = private_runtime?;
        arr.iter().filter_map(|v| v.as_str()).collect()
    };
    if pkgs.is_empty() {
        return None;
    }

    let dist = path.join("dist/index.js");
    if !dist.exists() {
        // No staging artifact yet to cross-check against; this lint only
        // fires once `node-app build` has produced dist/.
        return Some(Finding {
            pattern: "PrivateNativeDepsInPrivateModulesDir".into(),
            severity: Severity::Ok,
            message: format!(
                "{} private runtime pkg(s) declared; run `node-app build` then re-audit to cross-check staging",
                pkgs.len()
            ),
            fix: None,
        });
    }

    let mut missing: Vec<&str> = Vec::new();
    for pkg in &pkgs {
        let in_node_modules = path.join("node_modules").join(pkg).exists();
        let in_private_modules = path.join("private_modules").join(pkg).exists();
        if !in_node_modules && !in_private_modules {
            missing.push(pkg);
        }
    }
    if missing.is_empty() {
        Some(Finding {
            pattern: "PrivateNativeDepsInPrivateModulesDir".into(),
            severity: Severity::Ok,
            message: format!(
                "all {} declared private runtime pkg(s) resolvable for staging: {}",
                pkgs.len(),
                pkgs.join(", ")
            ),
            fix: None,
        })
    } else {
        Some(Finding {
            pattern: "PrivateNativeDepsInPrivateModulesDir".into(),
            severity: Severity::Error,
            message: format!(
                "manifest.nodeApp.privateRuntime declares pkg(s) not on disk: {}",
                missing.join(", ")
            ),
            fix: Some(
                "run `bun install` so node_modules/<pkg>/ exists; or remove the unused entries from manifest.nodeApp.privateRuntime".into(),
            ),
        })
    }
}

/// Lowest version a npm range can resolve to, for the simple range shapes
/// that actually appear in these manifests: an exact pin (`6.9.4`), a caret
/// or tilde (`^6.9.4`, `~6.9.4`), or a comparator (`>=6.9.0`, `=6.9.4`).
///
/// Deliberately conservative: anything it cannot parse returns `None` and the
/// caller reports "could not determine" rather than guessing a verdict. A
/// wrong PASS here would be worse than no check at all.
fn min_version_of_range(range: &str) -> Option<(u64, u64, u64)> {
    let trimmed = range
        .trim()
        .trim_start_matches(['^', '~', '=', '>', '<', 'v'])
        .trim();
    // Drop any pre-release/build suffix (`6.9.0-rc.1` -> `6.9.0`) and split.
    let core = trimmed.split(['-', '+', ' ', ',']).next()?.trim();
    let mut parts = core.split('.');
    let major = parts.next()?.parse::<u64>().ok()?;
    // A partial range (`6`, `6.9`) can resolve no lower than `.0`.
    let minor = parts.next().map_or(Some(0), |p| p.parse::<u64>().ok())?;
    let patch = parts.next().map_or(Some(0), |p| p.parse::<u64>().ok())?;
    Some((major, minor, patch))
}

/// `app_type: "bun"` apps must pin an `@econ-v1/app-sdk` at or above
/// [`APP_SDK_MEMORY_REPORTING_FLOOR`], or the host can never attribute their
/// memory. See the `Pattern::BunAppSdkReportsMemory` doc comment for why this
/// reports at `Severity::Ok` today instead of warning.
fn check_bun_app_sdk_reports_memory(
    path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    if manifest?.get("app_type")?.as_str()? != "bun" {
        return None;
    }
    let pkg_path = path.join("package.json");
    if !pkg_path.exists() {
        return None;
    }
    let raw = std::fs::read_to_string(&pkg_path).ok()?;
    let pkg: serde_json::Value = serde_json::from_str(&raw).ok()?;
    let declared = pkg
        .get("dependencies")
        .and_then(|d| d.get("@econ-v1/app-sdk"))
        .and_then(|v| v.as_str());

    let (floor_major, floor_minor, floor_patch) = APP_SDK_MEMORY_REPORTING_FLOOR;
    let floor = format!("{floor_major}.{floor_minor}.{floor_patch}");

    let Some(range) = declared else {
        return Some(Finding {
            pattern: "BunAppSdkReportsMemory".into(),
            severity: Severity::Ok,
            message:
                "app_type=bun but package.json declares no @econ-v1/app-sdk dependency — this app \
                 cannot self-report its JS heap, so the host will show it as unattributed"
                    .into(),
            fix: Some(format!(
                "add \"@econ-v1/app-sdk\": \"^{floor}\" to dependencies if this app runs on the SDK lifecycle"
            )),
        });
    };

    let Some(min) = min_version_of_range(range) else {
        return Some(Finding {
            pattern: "BunAppSdkReportsMemory".into(),
            severity: Severity::Ok,
            message: format!(
                "could not determine the lowest @econ-v1/app-sdk version \"{range}\" resolves to; \
                 memory self-reporting needs >= {floor}"
            ),
            fix: None,
        });
    };

    if min >= APP_SDK_MEMORY_REPORTING_FLOOR {
        Some(Finding {
            pattern: "BunAppSdkReportsMemory".into(),
            severity: Severity::Ok,
            message: format!(
                "@econ-v1/app-sdk \"{range}\" is at or above the {floor} memory-reporting floor — \
                 this app self-reports its heap (attribution: exact_isolate)"
            ),
            fix: None,
        })
    } else {
        Some(Finding {
            pattern: "BunAppSdkReportsMemory".into(),
            severity: Severity::Ok,
            message: format!(
                "@econ-v1/app-sdk \"{range}\" is BELOW the {floor} memory-reporting floor — the SDK \
                 never sends app_memory, so this app shows as unattributed in the per-app memory UI"
            ),
            fix: Some(format!(
                "bump the @econ-v1/app-sdk dependency to ^{floor} or newer (the 5.x -> 6.9.x jump is \
                 additive: no exports removed, engines unchanged)"
            )),
        })
    }
}

/// Parse `manifest.nodeApp.blueprint` (e.g. `">=2"`) and return the minimum
/// blueprint version it pins. Unrecognized / missing pin defaults to 1.
fn pinned_min_blueprint(manifest: Option<&serde_json::Value>) -> u32 {
    let s = manifest
        .and_then(|m| m.get("nodeApp"))
        .and_then(|n| n.get("blueprint"))
        .and_then(|v| v.as_str())
        .unwrap_or(">=1");
    // Only `>=N` semantics are honored today; anything else falls back to v1.
    if let Some(rest) = s.strip_prefix(">=") {
        rest.trim().parse::<u32>().unwrap_or(1)
    } else {
        s.trim().parse::<u32>().unwrap_or(1)
    }
}

/// See the `Pattern::StageDeclaresDataContract` doc comment for why this warns
/// rather than errors.
fn check_stage_declares_data_contract(
    _path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    let ui = manifest?.get("ui")?;
    if ui.get("kind")?.as_str()? != "stage" {
        return None;
    }
    if ui.get("data").is_some() {
        return None;
    }
    Some(Finding {
        pattern: "StageDeclaresDataContract".into(),
        severity: Severity::Warning,
        message: "ui.data is not declared, so this stage reports offline-ready with no data cached"
            .into(),
        fix: Some(
            "add ui.data — use offline: \"online-only\" if the stage genuinely needs a connection"
                .into(),
        ),
    })
}

/// A name whose last dot-separated segment is `vN` is the `ui.data.streams` shape.
/// `transport.state-changed` and `app.agent_session` carry no version suffix and
/// are therefore unambiguous.
fn looks_like_app_data_stream_name(name: &str) -> bool {
    if name.split('.').count() < 2 {
        return false;
    }
    let Some(last) = name.split('.').next_back() else {
        return false;
    };
    last.len() > 1 && last.starts_with('v') && last[1..].chars().all(|c| c.is_ascii_digit())
}

/// See the `Pattern::StageStreamListsAreDistinct` doc comment. Both messages name
/// the OTHER list and say what belongs in each, so the error teaches the
/// distinction the near-identical names fail to.
fn check_stage_stream_lists_are_distinct(
    _path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    let ui = manifest?.get("ui")?;
    let requires: Vec<&str> = ui
        .get("requires")
        .and_then(|r| r.get("streams"))
        .and_then(|s| s.as_array())
        .map(|entries| entries.iter().filter_map(|e| e.as_str()).collect())
        .unwrap_or_default();
    let data: Vec<&str> = ui
        .get("data")
        .and_then(|d| d.get("streams"))
        .and_then(|s| s.as_array())
        .map(|entries| {
            entries
                .iter()
                .filter_map(|e| e.get("name").and_then(|n| n.as_str()))
                .collect()
        })
        .unwrap_or_default();

    if let Some(shared) = requires.iter().find(|name| data.contains(name)) {
        return Some(Finding {
            pattern: "StageStreamListsAreDistinct".into(),
            severity: Severity::Error,
            message: format!(
                "'{shared}' is in both ui.requires.streams and ui.data.streams; they are different lists"
            ),
            fix: Some(
                "ui.requires.streams names shell/transport events; ui.data.streams names app-data sync streams — remove it from one".into(),
            ),
        });
    }
    let stray = requires
        .iter()
        .find(|name| looks_like_app_data_stream_name(name))?;
    Some(Finding {
        pattern: "StageStreamListsAreDistinct".into(),
        severity: Severity::Error,
        message: format!(
            "'{stray}' in ui.requires.streams has the ui.data.streams name shape and subscribes to nothing"
        ),
        fix: Some(
            "move it to ui.data.streams, or use an unversioned shell event name such as transport.state-changed".into(),
        ),
    })
}

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

    fn write_app(dir: &std::path::Path, app_type: &str, sdk: Option<&str>) {
        std::fs::write(
            dir.join("manifest.json"),
            format!(r#"{{"name":"t","version":"1.0.0","app_type":"{app_type}"}}"#),
        )
        .unwrap();
        let deps = match sdk {
            Some(v) => format!(r#"{{"@econ-v1/app-sdk":"{v}"}}"#),
            None => "{}".to_string(),
        };
        std::fs::write(
            dir.join("package.json"),
            format!(r#"{{"name":"t","dependencies":{deps}}}"#),
        )
        .unwrap();
    }

    fn finding_for(app_type: &str, sdk: Option<&str>) -> Option<Finding> {
        let tmp = tempfile::tempdir().unwrap();
        write_app(tmp.path(), app_type, sdk);
        let raw = std::fs::read_to_string(tmp.path().join("manifest.json")).unwrap();
        let manifest: serde_json::Value = serde_json::from_str(&raw).unwrap();
        check_bun_app_sdk_reports_memory(tmp.path(), Some(&manifest))
    }

    #[test]
    fn min_version_handles_the_range_shapes_these_manifests_actually_use() {
        assert_eq!(min_version_of_range("6.9.4"), Some((6, 9, 4)));
        assert_eq!(min_version_of_range("^6.9.4"), Some((6, 9, 4)));
        assert_eq!(min_version_of_range("~6.9.0"), Some((6, 9, 0)));
        assert_eq!(min_version_of_range(">=6.9.0"), Some((6, 9, 0)));
        assert_eq!(min_version_of_range("=5.28.4"), Some((5, 28, 4)));
        // Partial ranges can resolve no lower than `.0`.
        assert_eq!(min_version_of_range("^6"), Some((6, 0, 0)));
        assert_eq!(min_version_of_range("^6.9"), Some((6, 9, 0)));
        // Pre-release suffix is dropped down to its core version.
        assert_eq!(min_version_of_range("6.9.0-rc.1"), Some((6, 9, 0)));
        // Unparseable shapes must NOT be guessed at.
        assert_eq!(min_version_of_range("latest"), None);
        assert_eq!(min_version_of_range("workspace:*"), None);
    }

    #[test]
    fn caret_five_x_is_below_the_floor_even_though_it_floats() {
        // `^5.28.4` floats only within 5.x, so it can never reach 6.9.0 —
        // the exact trap that left every extracted app unattributed.
        let f = finding_for("bun", Some("^5.28.4")).expect("bun app yields a finding");
        assert!(f.message.contains("BELOW"), "got: {}", f.message);
        assert!(
            f.fix.is_some(),
            "a below-floor finding must say how to fix it"
        );
    }

    #[test]
    fn at_or_above_the_floor_passes() {
        for range in ["^6.9.0", "6.9.4", "^7.0.0"] {
            let f = finding_for("bun", Some(range)).expect("bun app yields a finding");
            assert!(
                f.message.contains("at or above"),
                "{range} should pass, got: {}",
                f.message
            );
        }
    }

    #[test]
    fn reports_ok_severity_so_strict_releases_do_not_break() {
        // `node-app audit --strict` exits non-zero on warnings too, so this
        // rule must stay informational until the apps are bumped.
        let f = finding_for("bun", Some("^5.28.4")).unwrap();
        assert_eq!(f.severity, Severity::Ok);
    }

    #[test]
    fn non_bun_apps_are_not_audited_for_this() {
        assert!(finding_for("native", Some("^5.28.4")).is_none());
        assert!(finding_for("standalone", None).is_none());
    }

    #[test]
    fn missing_sdk_dependency_is_called_out_rather_than_silently_passing() {
        let f = finding_for("bun", None).expect("a bun app with no SDK dep still yields a finding");
        assert!(
            f.message.contains("no @econ-v1/app-sdk"),
            "got: {}",
            f.message
        );
    }
}

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

    #[test]
    fn stage_without_ui_data_warns() {
        let manifest = serde_json::json!({
            "app_type": "bun",
            "ui": { "kind": "stage", "entry": "ui/dist/main.js", "ui_api": 1 }
        });
        let finding =
            check_stage_declares_data_contract(std::path::Path::new("."), Some(&manifest))
                .expect("a stage with no ui.data must be reported");
        assert_eq!(finding.severity, Severity::Warning);
    }

    #[test]
    fn stage_with_explicit_online_only_is_silent() {
        let manifest = serde_json::json!({
            "app_type": "bun",
            "ui": {
                "kind": "stage", "entry": "ui/dist/main.js", "ui_api": 1,
                "data": { "namespace": "notes", "offline": "online-only", "sync": "snapshot",
                          "queries": [{"name": "notes.snapshot.v1", "capability": "notes.snapshot", "kind": "snapshot"}],
                          "streams": [] }
            }
        });
        assert!(
            check_stage_declares_data_contract(std::path::Path::new("."), Some(&manifest))
                .is_none()
        );
    }

    #[test]
    fn a_widget_is_not_a_stage_and_is_never_reported() {
        let manifest = serde_json::json!({
            "app_type": "bun",
            "ui": { "kind": "widget", "entry": "ui/dist/main.js", "ui_api": 1 }
        });
        assert!(
            check_stage_declares_data_contract(std::path::Path::new("."), Some(&manifest))
                .is_none()
        );
    }
}

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

    #[test]
    fn a_name_in_both_stream_lists_is_an_error() {
        let manifest = serde_json::json!({
            "ui": { "kind": "stage",
                "requires": { "streams": ["notes.changes.v1"] },
                "data": { "namespace": "notes", "offline": "last-known", "sync": "cursor",
                          "queries": [{"name": "notes.snapshot.v1", "capability": "notes.snapshot", "kind": "snapshot"}],
                          "streams": [{"name": "notes.changes.v1", "kind": "changes"}] } }
        });
        let finding =
            check_stage_stream_lists_are_distinct(std::path::Path::new("."), Some(&manifest))
                .expect("a name in both lists must be reported");
        assert_eq!(finding.severity, Severity::Error);
    }

    // The mistake an author actually makes: putting an app-data stream name into the
    // shell/transport subscription list, where it silently subscribes to nothing.
    #[test]
    fn a_namespaced_versioned_name_in_ui_requires_streams_is_an_error() {
        let manifest = serde_json::json!({
            "ui": { "kind": "stage", "requires": { "streams": ["notes.changes.v1"] } }
        });
        assert_eq!(
            check_stage_stream_lists_are_distinct(std::path::Path::new("."), Some(&manifest))
                .expect("a data-shaped name in ui.requires.streams must be reported")
                .severity,
            Severity::Error
        );
    }

    #[test]
    fn real_shell_event_subscriptions_are_silent() {
        let manifest = serde_json::json!({
            "ui": { "kind": "stage",
                "requires": { "streams": ["transport.state-changed", "app.agent_session"] } }
        });
        assert!(
            check_stage_stream_lists_are_distinct(std::path::Path::new("."), Some(&manifest))
                .is_none()
        );
    }
}

// --- LazyAppCronCadenceFloor ------------------------------------------------

/// `GovernorPolicy::minimal_default_idle` — the idle floor applied to a lazy
/// app that declares no `governor.min_idle_secs`. Mirrors the default in
/// `core/app-host/src/governor.rs`; keep the two in step.
const GOVERNOR_DEFAULT_MIN_IDLE_SECS: u64 = 600;

/// The governor sweep's own cadence (`app-governor-sweep`, `0 */2 * * * *`).
/// An app has to still be past `min_idle` when a sweep LANDS, so the effective
/// pinning threshold is `min_idle + this`, not `min_idle`.
const GOVERNOR_SWEEP_CADENCE_SECS: u64 = 120;

/// Apps the governor never idle-stops whatever their manifest says
/// (`governor_eligibility::RUNTIME_CRITICAL_APPS`).
const RUNTIME_CRITICAL_APPS: &[&str] = &["ldk-node", "cron", "message-queue", "observability"];

/// Smallest interval between consecutive fires of a 6- or 7-field cron
/// expression, for the shapes that actually occur in this platform's job
/// catalogue. Returns `None` when the shape is not recognised or is coarser
/// than hourly — callers treat `None` as "no finding", so an unparsed
/// expression can never produce a false positive.
fn cron_min_period_secs(expr: &str) -> Option<u64> {
    let f: Vec<&str> = expr.split_whitespace().collect();
    if f.len() != 6 && f.len() != 7 {
        return None;
    }
    let (sec, min, hour) = (f[0], f[1], f[2]);

    // Smallest gap within one field's own value list (e.g. `0,30` -> 30).
    fn list_min_gap(spec: &str) -> Option<u64> {
        let mut vals: Vec<u64> = spec
            .split(',')
            .map(|p| p.trim().parse::<u64>().ok())
            .collect::<Option<Vec<_>>>()?;
        if vals.len() < 2 {
            return None;
        }
        vals.sort_unstable();
        vals.windows(2).map(|w| w[1] - w[0]).min()
    }

    fn step_of(spec: &str) -> Option<u64> {
        let rest = spec.strip_prefix("*/")?;
        rest.parse::<u64>().ok()
    }

    // Seconds field drives the period whenever it is not a single fixed value.
    if sec == "*" {
        return Some(1);
    }
    if let Some(n) = step_of(sec) {
        return Some(n);
    }
    if let Some(g) = list_min_gap(sec) {
        return Some(g);
    }

    // Fixed second -> the minute field drives it.
    if min == "*" {
        return Some(60);
    }
    if let Some(n) = step_of(min) {
        return Some(n * 60);
    }
    if let Some(g) = list_min_gap(min) {
        return Some(g * 60);
    }

    // Fixed second + minute -> the hour field drives it.
    if hour == "*" {
        return Some(3600);
    }
    if let Some(n) = step_of(hour) {
        return Some(n * 3600);
    }
    // Daily or coarser, or a shape we do not model: no finding.
    None
}

/// True when the governor may idle-stop this app, so a fast cron pins it.
///
/// Deliberately errs toward "eligible": an undeclared or unrecognised
/// `auto_start` resolves to `Lazy` in the host (see the standalone guard in
/// `NodeAppService`'s sweep-candidate builder), and several shipped manifests
/// carry `auto_start: true` — a bool, not one of the `auto`/`manual`/`lazy`
/// mode strings — which lands in exactly that fallback.
fn governor_may_stop(name: &str, manifest: &serde_json::Value) -> bool {
    if RUNTIME_CRITICAL_APPS.contains(&name) {
        return false;
    }
    if manifest.get("critical").and_then(|v| v.as_bool()) == Some(true) {
        return false;
    }
    // Standalone apps are systemd-owned; the sweep skips them outright.
    if manifest.get("app_type").and_then(|v| v.as_str()) == Some("standalone") {
        return false;
    }
    if manifest
        .get("governor")
        .and_then(|g| g.get("terminable"))
        .and_then(|v| v.as_bool())
        == Some(false)
    {
        return false;
    }
    !matches!(
        manifest.get("auto_start").and_then(|v| v.as_str()),
        Some("auto") | Some("manual")
    )
}

/// Every quoted string literal in `src` that parses as a cron expression.
fn cron_literals(src: &str) -> Vec<String> {
    let mut out = Vec::new();
    for quote in ['"', '\'', '`'] {
        for part in src.split(quote).skip(1).step_by(2) {
            if part.len() <= 64 && cron_min_period_secs(part).is_some() {
                out.push(part.to_string());
            }
        }
    }
    out
}

fn collect_sources(dir: &Path, out: &mut Vec<std::path::PathBuf>) {
    let Ok(entries) = std::fs::read_dir(dir) else {
        return;
    };
    for e in entries.flatten() {
        let p = e.path();
        let name = p.file_name().and_then(|n| n.to_str()).unwrap_or("");
        if p.is_dir() {
            if !matches!(name, "node_modules" | "dist" | ".git" | "target") {
                collect_sources(&p, out);
            }
        } else if matches!(
            p.extension().and_then(|x| x.to_str()),
            Some("ts") | Some("tsx") | Some("js")
        ) && !name.contains(".test.")
        {
            out.push(p);
        }
    }
}

/// Flag recurring cron jobs a governor-eligible app registers for itself at a
/// cadence that can only ever pin its own isolate. See the
/// `Pattern::LazyAppCronCadenceFloor` doc comment.
fn check_lazy_app_cron_cadence_floor(
    path: &Path,
    manifest: Option<&serde_json::Value>,
) -> Option<Finding> {
    let manifest = manifest?;
    let name = manifest.get("name").and_then(|v| v.as_str()).unwrap_or("");
    if !governor_may_stop(name, manifest) {
        return None;
    }

    let min_idle = manifest
        .get("governor")
        .and_then(|g| g.get("min_idle_secs"))
        .and_then(|v| v.as_u64())
        .unwrap_or(GOVERNOR_DEFAULT_MIN_IDLE_SECS);
    let floor = min_idle + GOVERNOR_SWEEP_CADENCE_SECS;

    let mut sources = Vec::new();
    collect_sources(&path.join("src"), &mut sources);

    let mut worst: Option<(u64, String, String)> = None;
    for file in sources {
        let Ok(src) = std::fs::read_to_string(&file) else {
            continue;
        };
        // Only files that actually register a cron job — a bare cron-shaped
        // literal elsewhere (a test fixture, a docs string) is not a finding.
        if !src.contains("core.cron.register") {
            continue;
        }
        for lit in cron_literals(&src) {
            let Some(period) = cron_min_period_secs(&lit) else {
                continue;
            };
            if period <= floor && worst.as_ref().is_none_or(|(w, _, _)| period < *w) {
                let rel = file
                    .strip_prefix(path)
                    .unwrap_or(&file)
                    .display()
                    .to_string();
                worst = Some((period, lit, rel));
            }
        }
    }

    let (period, expr, file) = worst?;
    Some(Finding {
        pattern: "LazyAppCronCadenceFloor".into(),
        severity: Severity::Warning,
        message: format!(
            "{file} registers a cron job every {period}s (\"{expr}\"), but the governor cannot \
             idle-stop this app until it has been quiet for {min_idle}s and a sweep lands in that \
             window (>{floor}s). This job re-wakes the app before that can ever happen, so its \
             isolate stays resident for the life of the process and its memory is never reclaimed."
        ),
        fix: Some(format!(
            "slow the job to comfortably more than {floor}s, or — if it genuinely must run that \
             often — declare the intent in manifest.json: a lower \"governor\": {{\"min_idle_secs\": \
             ...}} so the floor matches reality, or \"governor\": {{\"terminable\": false}} to opt \
             out of idle termination altogether"
        )),
    })
}

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

    #[test]
    fn parses_the_shapes_this_platform_actually_uses() {
        assert_eq!(cron_min_period_secs("*/15 * * * * *"), Some(15));
        assert_eq!(cron_min_period_secs("*/30 * * * * *"), Some(30));
        assert_eq!(cron_min_period_secs("0 * * * * *"), Some(60));
        assert_eq!(cron_min_period_secs("0 */1 * * * *"), Some(60));
        assert_eq!(cron_min_period_secs("0 */5 * * * *"), Some(300));
        assert_eq!(cron_min_period_secs("0 */10 * * * *"), Some(600));
        assert_eq!(cron_min_period_secs("0 */15 * * * *"), Some(900));
        assert_eq!(cron_min_period_secs("0 0 * * * *"), Some(3600));
        assert_eq!(cron_min_period_secs("0 15 * * * *"), Some(3600));
        assert_eq!(cron_min_period_secs("0 0 */2 * * *"), Some(7200));
    }

    /// Unmodelled or coarser-than-hourly shapes must yield no finding rather
    /// than a guess — a false positive here costs an app owner real time.
    #[test]
    fn coarse_and_unknown_shapes_are_not_findings() {
        assert_eq!(cron_min_period_secs("0 0 3 * * *"), None); // daily
        assert_eq!(cron_min_period_secs("0 0 3 * * 1"), None); // weekly
        assert_eq!(cron_min_period_secs("44 13 11 26 8 * 2026"), None); // one-shot
        assert_eq!(cron_min_period_secs("not a cron"), None);
        assert_eq!(cron_min_period_secs("* * * *"), None); // wrong arity
    }

    #[test]
    fn comma_lists_use_the_smallest_gap() {
        assert_eq!(cron_min_period_secs("0,30 * * * * *"), Some(30));
        assert_eq!(cron_min_period_secs("0 0,10,40 * * * *"), Some(600));
    }

    fn manifest(json: &str) -> serde_json::Value {
        serde_json::from_str(json).unwrap()
    }

    #[test]
    fn runtime_critical_and_opted_out_apps_are_never_flagged() {
        // In RUNTIME_CRITICAL_APPS: ldk-node's 15s cycle watch is fine.
        assert!(!governor_may_stop(
            "ldk-node",
            &manifest(r#"{"name":"ldk-node","auto_start":true}"#)
        ));
        assert!(!governor_may_stop(
            "x",
            &manifest(r#"{"name":"x","critical":true}"#)
        ));
        assert!(!governor_may_stop(
            "x",
            &manifest(r#"{"name":"x","app_type":"standalone"}"#)
        ));
        assert!(!governor_may_stop(
            "x",
            &manifest(r#"{"name":"x","governor":{"terminable":false}}"#)
        ));
        assert!(!governor_may_stop(
            "x",
            &manifest(r#"{"name":"x","auto_start":"auto"}"#)
        ));
    }

    /// `auto_start: true` is a bool, not one of the mode strings, so the host
    /// falls back to `Lazy` — several shipped manifests (economic among them)
    /// look exactly like this and ARE governor-managed. Flag them.
    #[test]
    fn bool_and_absent_auto_start_count_as_lazy() {
        assert!(governor_may_stop(
            "economic",
            &manifest(r#"{"name":"economic","auto_start":true}"#)
        ));
        assert!(governor_may_stop("x", &manifest(r#"{"name":"x"}"#)));
        assert!(governor_may_stop(
            "onboarding",
            &manifest(r#"{"name":"onboarding","auto_start":"lazy"}"#)
        ));
    }

    fn app_with(manifest_json: &str, src: &str) -> tempfile::TempDir {
        let dir = tempfile::tempdir().unwrap();
        std::fs::create_dir_all(dir.path().join("src")).unwrap();
        std::fs::write(dir.path().join("src/index.ts"), src).unwrap();
        std::fs::write(dir.path().join("manifest.json"), manifest_json).unwrap();
        dir
    }

    #[test]
    fn flags_a_job_faster_than_the_default_floor() {
        // economic's real shape before the fix: 300s against a 600s min_idle.
        let dir = app_with(
            r#"{"name":"economic","app_type":"bun","auto_start":true}"#,
            r#"await invokeCapability("core.cron.register", { schedule: "0 */5 * * * *" });"#,
        );
        let m = manifest(r#"{"name":"economic","app_type":"bun","auto_start":true}"#);
        let f = check_lazy_app_cron_cadence_floor(dir.path(), Some(&m)).expect("expected a finding");
        assert_eq!(f.severity, Severity::Warning);
        assert!(f.message.contains("every 300s"), "{}", f.message);
        assert!(f.message.contains("600s"), "{}", f.message);
    }

    /// onboarding declares `min_idle_secs: 180`, so its floor is 300s, not
    /// 720s — the check must honour the per-app override in both directions.
    #[test]
    fn honours_a_per_app_min_idle_override() {
        let mj = r#"{"name":"onboarding","app_type":"bun","auto_start":"lazy","governor":{"min_idle_secs":180}}"#;
        let m = manifest(mj);

        let bad = app_with(
            mj,
            r#"invokeCapability("core.cron.register", { schedule: "0 * * * * *" });"#,
        );
        let f = check_lazy_app_cron_cadence_floor(bad.path(), Some(&m)).expect("60s must flag");
        assert!(f.message.contains("every 60s"), "{}", f.message);

        // 900s clears the 300s floor even though it would NOT clear the
        // default 720s one — the override is what matters.
        let good = app_with(
            mj,
            r#"invokeCapability("core.cron.register", { schedule: "0 */15 * * * *" });"#,
        );
        assert!(check_lazy_app_cron_cadence_floor(good.path(), Some(&m)).is_none());
    }

    #[test]
    fn ignores_cron_literals_in_files_that_never_register_a_job() {
        let mj = r#"{"name":"x","app_type":"bun"}"#;
        let m = manifest(mj);
        let dir = app_with(mj, r#"const DOC_EXAMPLE = "0 */5 * * * *"; // not a registration"#);
        assert!(check_lazy_app_cron_cadence_floor(dir.path(), Some(&m)).is_none());
    }

    #[test]
    fn reports_the_fastest_offending_job_when_several_are_present() {
        let mj = r#"{"name":"economic","app_type":"bun","auto_start":true}"#;
        let m = manifest(mj);
        let dir = app_with(
            mj,
            r#"invokeCapability("core.cron.register", [
                 { schedule: "0 */10 * * * *" },
                 { schedule: "0 */5 * * * *" },
                 { schedule: "0 0 3 * * *" },
               ]);"#,
        );
        let f = check_lazy_app_cron_cadence_floor(dir.path(), Some(&m)).unwrap();
        assert!(f.message.contains("every 300s"), "{}", f.message);
    }
}