main
Drew Owens 0.9.0: bridge data moved out of the install cache (recovery survives updates), closed-in-place records, gates route thr… 5d02ca9 20d ago
//! ADVERSARIAL REVIEW OF 0.8.5 — pass thirty-five.
//!
//! RELEASE HYGIENE (target 5) IS CLEAN, and I checked it properly rather than
//! by eye:
//!
//!   * `bridge.json` declares 66 verbs; the server dispatches 66; the two sets
//!     are EQUAL — nothing declared that cannot be called, nothing callable that
//!     ab cannot see.
//!   * `verbPrefixes: ["swx_"]` covers all 66. `statusVerb: "swx_status"` exists.
//!   * Version is 0.8.5 in all four places that carry one: `Cargo.toml`, the
//!     `BRIDGE_VERSION` file, `bridge.json`, and `BRIDGE_VERSION` in `lib.rs`
//!     (which is `env!("CARGO_PKG_VERSION")`, so it cannot drift).
//!   * The only file `bridge.json` names is `swx-bridge.exe`, the built artifact.
//!     Its other paths are SOLIDWORKS install locations on the USER's machine
//!     and the manifest URL, neither of which belongs in the zip.
//!   * `resources/macros/adom-hello.swp` is present, and the one code path that
//!     needs a macro (`quit` rung e) already degrades to `skipped` with a reason
//!     when it is absent, so a zip missing it would refuse rather than misbehave.
//!
//! THE DETECTOR ADAPTATIONS (the coordinator's real question) mostly HELD. The
//! never-ask pin now joins `\`-continued literals, walks the repo-root markdown,
//! and its dialog exemption gained the negative conjuncts that close the hole I
//! reported — "answer or dismiss a DIALOG, and not also close the documents".
//! That is a stronger pin, not a weaker one. The residual is below.
//!
//! The drain gate's manifest is well designed on the point I expected to break:
//! `remember()` records a creation BEFORE making it, so a crash in between
//! leaves a PHANTOM entry rather than a missing one — and cleanup tolerates
//! `document_not_open` / `no_target_document` and wraps every removal in a
//! swallow. A phantom is harmless; the reverse would not have been.

mod common;
use serde_json::json;
use std::path::Path;

fn gate_src() -> String {
    std::fs::read_to_string(Path::new(env!("CARGO_MANIFEST_DIR")).join("tools/drain-gate.py"))
        .expect("tools/drain-gate.py")
}

/// SEVERITY 1 — THE TWO FIXES CANCEL: PRIOR-RUN RECOVERY CAN NEVER FIRE.
///
/// 0.8.5 took two of the three fixes I offered in pass 34, and they are mutually
/// exclusive as built.
///
///   * the scratch root is now UNIQUE PER RUN:
///     `swx-drain-gate-<pid>-<epoch>` (drain-gate.py:392);
///   * `prior_manifests(root)` — "Manifests left by earlier runs — their
///     leftovers are ours too" — lists `run-*.json` inside **that same unique
///     root** (:255, called at :279).
///
/// A directory created seconds ago by this process contains no earlier run's
/// manifest, and the one manifest it does contain is excluded by
/// `p != MANIFEST_PATH`. So the whole adoption path is unreachable. Every
/// crashed run's leftovers sit in ITS OWN `swx-drain-gate-<pid>-<time>`
/// directory that nothing will ever look at again.
///
/// `atexit` covers the cases that mattered most — a refusal, an exception, a
/// Ctrl-C — and that is the important half and it works. It does not cover
/// SIGKILL, a hard interpreter crash, or a reboot, which is exactly what
/// `prior_manifests` was written for. And the unique root makes the residue
/// worse in one respect: crashed runs now leave a DIRECTORY each instead of
/// sharing one.
///
/// One line: scan `tempfile.gettempdir()` for `swx-drain-gate-*`, not just this
/// run's root. Do it together with the containment check in the next test —
/// fixing this one without that one is what turns it into a real hazard.
#[test]
fn the_gate_can_still_find_leftovers_from_a_run_that_died_hard() {
    let src = gate_src();
    let unique_root = src.contains("swx-drain-gate-%d-%d") || src.contains("swx-drain-gate-{");
    // NOT `contains("glob")` — this file has `global _CLEANED` and `global LOCAL`,
    // and matching those is how my first version of this test PASSED on a bug
    // that is really there. Look for a scan of the TEMP ROOT specifically.
    let scans_temp_root = src.contains("glob.glob")
        || src.contains("iglob")
        || src.contains("listdir(tempfile.gettempdir())")
        || src.contains("listdir(TEMP_ROOT")
        || src.contains("prior_manifests(tempfile.gettempdir())")
        || src.contains("prior_manifests(TEMP_ROOT");

    assert!(
        !unique_root || scans_temp_root,
        "the scratch root is unique per run (swx-drain-gate-<pid>-<epoch>) and prior_manifests() \
         lists run-*.json inside THAT root, so it can only ever see this run's own manifest — \
         which the caller then filters out with `p != MANIFEST_PATH`. The adoption path is \
         unreachable: \"Manifests left by earlier runs — their leftovers are ours too\" never \
         adopts anything. atexit covers refusals, exceptions and Ctrl-C, which is the half that \
         mattered and it works; it does not cover SIGKILL, a hard crash or a reboot, which is \
         what prior_manifests exists for. And a unique root means each such run now leaves its \
         own DIRECTORY behind. Scan the temp root for swx-drain-gate-*. \
         (unique root: {unique_root}, scans temp root: {scans_temp_root})"
    );
}

