RedBear-OS/local/docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md

# Canonical Build System Improvement Proposal

**Status:** Proposal — not yet implemented
**Context:** Identified while debugging the `redbear-mini` boot crash chain (`require_zero_offset`, `ContextHandle::Start`, `sys_statfs cbindgen.toml`, `SYS_MKNS`/`SYS_SETNS`, `mmap_min_addr` capping). The crash fixes were correct, but getting them through the build pipeline was painful and error-prone. This document proposes specific improvements to make that workflow reliable.

---

## Problems Encountered (Root-Caused)

1. **Edits in `local/sources/<fork>/` are silently stashed** by `build-redbear.sh` (line 160-172), then never restored. Build runs against the clean HEAD. User loses WIP from the working tree.
2. **`--force-rebuild` bypasses the BLAKE3 content-hash cache** (cook_build.rs:462) and all 14 validation gates in `build-redbear.sh`.
3. **Manual `repo cook` outside `build-redbear.sh`** skips stale detection, prefix rebuild, pre-cook sequencing, source-fingerprint tracking, and CI=1 (TUI protection).
4. **Source invalidation is mtime-only** (cook_build.rs:425-442). A `git checkout` or `cp -a` that preserves timestamps produces a stale cache. No content hash is computed for any source file.
5. **Cookbook binary staleness** (build-redbear.sh:182-185): only checks existence, not whether `src/` has changed since last build.
6. **No failure cleanup** in `build-redbear.sh`: no `trap` handler, no signal handler, partially-built artifacts left in `repo/` after failure.
7. **No strict-by-default gate** for uncommitted edits: `verify-durable-source-edits.py` only blocks under `--strict`, which is not the default.
8. **`recipes/core/relibc/source` is stashed but the other 5 fork sources are not** — silent inconsistency.

---

## Proposed Improvements (Ranked by Impact)

### Improvement 1: Make `stash_nested_repo_if_dirty` restore on exit

**Where:** `local/scripts/build-redbear.sh:160-172`
**Impact:** HIGH — eliminates the "where did my edits go?" confusion
**Complexity:** LOW

The current `stash_nested_repo_if_dirty` pushes a stash with `--all` and never restores it. Replace with a `trap`-based push-and-restore pattern:

```bash
REDBEAR_STASHED_REPOS=()

stash_nested_repo_if_dirty() {
    local target_dir="$1"
    local label="$2"
    if [ -d "$target_dir/.git" ]; then
        if ! git -C "$target_dir" diff --quiet || \
           ! git -C "$target_dir" diff --cached --quiet || \
           [ -n "$(git -C "$target_dir" ls-files --others --exclude-standard)" ]; then
            echo ">>> Stashing dirty $label checkout..."
            rm -f "$target_dir/.git/index.lock"
            if git -C "$target_dir" stash push --all \
                -m "build-redbear-auto-stash-$(date +%s)" >/dev/null 2>&1; then
                REDBEAR_STASHED_REPOS+=("$label:$target_dir")
            fi
        fi
    fi
}

restore_all_stashes() {
    local rc=$?
    local entry
    for entry in "${REDBEAR_STASHED_REPOS[@]}"; do
        local label="${entry%%:*}"
        local dir="${entry#*:}"
        if [ -d "$dir/.git" ]; then
            echo ">>> Restoring $label working tree..."
            rm -f "$dir/.git/index.lock"
            git -C "$dir" stash pop >/dev/null 2>&1 || \
                echo "WARN: failed to restore $label stash — check 'git -C $dir stash list'"
        fi
    done
    exit $rc
}

trap restore_all_stashes EXIT
```

Apply this to ALL fork sources, not just relibc:

```bash
for fork in relibc kernel base bootloader installer redoxfs; do
    local_fork_dir="$PROJECT_ROOT/local/sources/$fork"
    if [ -d "$local_fork_dir/.git" ]; then
        stash_nested_repo_if_dirty "$local_fork_dir" "$fork"
    fi
done
```

---

### Improvement 2: Source-content hashing for cache invalidation

**Where:** `src/cook/cook_build.rs:425-442` and `src/cook/fs.rs:160-168`
**Impact:** HIGH — eliminates the "stale cache after git checkout" bug class
**Complexity:** MEDIUM

Replace the mtime-based `modified_dir_ignore_git()` with a content-hash approach that covers all meaningful inputs:

```rust
// In src/cook/fs.rs or a new src/cook/source_hash.rs
fn compute_source_content_hash(source_dir: &Path, recipe_toml: &Path, patches: &[Path]) -> String {
    let mut hasher = blake3::Hasher::new();

    // Hash source directory contents (file-by-file, sorted by path)
    if let Ok(entries) = walk_dir_files_sorted(source_dir) {
        for (rel_path, abs_path) in entries {
            // Skip .git, target/, *.swp, etc.
            if rel_path.starts_with(".git/") || rel_path.contains("/target/") {
                continue;
            }
            hasher.update(rel_path.as_bytes());
            hasher.update(b"\0");
            if let Ok(content) = std::fs::read(&abs_path) {
                hasher.update(&content);
            }
            hasher.update(b"\0");
        }
    }

    // Hash recipe.toml
    if let Ok(content) = std::fs::read(recipe_toml) {
        hasher.update(b"recipe.toml\0");
        hasher.update(&content);
    }

    // Hash each patch file
    for patch in patches {
        if let Ok(content) = std::fs::read(patch) {
            hasher.update(patch.file_name().unwrap().as_encoded_bytes());
            hasher.update(b"\0");
            hasher.update(&content);
        }
    }

    hasher.finalize().to_hex().to_string()
}
```

In `build()` (cook_build.rs:425-460):

```rust
// Replace mtime-based source_modified with content hash
let source_hash = compute_source_content_hash(source_dir, &recipe_dir.join("recipe.toml"), &patches);
let stored_hash_file = get_sub_target_dir(target_dir, "source_hash.txt");

let source_changed = match std::fs::read_to_string(&stored_hash_file) {
    Ok(stored) if stored.trim() == source_hash => false,
    _ => true,
};

// ... after successful build:
std::fs::write(&stored_hash_file, &source_hash)?;
```

This makes the cache invalidation robust against `git checkout`, `cp -a`, and any other operation that changes content without updating mtime. It also eliminates the cbindgen.toml-specific fragility: the generated headers' source is the `*.rs` files + `cbindgen.toml`, all of which are hashed.

---

### Improvement 3: Cookbook binary freshness check

**Where:** `local/scripts/build-redbear.sh:182-185`
**Impact:** MEDIUM — prevents stale binary causing silent failures
**Complexity:** LOW

```bash
COOKBOOK_SRC_FINGERPRINT="$PROJECT_ROOT/target/release/.cookbook-src-fingerprint"
COOKBOOK_BIN="$PROJECT_ROOT/target/release/repo"
COOKBOOK_SRC_HASH=$(find "$PROJECT_ROOT/src" -name "*.rs" -print0 | sort -z | xargs -0 sha256sum | sha256sum | cut -d' ' -f1)

NEEDS_REBUILD=0
if [ ! -f "$COOKBOOK_BIN" ]; then
    NEEDS_REBUILD=1
elif [ ! -f "$COOKBOOK_SRC_FINGERPRINT" ] || [ "$(cat "$COOKBOOK_SRC_FINGERPRINT")" != "$COOKBOOK_SRC_HASH" ]; then
    NEEDS_REBUILD=1
fi

if [ "$NEEDS_REBUILD" = "1" ]; then
    echo ">>> Rebuilding cookbook binary (source changed or missing)..."
    cargo build --release
    echo -n "$COOKBOOK_SRC_HASH" > "$COOKBOOK_SRC_FINGERPRINT"
fi
```

---

### Improvement 4: Strict-by-default uncommitted-edit gate

**Where:** `local/scripts/build-redbear.sh` and `local/scripts/verify-durable-source-edits.py:48`
**Impact:** MEDIUM — prevents "what was that warning?" confusion
**Complexity:** LOW

Add an explicit gate before any source-touching operation:

```bash
# After .config parsing, before any fetch/cook
echo ">>> Checking for uncommitted source edits..."
DIRTY_FORKS=()
for fork in local/sources/relibc local/sources/kernel local/sources/base \
           local/sources/bootloader local/sources/installer local/sources/redoxfs; do
    fork_dir="$PROJECT_ROOT/$fork"
    if [ -d "$fork_dir/.git" ] && \
       (! git -C "$fork_dir" diff --quiet HEAD 2>/dev/null || \
        ! git -C "$fork_dir" diff --cached --quiet HEAD 2>/dev/null || \
        [ -n "$(git -C "$fork_dir" ls-files --others --exclude-standard 2>/dev/null)" ]); then
        DIRTY_FORKS+=("$fork")
    fi
done

if [ ${#DIRTY_FORKS[@]} -gt 0 ]; then
    echo "WARNING: uncommitted edits detected in:"
    for f in "${DIRTY_FORKS[@]}"; do
        echo "  - $f"
    done
    echo ""
    if [ "${REDBEAR_ALLOW_DIRTY:-0}" = "1" ]; then
        echo ">>> REDBEAR_ALLOW_DIRTY=1 set; proceeding with WIP edits."
    else
        echo "ERROR: refuse to build with uncommitted edits."
        echo "       Either commit your changes or set REDBEAR_ALLOW_DIRTY=1 to override."
        exit 1
    fi
fi
```

For `verify-durable-source-edits.py`, flip the default to strict and make `--no-strict` the override:

```python
# Line 48: change default from non-strict to strict
if not args.no_strict and dirty:
    print("ERROR: uncommitted edits detected in upstream-owned source trees.")
    print("       Use --no-strict to allow, or commit your changes.")
    sys.exit(1)
```

---

### Improvement 5: Stash-all-forks consistency

**Where:** `local/scripts/build-redbear.sh:160-172`
**Impact:** MEDIUM — eliminates the "only relibc is special" asymmetry
**Complexity:** LOW