/// SEVERITY 1, FORWARD-LOOKING — THE USER-WRITE OPT-IN HAS NO CONTAINMENT CHECK.
///
/// `cleanup` closes every document in every manifest with the full escalation:
///
/// ```python
/// c = verb("swx_close_doc", {
///     "path": doc,
///     "allowUserWrites": True,
///     "confirm": True,
///     "reason": "… (its own manifest is the ownership proof)",
/// })
/// ```
///
/// Two problems, and the second is only latent because the first test's bug is
/// keeping it that way.
///
/// The opt-in is applied to EVERY document in the manifest, though the comment
/// beside it explains that only the TWIN needs it — "Most of these the bridge
/// created and can close in the default posture." Widening a user-write
/// escalation past the one case that requires it is the opposite of how every
/// other gate in this project is written.
///
/// And the target list comes from a FILE ON DISK with nothing binding it: no
/// check that `doc` lies under the gate's own scratch root, no pid, no
/// timestamp. "Its own manifest is the ownership proof" is the claim; the code
/// verifies neither that the manifest is its own nor that the path is somewhere
/// it could have written. Today `prior_manifests` is dead, so the only manifest
/// read is the one this process just wrote — which is why this is latent rather
/// than live. Fix the previous finding without this one and the gate starts
/// reading `run-*.json` out of `%TEMP%`, a directory anything can write to, and
/// feeding those paths to a confirmed user-write close.
///
/// This bridge's whole doctrine is "no handle, no kill" — attribution before
/// action. One `os.path.commonpath` against the scratch root restores it.
#[test]
fn the_gates_user_write_close_is_scoped_to_what_it_could_have_written() {
    let src = gate_src();
    let block = src
        .split("def cleanup(")
        .nth(1)
        .map(|s| s.chars().take(2600).collect::<String>())
        .expect("cleanup exists");

    let uses_opt_in = block.contains("\"allowUserWrites\": True");
    let contained = block.contains("commonpath")
        || block.contains("startswith(scratch_root")
        || block.contains("_is_ours(");
    let opt_in_scoped_to_twin = block.contains("doc == twin") || block.contains("is_twin");

    assert!(
        !uses_opt_in || (contained && opt_in_scoped_to_twin),
        "cleanup passes allowUserWrites+confirm for EVERY document in EVERY manifest, though its \
         own comment says only the TWIN needs it, and it never checks that the path lies under \
         the scratch root it created. The target list is a JSON file on disk and \"its own \
         manifest is the ownership proof\" is asserted, not verified. It is latent only because \
         prior_manifests is currently unreachable — fix that without this and the gate will read \
         run-*.json out of %TEMP% and feed those paths to a confirmed user-write close. This \
         project's doctrine is attribution before action. \
         (opt-in: {uses_opt_in}, containment: {contained}, scoped to twin: {opt_in_scoped_to_twin})"
    );
}