Combined with Improvement 1, replace the single-relibc stash with a loop over all fork sources. The key insight: `local/sources/<fork>/` is the source of truth, but the cookbook fetches into `recipes/core/<fork>/source` (which is a symlink to `local/sources/<fork>/`). Stashing must happen at the `local/sources/<fork>/` level, not at the recipe-level symlink.

---

### Improvement 6: Failure-cleanup trap

**Where:** `local/scripts/build-redbear.sh` (top-level, after `set -euo pipefail`)
**Impact:** MEDIUM — makes failed builds debuggable
**Complexity:** LOW

```bash
REDBEAR_BUILD_STATE_DIR=$(mktemp -d)

cleanup_on_failure() {
    local rc=$?
    if [ $rc -ne 0 ]; then
        echo ""
        echo "========================================"
        echo "  BUILD FAILED (exit code: $rc)"
        echo "========================================"
        echo "Diagnostic info preserved at: $REDBEAR_BUILD_STATE_DIR"
        echo ""
        echo "Last 30 lines of each recipe log:"
        for log in /tmp/build-*.log; do
            [ -f "$log" ] || continue
            echo "--- $log ---"
            tail -30 "$log"
            echo ""
        done 2>/dev/null
        echo "Recipe build state:"
        for d in "$PROJECT_ROOT"/recipes/*/target/*/; do
            [ -d "$d" ] || continue
            echo "  $d: $(du -sh "$d" 2>/dev/null | cut -f1)"
        done
    fi
    rm -rf "$REDBEAR_BUILD_STATE_DIR"
    # Then restore stashes (from Improvement 1)
    restore_all_stashes
}
trap cleanup_on_failure EXIT
```

---

### Improvement 7: Pre-cook offline/upstream consistency

**Where:** `local/scripts/build-redbear.sh:339-357`
**Impact:** LOW-MEDIUM — prevents pre-cook from fetching packages the main build can't use
**Complexity:** LOW

```bash
# Replace line 349
if [ "${REDBEAR_ALLOW_UPSTREAM:-0}" = "1" ]; then
    COOKBOOK_OFFLINE=false "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1
elif [ -n "${REDBEAR_RELEASE:-}" ]; then
    COOKBOOK_OFFLINE=true "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1
else
    COOKBOOK_OFFLINE=true "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1
fi
```

---

### Improvement 8: Cookbook protection against out-of-band invocations

**Where:** `src/bin/repo.rs` (CLI entry point)
**Impact:** LOW — prevents accidental misuse, gentle guardrail
**Complexity:** LOW

Add a non-blocking warning when `repo cook` is invoked outside `build-redbear.sh`:

```rust
// In src/bin/repo.rs main_inner()
let canonical_marker = std::env::var_os("REDBEAR_CANONICAL_BUILD");
if canonical_marker.is_none() {
    eprintln!("WARNING: repo cook invoked outside build-redbear.sh.");
    eprintln!("         Cache invalidation, prefix rebuild, and source-fingerprint");
    eprintln!("         tracking will not run. Set REDBEAR_CANONICAL_BUILD=1 to suppress");
    eprintln!("         this warning if you know what you're doing.");
}
```

In `build-redbear.sh`:
```bash
export REDBEAR_CANONICAL_BUILD=1
```

---

## Implementation Order

Suggested order (each step is independently useful and can be merged separately):

1. **Improvement 1 + 5** (trap-based stash restore, all forks) — eliminates the biggest UX pain point
2. **Improvement 6** (failure-cleanup trap with diagnostics) — makes failures debuggable
3. **Improvement 3** (cookbook binary freshness) — 5-line change, high signal
4. **Improvement 4** (strict-by-default dirty gate) — prevents repeat-accumulation of uncommitted edits
5. **Improvement 7** (pre-cook offline consistency) — small correctness fix
6. **Improvement 8** (cookbook out-of-band warning) — gentle nudge
7. **Improvement 2** (source content hashing) — biggest engineering effort, defer until after the above land

---

## Testing Strategy

Each improvement needs:
1. Unit test in `src/` if it touches cookbook code (cookbook has a test directory)
2. Manual smoke test: `./local/scripts/build-redbear.sh redbear-mini` end-to-end
3. Regression test: dirty `local/sources/relibc/` + `./local/scripts/build-redbear.sh redbear-mini` + verify stash is restored on exit
4. Documentation update: this file's "Problems Encountered" should be empty after all improvements land

---

## Related Documents

- `local/AGENTS.md` — project-wide build and commit policies
- `local/docs/LOCAL-FORK-SUPREMACY-POLICY.md` — why local forks must be complete and committed
- `local/docs/BUILD-CACHE-PLAN.md` — content-hash cache design (Phase 1-3 status, sysroot deferral)
- `local/docs/BUILD-SYSTEM-INVARIANTS.md` — what the build system guarantees
- `local/docs/BUILD-SYSTEM-HARDENING-PLAN.md` — broader hardening roadmap
- `mk/repo.mk` — make-integration of cookbook
- `local/scripts/build-redbear.sh` — canonical entry point