/// SEVERITY 2 — THE PIN PASSES BECAUSE ITS EXEMPTION LIST GREW UNTIL IT DID.
///
/// The rewrite is a real improvement and I want to say so plainly: joining
/// `\`-continued literals closed the wrapping hole, the repo-root markdown is
/// scanned, and the dialog exemption is now a conjunction —
/// `(answer|dismiss) && dialog && !close the document && !close them &&
/// !close their && !by hand` — which is precisely the narrowing I asked for.
///
/// What came with it is eight ad-hoc escape hatches, added so the pin would pass
/// over this project's own commentary:
///
/// ```ignore
/// || l.contains("used to end here") || l.contains("passed.")
/// || l.contains("is not, and") || l.contains("a test now fails")
/// || l.contains("restart") || l.contains("their screen")
/// || l.contains("has closed") || l.contains("was closed")
/// ```
///
/// MEASURED, by planting each sentence in `src/verbs3.rs` and running the pin:
///
/// ```text
/// "A dialog is up, so ask the user to \ close the documents by hand."   CAUGHT
///   (same sentence as a SKILL.md list item)                             CAUGHT
/// "The sweep passed. Ask the user to close the remaining documents."    SLIPS
/// "After the restart, ask the user to close the leftover documents."    SLIPS
/// ```
///
/// The first two are the holes I reported in pass 33, and the rewrite genuinely
/// closes them — including the wrapped form and the repo-root markdown. The
/// second two are the new exemptions, and they are not theoretical:
/// `l.contains("passed.")` exempts any statement containing those seven
/// characters. *"The sweep passed. Ask the user to close the remaining
/// documents."* is exempt. `l.contains("restart")` exempts *"after the restart,
/// ask the user to close the leftover documents"*. These are not principled
/// categories like the dialog one; they are the text of particular sentences
/// that exist today, and each is a permanent hole shaped like a phrase.
///
/// The structural fix is the one every classifier in this project already has
/// and this one does not: a TABLE OF CASES. `classify_window` is pinned by
/// `cases: &[(&str, u32, &str, Option<&str>)]` twenty lines below — known input,
/// expected verdict. The same table here, holding the sentence from my pass-33
/// report and the legitimate ones each exemption was added for, would make every
/// future exemption provably safe instead of provably convenient. Without it the
/// only way to know whether this pin works is to read it, which is what made the
/// original version wrong for a week.
#[test]
fn the_never_ask_pin_is_pinned_by_examples_not_by_reading() {
    let src = std::fs::read_to_string(
        Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/wiring.rs"),
    )
    .expect("wiring.rs");
    let pin = src
        .split("fn nothing_asks_the_user_to_close_documents")
        .nth(1)
        .map(|s| s.chars().take(6000).collect::<String>())
        .expect("the pin exists");

    // A sentence the rule forbids, and one it must allow: does the pin carry
    // either as a case it actually evaluates?
    let has_cases = pin.contains("cases") || pin.contains("EXAMPLES")
        || pin.contains("known_bad") || pin.contains("must_flag");
    let broad_escape = pin.contains(r#"l.contains("passed.")"#)
        || pin.contains(r#"l.contains("restart")"#);

    assert!(
        has_cases && !broad_escape,
        "the pin has no table of example sentences, and it acquired eight phrase-shaped \
         exemptions so it would pass over this project's own prose. `l.contains(\"passed.\")` \
         exempts \"The sweep passed. Ask the user to close the remaining documents.\"; \
         `l.contains(\"restart\")` exempts \"after the restart, ask the user to close the \
         leftover documents\". The dialog exemption IS principled — a conjunction with three \
         negative conjuncts — and these are not; they are the text of sentences that happen to \
         exist. classify_window twenty lines below is pinned by a `cases` table of input and \
         expected verdict. The same table here, carrying the sentence from my pass-33 report \
         and the legitimate ones each exemption was added for, makes every future exemption \
         provably safe rather than provably convenient. \
         (cases: {has_cases}, broad escapes: {broad_escape})"
    );
}

/// The release manifest and the server agree, checked rather than assumed.
/// Passing today — kept because nothing else in the suite compares them, and a
/// verb missing from `bridge.json` is invisible to ab with no other symptom.
#[test]
fn every_verb_the_server_dispatches_is_declared_to_ab() {
    let root = Path::new(env!("CARGO_MANIFEST_DIR"));
    let manifest: serde_json::Value =
        serde_json::from_str(&std::fs::read_to_string(root.join("bridge.json")).expect("bridge.json"))
            .expect("bridge.json parses");
    let declared: std::collections::BTreeSet<String> = manifest["verbs"]
        .as_array()
        .expect("verbs is an array")
        .iter()
        .filter_map(|v| v.as_str().map(|s| s.to_string()))
        .collect();

    let mut dispatched: std::collections::BTreeSet<String> = Default::default();
    for f in ["verbs.rs", "verbs2.rs", "verbs3.rs"] {
        let p = root.join("src").join(f);
        let Ok(text) = std::fs::read_to_string(&p) else { continue };
        for line in text.lines() {
            if let Some(i) = line.find("\"swx_") {
                let rest = &line[i + 1..];
                if let Some(j) = rest.find('"') {
                    let name = &rest[..j];
                    if rest[j..].trim_start_matches('"').trim_start().starts_with("=>") {
                        dispatched.insert(name.to_string());
                    }
                }
            }
        }
    }

    assert!(
        !dispatched.is_empty(),
        "precondition: the dispatch table was found"
    );
    let undeclared: Vec<&String> = dispatched.difference(&declared).collect();
    let uncallable: Vec<&String> = declared.difference(&dispatched).collect();
    assert!(
        undeclared.is_empty() && uncallable.is_empty(),
        "bridge.json and the server's dispatch table disagree. A verb the server dispatches but \
         bridge.json does not declare is INVISIBLE to ab — it simply does not appear, with no \
         error anywhere. One declared but not dispatched is advertised and fails on use.\n  \
         dispatched, undeclared: {undeclared:?}\n  declared, not dispatched: {uncallable:?}"
    );
    assert_eq!(
        manifest["version"].as_str(),
        Some(env!("CARGO_PKG_VERSION")),
        "bridge.json's version must match the crate it ships"
    );
    let _ = json!({});
}