tofu/.omo/notepads/tofu-core/learnings.md

# Learnings — tofu-core

Conventions, patterns, and successful approaches discovered during work on this plan.

_Auto-scaffolded by /start-work. Append new entries below - never overwrite._

---

## Task 5 — tofu.vercmp (RPM-style version comparison)

### Module naming
- `version` is a D reserved keyword (conditional compilation). Module is named
  `tofu.vercmp`, file `src/tofu/vercmp.d`.  The `DepConstraint` struct in
  `types.d` uses field `ver` (not `version`) for the same reason.

### Algorithm (exact port from ZETA/lib/vercmp.lua)
- `nextSegment`: skips any non-digit/non-letter chars (separators); then reads
  a contiguous run of same-type chars.  Returns `(segment, nextIndex)`.
- `cmpNumeric`: strips leading zeros from both segments (`"007"` → `"7"`,
  `"000"` → `"0"`).  Longer stripped string wins; then lexicographic.
- `compare`: strips all whitespace, then loops over alternating segments.
  If both segments are digit-typed → `cmpNumeric`, else lexical.  When one
  version exhausts segments: the exhausted one is *older* (shorter = older).
  This matches RPM semantics: `compare("1.0.0", "1.0") > 0`.
- `satisfies`: uses `final switch` on `DepOp` enum.  Maps to `compare` result
  as expected (ge→c>=0, le→c<=0, eq→c==0, ne→c!=0, gt→c>0, lt→c<0).
- `parseDep`: delegates to `DepConstraint.parse` (already ported in types.d by
  task 4).  Both implementations are kept consistent.

### whitespace stripping
- Could not use `filter`+`array` because Phobos auto-decodes `string` to
  `dchar` range elements, returning `dchar[]` not `string`.  Implemented a
  manual `removeWhitespace` helper using raw code-unit indexing, with a
  `@trusted` cast since the freshly-allocated `char[]` has no aliasing.

### Test porting
- All 10 ZETA test categories ported as separate `@safe unittest` blocks.
- 4 modules pass unittests (types, vercmp, plus pre-existing ones).
- `dub build` passes with warnings-as-errors enabled.

### Future considerations
- When task 12 (constraint resolution) imports `tofu.vercmp`, the `compare`
  and `satisfies` functions are ready.  The `parseDep` integration with
  `DepConstraint.parse` ensures consistency between ZETA and tofu's dep
  constraint semantics.

---

## Task 2: Config Loading (src/tofu/config.d)

### toml package API (v1.0.0, Kripth/toml)
- Import: `import toml;` → provides `TOMLDocument`, `TOMLValue`, `parseTOML`, `TOMLException`, `TOMLParserException`
- `TOMLDocument` has `alias table this` — acts like `TOMLValue[string]` AA
- Access sections: `doc["core"]` returns `TOMLValue` with `type == TOML_TYPE.TABLE`
- Access string values: `doc["core"]["key"].str` (throws if type mismatch)
- Check existence: `"key" in doc` returns `TOMLValue*` (null if absent)
- `parseTOML()` is `@system` — must wrap in `@trusted` block when calling from `@safe` code
- Keys without `.str` / `.integer` access will throw `TOMLException`

### D language gotchas
- **Nested function declarations**: D does NOT allow `void foo() { ... }` inside a function body. Use lambdas: `auto foo = () { ... }; foo();` or anonymous `() @trusted { ... }();`
- **`version` is a keyword**: Cannot use `string version;` as field name — use `string version_;` or initialize with `= ""`
- **`parseTOML` is `@system`**: The toml package's parse function isn't marked `@safe` — must wrap in `@trusted` lambda
- **`std.file.readText` is `@system`**: Same — wrap in `@trusted`
- **`std.process.environment.get()`**: Returns empty string when env var is unset; `"VAR" in environment` checks existence but `environment.get("VAR", "default")` is cleaner
- **`stderr.writefln` is `@safe`** in recent DMD (2.106+) — usable directly in `@safe` code
- **AA `in` operator**: Returns pointer (`V*`) or `null` — must check for null before dereferencing

### Pattern: env-overrideable config loading
- Priority: env vars > TOML file > hardcoded defaults
- TOML read happens BEFORE env override — so TOML values serve as base, env vars overwrite
- Missing config file → no error, just skip to defaults
- Malformed TOML → `TOMLParserException` caught, warning to stderr, fall back to defaults
- Testability: `load()` accepts optional `envOverrides` map and explicit `configFile` path to avoid real env/filesystem

### Config design decisions
- `TOFU_CONFIG` env var overrides default config file path (`~/.config/tofu/config.toml`)
- `~` in default paths expanded with `std.path.expandTilde`
- All path fields default to empty string when "find on PATH" — no hardcoded binary paths
- `defaultJobs` typed as `int` (not `size_t`/`ulong`) since it's a user-facing count

### buildOptions warningsAsErrors deprecation
- DUB warns about `warningsAsErrors` in `buildOptions` — recommends `buildRequirements` instead
- Not blocking, informational only

## Task 3 — `tofu.log` (colored, NO_COLOR-aware logging)

- **dmd 2.112 gotcha**: the `std.stdio` globals `stdout`/`stderr` are `@system` to *access* — `makeGlobal` uses `__gshared File result`, which fails `@safe` inference. An `@safe` function cannot call `stdout.writeln(...)` directly. Fix: route every write through a tiny `@trusted` helper (`writeStdout(string)`/`writeStderr(string)`), keep the public log functions `@safe`. Compiler hint: *"using `__gshared` instead of `shared` makes it fail to infer `@safe`"*.
- **`std.process.environment` (dmd 2.112)**: it is an `abstract final class` of *static* methods — `environment.get(name)` is `@safe` and returns `null` when the var is unset (so "NO_COLOR set" == `get !is null`, even for an empty string). `environment["K"] = v` and `environment.remove(name)` are `@trusted`. NOT deprecated in 2.112.
- **Testable color decision**: make `colorEnabled()` re-read the environment on every call instead of caching in `static this()`. Enables NO_COLOR/TERM tests without restarting the process. Costs one `getenv` per log line — negligible.
- **Env save/restore in unittests**: a `private struct ColorEnv` (in `version(unittest)`) that snapshots NO_COLOR+TERM in its ctor and restores them in `~this()` (destructor) — `auto env = ColorEnv("1", "xterm")` scopes the restore. `null` value = remove the var. Double-destruction from a copied temporary is harmless (restore is idempotent).
- **Capturing stdout/stderr in unittests**: swap the global `stdout`/`stderr` File to a `File(name, "w+")`, write, `flush()`, swap back, then `rewind()` + `readln()`. Restore the global before the temp File's destructor runs.
- **zeta log format details**: `warn ` paints the *whole prefix* yellow (`paint("yellow", "warn ") ~ msg`), but `error` paints *only the word* then `" " ~ msg` (`paint("red","error") ~ " " ~ msg`). Assertion must expect `\x1b[31merror\x1b[0m failed deploy` — the reset code sits between "error" and the message.
- **`version` is a D keyword** — cannot be used as a struct field / identifier (trap for the parallel `types.d` work).
- **`std.stdio.writeln` flushes per call** — already satisfies "no buffering"; no explicit `flush()` needed in production paths.
- **Verifying a single module without the full build**: `dmd -main -unittest -i src/tofu/log.d` runs just that module's unittests when sibling modules (`config.d`, `types.d`) are mid-edit by parallel agents. `-i` pulls imports in automatically.

## Task 4 — `tofu.types` (core data structures)

- **`version` is a D keyword** — all struct fields named `version` must be renamed to `ver`. The D compiler treats `version` as a conditional-compilation directive and rejects it as a field name (see task 3 learnings above).
- **Struct field defaults in D**: `string` fields initialise to `null` by default. To enforce empty-string semantics (`""`), every string field must be explicitly `= ""`.
- **`final switch` for exhaustive enum handling**: using `final switch` on enum types forces the compiler to verify all cases are covered — catches missing branches at compile time. Used for `poolToString()`.
- **Manual character scanning preferred over `std.regex`**: the dep-spec parser uses simple char-by-char scanning with `isNameChar()` / `isWhite()` helpers instead of regex. This avoids the `std.regex` dependency, keeps the parser `@safe`, and matches the Lua reference semantics exactly.
- **Dep spec parsing order matters**: 2-character operators (`>=`, `<=`, `==`, `~=`) MUST be checked before 1-character operators (`>`, `<`, `=`) to avoid false matches. The Lua reference uses an ordered table `OPS = { ">=", "<=", "==", "~=", ">", "<", "=" }` — we replicate this with if/else chains.
- **`=` normalises to `==`**: the Lua code has `if op == "=" then op = "==" end`; we map both to `DepOp.eq`.
- **Trailing garbage detection**: after extracting version chars, any non-whitespace remaining in the spec string causes a `TypesException`. The Lua pattern uses `$` anchor for this.
- **`dub test` includes all `.d` files in source paths** for executable targets, not just imported modules. Pre-existing compile errors in sibling modules (config.d) block test runs. Fixed config.d by removing `private` from a nested function (access specifiers not allowed on local functions in D).
- **`BuildResult.failed` as `BuildFailure[]`**: the plan describes it as `struct {string name; string reason}[]` — in D this is a named struct `BuildFailure` with array field `BuildFailure[] failed`. The `failedNames()` helper extracts just the names for caller convenience.
- **`DepConstraint.parse` is a static factory method**: returns a new `DepConstraint` by value, never allocates on the heap. `@safe` throughout.
- **All structs are `@safe`**: no methods do I/O, no `@system` calls. The module stands alone — no imports from other tofu modules.

---

## Task 6 — `tofu.http` (sync HTTP client via std.net.curl)

### std.net.curl API (key members for HTTP struct)
- **Construction**: `HTTP(url)` via `opCall`, or `HTTP()` + `.url = url` setter.
  The `.url` setter auto-prepends `http://` if no scheme is present.
- **Timeouts** (all are `@property` setters taking `core.time.Duration`):
  - `connectTimeout` — connect phase only (`CurlOption.connecttimeout_ms`).
  - `operationTimeout` — DNS + connect + transfer total (`CurlOption.timeout_ms`).
  - `dataTimeout` — low-speed activity timeout (NOT total read timeout;
    sets `low_speed_limit` + `low_speed_time`).
  - For a "read 120s" requirement use `operationTimeout = 120.seconds` —
    it covers the entire operation including data transfer.
- **Redirects**: `maxRedirects` (property setter, takes `uint`).
  Defaults to 10.  Set to 0 to disable, `uint.max` for infinite.
  Internally sets `CurlOption.followlocation` + `CurlOption.maxredirs`.
- **User-Agent**: `setUserAgent(string)` — instance method (not property).
- **onReceive**: `void delegate(ubyte[])` returning `size_t`.  Must accept
  all bytes or the request aborts.  Use `return data.length;`.
- **onReceiveStatusLine**: Not a direct property on HTTP struct — must
  be set via the `onReceiveHeader` property setter which wraps the
  delegate and internally fires `onReceiveStatusLine` when it detects
  an HTTP status line.  After `perform()` the final status is available
  via `http.statusLine`.
- **statusLine**: `HTTP.StatusLine` struct with `.code` (`ushort`),
  `.majorVersion`, `.minorVersion`, `.reason`.  Reset to zero before
  each `perform()`.
- **perform()**: `CurlCode perform(ThrowOnError = Yes.throwOnError)`.
  Default throws on curl errors, but the thrown exception classes are:
  - `CurlTimeoutException : CurlException` — operation timed out (code 28).
  - `CurlException` — all other curl errors.
  - `HTTPStatusException : CurlException` — only thrown by high-level
    wrappers (_basicHTTP); low-level `perform()` does NOT throw on
    HTTP status codes — you must check `statusLine.code` yourself.
- **Exception isolation pattern**: catch `CurlTimeoutException` and
  `CurlException` in `@trusted` helpers, re-throw as `HttpException`
  (which extends `Exception`, not `CurlException`).  Callers never see
  curl types.
- **verifyPeer / verifyHost**: On by default in HTTP struct (no change needed).

### @safe / @trusted architecture
- All public functions (`get`, `downloadFile`) are `@safe`.
- The `configure(HTTP)`, `getImpl`, and `downloadFileImpl` helpers are
  `@trusted` — they are the ONLY places `std.net.curl` (which is
  `@system`-heavy) is used.

### Download pattern (.part temp file + rename)
- Ported from ZETA `lib/fetch.lua`: write to `dest.part`, rename on
  success, remove partial on error.  `scope(failure)` at function
  level handles all error paths automatically.
- `File(tmpPath, "wb")` — binary write mode.  File destructor auto-closes;
  explicit `f.close()` needed before rename.

### Unittest local server
- Used `std.concurrency.spawn` + `std.socket.TcpSocket` for a
  self-contained one-shot HTTP server (no external dependency on Python3).
- Pattern: bind ephemeral port (`InternetAddress.PORT_ANY`), spawn thread
  that `accept()`s one connection, sends canned HTTP response, exits.
  `cast(shared)` the listener to pass to `spawn`.  Mark test helpers
  `@trusted`.
- HTTP response must include `Content-Length` header or libcurl hangs.

### Connection refused test
- Bind socket, record port, close it, then `get()` to that port.
  curl returns error 7 (`CURLE_COULDNT_CONNECT`) → `CurlException`
  → caught and re-thrown as `HttpException`.

### Build verified
- `dub build` passes with warnings-as-errors.
- `dub test` passes — all 5 modules (config, log, types, vercmp, http).

---

## Task 8 — `tofu.fetch` (ZUUR recipe download to cache)

### Architecture
- `fetchRecipe(name, cfg)` orchestrates a known-file download sequence
  (no directory listing assumed — plan constraint).
- Download order: .recipe (required) → package.lua (optional) → build.sh
  (optional) → custom build_script (if referenced).
- 404 on .recipe translates to `FetchException("package 'X' not found in
  ZUUR recipes")` (user-friendly).
- 404 on optional files (package.lua, build.sh) is logged and skipped.
- 404 on a referenced `build_script` is a real error (build cannot proceed).
- Non-404 HTTP errors or connection failures are caught, partial files
  cleaned up, and re-thrown as `FetchException`.

### Light recipe scanning
- Full Lua parsing is task 9/10's job. Task 8 only needs to discover
  `build_system = "custom"` + `build_script = "..."` to fetch referenced
  build scripts.
- Manual string scanning: find key, skip whitespace/`=`, read quoted value.
  Handles arbitrary whitespace variations. Avoids `std.regex` dependency.
- `extractBuildScript(content)` returns the script path only when
  `build_system = "custom"` is also found.

### `@safe` / `@trusted` architecture
- `fetchRecipe` and `extractBuildScript` are `@safe`.
- Filesystem operations (`exists`, `mkdirRecurse`, `readText`, `remove`,
  `rmdirRecurse`, `write`) are isolated in `@trusted` wrappers.
- Follows the same pattern as `http.d` and `config.d`.
- `std.file.readText` and friends are `@system` in DMD 2.112.

### Name conflicts between imports
- `tofu.config.Config` conflicts with `std.process.Config` (both imported
  in `version(unittest)` blocks). Solution: fully qualify as
  `tofu.config.Config` in function signatures where `std.process` is
  also imported.

### `std.string.indexOf` vs `std.algorithm.canFind`
- `"string".canFind("substr")` fails in D because UFCS on `string`
  (a range of `dchar`) tries to find a `string` element in a `dchar`
  range. Use `e.msg.indexOf("pattern") >= 0` instead.
- `std.algorithm.searching.canFind(haystack, needle)` as a free function
  works but is less readable.

### Test strategy
- Used `python3 -m http.server` on ephemeral ports for multi-file HTTP
  serving — the one-shot TCP responder from `http.d` handles only one
  connection per spawn.
- Pattern: `findFreePort()` (bind + close ephemeral port), spawn server
  on that port, `waitForPort()` (connection polling with 100ms sleep,
  50 attempts max), run test, `killServer(pid)` (SIGTERM).
- `spawnProcess` returns `Pid` (a class, not a struct with `.pid` field).
  Store the `Pid` object directly, not `.pid`.
- `Socket.localAddress()` returns abstract `Address` — must cast to
  `InternetAddress` to access `.port`.
- Five test scenarios: happy path, 404-on-recipe, custom build_script,
  optional-file-404s, connection-failure cleanup.

### Partial file cleanup
- `downloadFile` internally cleans up `.part` files on failure (scope(failure)
  removes temp file). Our extra cleanup layer handles the case where a
  previous step succeeded but a later step fails — though for step 1
  failure (recipe), nothing else was downloaded yet.
- Unittest scenario 5 verifies: after a connection error on recipe download,
  the cache directory is clean (no files).

### DUB details
- `dub.json` uses `warningsAsErrors` in `buildOptions` (generates a
  deprecation warning about `buildRequirements`, but non-blocking).
- All 8 modules pass unittests with warnings-as-errors enabled.
- `dub build` produces the `tofu` binary successfully.

---

## Task 10 — `tofu.binary` (zuur/binary package.lua version checking)

### Architecture
- Module `tofu.binary` depends on: `tofu.types` (BinaryCheckResult, DepConstraint), `tofu.http` (get, HttpException), `tofu.config` (Config, binaryManifestUrl), `tofu.vercmp` (satisfies), `tofu.log` (logDetail, logInfo).
- `checkBinaryVersion(name, constraint, cfg)` fetches the package.lua manifest from zuur/binary via HTTP, parses the version, and checks satisfaction.

### Key logic — 404 vs other errors
- HTTP 404 → binary doesn't exist (recipe-only). Return `BinaryCheckResult(false, "", false)`.
- Any other HttpException (500, timeout, connection failure) → **rethrow** — network problems must propagate, not be silently swallowed.
- Detection: `e.msg.canFind("404")` — the HttpException format is `"HTTP %d fetching <url>"`.

### Version extraction from Lua package.lua
- Light parse: scan for `version = "..."`  in the manifest body.
- Word-boundary check on "version" keyword (preceding char must be whitespace/`{`/`,`/`;`, following char must be whitespace/`=`).
- If version field not found or malformed → `BinaryException`.
- This avoids a full Lua parser — package.lua manifests are flat key-value tables.
- Empty version string (e.g. `version = ""`) also throws BinaryException.

### Memoization
- Module-level `BinaryCheckResult[string] _versionCache` — keyed by package name.
- On cache hit: recompute satisfaction against caller's constraint (same version, different constraint possible).
- On 404: cache the "not found" result so subsequent calls don't re-fetch.
- Lifetime: one command run. Tests use distinct package names to avoid cross-state contamination.
- The cache stores `exists` and `ver`; `satisfies` is always recomputed per-constraint.

### `version` keyword trap (again)
- Local variable `version` conflicts with D keyword. Used `ver` instead. Same pattern as types.d where struct field is `ver`.

### Test infrastructure
- Reused local one-shot TCP server pattern from http.d:
  - `bindAndSpawn(response)` — bind ephemeral port, spawn thread, return URL.
  - `oneShotResponder(listener, response)` — accept one connection, send canned HTTP response, exit.
  - `httpResponse(code, reason, body)` — build minimal HTTP/1.1 response with Content-Length.
  - `testConfig(baseUrl)` — create a Config pointing zuurUrl at the test server.

### Test cases (6/6 pass)
1. Binary exists, version 2.1.0, constraint ge 2.0 → satisfies:true ✓
2. Binary exists, version 1.9, constraint ge 2.0 → satisfies:false ✓
3. 404 → exists:false, ver:"", satisfies:false ✓
4. 500 → HttpException rethrown (not swallowed) ✓
5. Manifest without version field → BinaryException ✓
6. Version 2.1.0 with unconstrained dep (op none) → satisfies:true ✓

### `@safe` annotation consistency
- `checkBinaryVersion` and all helpers marked `@safe`. The function calls `http.get()` (which is `@safe`), `satisfies` (`@safe`), and log functions (`@safe`). No `@trusted` blocks needed in production code.
- Test helpers (`oneShotResponder`, `bindAndSpawn`, `httpResponse`) are `@trusted` since they use `std.socket`.

### Note: pre-existing fetch.d compile issue
- `fetch.d` (from parallel task 7) has a missing `std.algorithm.searching : canFind` import that blocks `dub test`. `dub build` passes (fetch.d is excluded via `.skip` extension renames by the parallel agent). Binary.d tests verified via standalone `dmd -I... -i -main -unittest` compilation — all 7 modules pass.

---

## Task 7 — `tofu.index` (ZUUR index fetch & sandboxed Lua parse)

### Security: whitelist sandbox (NOT blacklist)

The task spec suggested stripping dangerous globals (blacklist: `io=nil, os=nil, ...`). We chose the ZETA `lib/sandbox.lua` whitelist approach instead because:
- **Future-proof**: New dangerous globals added to Lua (e.g. `rawlen` in 5.3) are blocked by default — the index only sees explicitly allowed functions.
- **Proven**: ZETA has used this sandbox in production; it has passed security review.
- **Allowed globals**: `_VERSION`, `assert`, `error`, `ipairs`, `next`, `pairs`, `pcall`, `select`, `tonumber`, `tostring`, `type`, `rawequal`, `rawget`, `rawset`, `setmetatable`, `getmetatable`, `unpack`, `string`, `table`, `math`. NOTHING else.

### Lua 5.1 vs 5.2+ sandbox differences — CRITICAL for future tasks

The sandbox must work on both Lua 5.1/5.2+/5.5.x. The key difference:

| Version | Compile API | Set environment |
|---------|------------|----------------|
| 5.1 / LuaJIT | `loadstring(src, name)` | `setfenv(chunk, env)` |
| 5.2+ (incl. 5.5.1) | `load(src, name, "t", env)` | env is 4th arg to `load` |

Detection: `if setfenv then ... else ... end`. In Lua 5.2+, `setfenv` was removed entirely, so `if setfenv` is `nil` (falsy) on 5.2+ — this is the canonical cross-version detection pattern.

**What NOT to mix up**: `loadfile` is the FILE loader (takes a path). `load`/`loadstring` are STRING loaders. The sandbox loader script uses `io.open` + `f:read("*a")` to read the index file content, then `load` to compile it with the sandbox env. This is deliberately explicit — it's exactly what ZETA's `sandbox.loadfile` does.

**System**: The system has Lua 5.5.1 (`lua -v` → `Lua 5.5.1`). Our sandbox script uses the 5.2+ branch (`load(src, name, "t", env)`). The 5.1 branch is retained for portability.

### `std.process` import conflicts

`import std.process;` (unqualified) pulls in `std.process.Config`, which conflicts with `tofu.config.Config`. Fix: use selective imports — `import std.process : execute, ProcessException, thisProcessID;`. This affected both `index.d` and `fetch.d`.

### `std.json` API notes (Phobos)

- `parseJSON(string)` → `JSONValue` (return type, not `auto`)
- `JSONValue.type` → `JSONType` enum: `JSONType.array`, `JSONType.object` (NOT `object_`)
- `.array` property → `@system` (must wrap in `@trusted`)
- `.str` → string field on object values
- Indexing: `entry["name"]` → `JSONValue`

### D heredoc syntax (`q"DELIM ... DELIM"`)

- Opening: `q"SCRIPT` (no closing quote on same line — rest of line must be blank)
- Closing: `SCRIPT"` at start of a new line
- Then `;` on the same line after `"` to end the statement

### Concurrent agent issues

Both `binary.d` and `fetch.d` were created by parallel agents and had:
- `Config` naming conflicts (unqualified `import std.process;`)
- `version` keyword used as variable name (`binary.d` — already fixed by agent)
- Missing `canFind` import (`fetch.d`)
- Different test server patterns (`spawnProcess` vs `spawn` — fetch.d)

When testing, temporarily exclude broken sibling modules with `mv file.d file.d.skip`.

### JSON escaping in Lua

Must escape `\\`, `\"`, `\n`, `\r`, `\t` in string values before embedding in JSON. Order matters: escape backslash BEFORE quote, or doubled backslashes leak:

```lua
s = s:gsub("\\", "\\\\"):gsub('"', '\\"')
```

### Test patterns ported from http.d

Reused the one-shot TCP responder pattern (`bindAndSpawn`, `oneShotResponder`, `httpResponse`) directly from `tofu.http` test harness. The same spawn+accept+send pattern works for index serving.

### Malicious index verification

The sandbox test (test 2) creates a sentinel file, serves an index containing `os.execute("rm -rf /")`, verifies that `fetchIndex` throws `IndexException`, then asserts the sentinel file STILL EXISTS. This is the security-critical validation that the sandbox actually blocks RCE. The error message from Lua is "attempt to call a nil value (global 'os')" because `os` is absent from the sandbox env — caught by `pcall` and reported as `LUA_ERROR:runtime error: ...`.

---

## Task 11 — `tofu.deps` (topological dependency tree builder)

### Architecture
- Module `tofu.deps` depends on: `tofu.types` (Recipe, DepConstraint, TypesException, DepOp).
- Port of ZETA `lib/deps.lua:19-74` depth-first resolution algorithm.
- `DepNode` stores: `name`, `constraints` (parsed `DepConstraint[]` from `recipe.deps`), `recipePath`.
- `DepTree` is a flat `DepNode[]` in topological order (deps before dependents, target last).

### Algorithm (exact port from ZETA deps.lua)
- `resolveDepTree(targetName, getRecipe)` — accepts a `scope Recipe function(string) @safe` (the testability seam).
- Nested `walk(name, ref chain)` does the depth-first walk:
  1. **Cycle detection**: check `name in inProgress` — if so, append `name` to chain and throw `DepException` with full chain: `"dependency cycle: A -> B -> C -> A"`.
  2. **Memoization**: if `name in done`, return immediately (skip already-resolved).
  3. **Fetch**: call `getRecipe(name)` — exceptions propagate (e.g. missing package).
  4. **Mark**: `inProgress[name] = true`, `chain ~= name`.
  5. **Recurse**: for each dep spec in `recipe.deps`, parse via `DepConstraint.parse(depSpec)` and recurse on `constraint.name`.
  6. **Pop & mark**: `chain = chain[0 .. $ - 1]`, `inProgress.remove(name)`, `done[name] = true`.
  7. **Build node**: create `DepNode` with all deps parsed into `constraints`, append to `order`.

### Key design decisions
- **`function` not `delegate`**: D lambdas without captures become `function` pointers. Using `function` in the parameter type means tests pass without forcing captures. Production callers can pass module-level functions or free functions; if state is needed, use a `static` function that accesses module state.
- **`ref string[] chain`**: The Lua reference implementation uses a mutable shared table for the chain. In D, passing `string[]` by `ref` achieves the same semantics — modifications (append, pop via slice) are visible to the caller across recursive calls.
- **Cycle message**: Build manually with a `for` loop rather than importing `std.array.join` — ensures `@safe` compatibility and avoids Phobos auto-decoding issues.
- **`indexOf`**: Imported `std.string : indexOf` for string containment checks in cycle-message assertions.

### Test cases (7/7 pass)
1. A deps=[B, C>=1.0] — constraint parsing verified (B unconstrained, C ge 1.0); both B,C before A.
2. A deps=[B], B deps=[C] — linear chain → [C, B, A].
3. A deps=[A] — self-cycle → DepException "A -> A".
4. A deps=[B], B deps=[A] — cycle → DepException with both A and B in message.
5. Missing dep (getRecipe throws) — TypesException propagates.
6. Diamond A→B,C, B→D, C→D — D appears once; order [D, B, C, A].
7. Leaf (empty deps) — single node, no constraints.

### Build verified
- `dub build` passes with warnings-as-errors.
- `dub test` passes — all 10 modules, including deps.d's 7 unittests.
- Standalone dmd compilation with `-unittest` also passes.

---

## Task 9 — `tofu.cache` (recipe cache with version-based staleness)

### Architecture
- Module `tofu.cache` depends on: `tofu.types` (CacheManifest), `tofu.config` (Config, load), `tofu.vercmp` (compare), `tofu.log` (logWarn).
- Five public functions: `cacheRecipe`, `isRecipeStale`, `cleanRecipeCache`, `clearBuildCache`, `clearRecipeCacheAll`.
- Cache format: `.tofu-cache.json` in `cfg.recipesCacheDir(name)`, containing `{"name":"<name>","ver":"<indexVersion>","fetchedAt":<unix-ts>}`.

### `@safe` / `@trusted` architecture
- All public functions marked `@safe`.
- Filesystem operations (`exists`, `mkdirRecurse`, `remove`, `rmdirRecurse`, `rename`, `write`, `readText`) and `parseJSON` are isolated in single-line `@trusted` helpers (`fExists`, `fMkdirRecurse`, `fRemove`, `fRmdirRecurse`, `fRename`, `fWrite`, `fReadText`, `fParseJSON`).
- Follows the same pattern as `tofu.config`, `tofu.http`, `tofu.fetch`, `tofu.index`.

### Atomic write (tmp + rename)
- Ported the `.part` → rename pattern from `http.d downloadFileImpl`.
- Manifest written to `.tofu-cache.json.tmp`, existing manifest removed, then `rename(tmp, final)`.
- `rmdirRecurse` from `std.file` used for recursive directory cleanup — available since D 2.104, present on DMD 2.112.

### `Clock.currTime()` not `currTime()`
- In DMD 2.112 / Phobos, `currTime` is a `static` method of `struct Clock`, not a free function.
- Correct import: `import std.datetime : Clock;`, usage: `Clock.currTime().toUnixTime()`.
- Attempting `import std.datetime.systime : currTime;` or `import std.datetime : currTime;` both fail — the symbol is not exported at module level.

### JSON writing: `q"..."` token string gotcha
- D's `q"DELIM ... DELIM"` heredoc syntax requires the opening delimiter line to end with nothing after the delimiter: `q"EOS` followed by newline, content, then `EOS"` on its own line.
- Attempted `q"{"...`}" — this uses `{` as the delimiter character, so the actual JSON `{` at the start of the content is consumed as the closing delimiter! Resulted in content missing the outer braces → invalid JSON.
- Fix: used `q"EOS` (multi-line heredoc) with `{"name":"...` on the content line.
- `std.format.format` is used to interpolate values into the JSON template.

### Staleness: `vercmp.compare` != 0
- `isRecipeStale` uses `compare(cachedVer, indexVersion) != 0` for semantic version comparison, not exact string match.
- RPM-style comparison: leading zeros ignored (`"01.05"` == `"1.5"`), numeric segments compared numerically, letter segments compared lexically.
- Missing cache → true (stale). Corrupted JSON → `logWarn` + true. Read failure → `logWarn` + true.
- `isRecipeStale` never throws — all error paths caught and handled.

### Test strategy
- Pure file ops, no network. Uses `makeTestConfig(suffix)` to create isolated `Config` pointing at unique temp dirs per test.
- `scope(exit) cleanupTestDir(cfg.cacheDir)` ensures temp dirs are removed after each test.
- 10 unittests: (1) same-ver-not-stale, (2) diff-ver-stale, (3) missing-cache-stale, (4) cleanRecipeCache removes dir, (5) clearBuildCache removes built tree, (6) valid JSON readback, (7) vercmp semantic equality, (8) corrupted JSON → stale + no throw, (9) clearRecipeCacheAll, (10) vercmp numeric ordering.
- Pre-existing `deps.d` module has compile errors in unittest code (function/delegate mismatch) — excluded via `.skip` rename for `dub test`. `dub build` passes clean with all modules.

### `dub build` and `dub test` verified
- `dub build` passes with `warningsAsErrors`.
- `dub test` (with deps.d.skip) — 9 modules passed unittests.
- Evidence logged to `.omo/evidence/task-9-tofu-core.log`.

---

## Task 12 — `tofu.resolve` (version-constraint-aware dep resolution)

### Architecture
- Module `tofu.resolve` depends on: `tofu.types` (DepConstraint, DepOp, PackageIndex, Pool, BinaryCheckResult), `tofu.deps` (DepTree, DepNode), `tofu.log` (logInfo), `std.string` (indexOf).
- `constrainDepTree(tree, index, binaryCheck)` annotates a dep tree with source decisions: binary (pre-built zuur binary satisfied) vs recipe (build from source).
- The `binaryCheck` delegate is the testability seam — production wires `tofu.binary.checkBinaryVersion`; tests inject mocks.

### Algorithm — three-pass resolution
1. **Pass 1 (aggregate)**: Walk all nodes, collect all `DepConstraint[]` per dep name into `allConstraints` AA. Same dep constrained by multiple parents → all constraints aggregated.
2. **Pass 2 (resolve)**: Walk nodes in topological order. First time a dep name is encountered, resolve it:
   - Test ALL aggregated constraints via `binaryCheck(depName, constraint)`.
   - If ANY constraint returns `!satisfies` → dep goes recipe (after verifying recipe exists in index).
   - If ALL constraints return `satisfies` → dep goes binary.
   - Root (last node) is always recipe (it's what the user asked to build from ZUUR recipes).
   - Neither binary nor recipe in index → `ResolveException("dependency '<name>' not found in ZUUR (neither binary nor recipe)")`.
3. **Pass 3 (output)**: Build `ConstrainedNode[]` array — one entry per unique node name in tree order, source from resolved map (fallback recipe).

### Deduplication semantics
- Same dep appearing under multiple parents → resolved ONCE. All constraints tested; if ANY fails → recipe. This implements the "strictest wins" rule from the plan.
- Unconstrained dep (`DepOp.none`) with binary → binary (satisfies always true). Without binary → recipe (if in index).

### Log format
- `"binary libfoo-2.1 satisfies libfoo>=2.0"` — binary satisfied
- `"libfoo: binary 1.9 too old, building from recipe"` — binary exists but too old
- `"libfoo: no binary available, building from recipe"` — binary 404/skip

### `delegate` vs `function` in testability seams
- **CRITICAL gotcha**: Non-capturing D lambdas in `@safe unittest` blocks are inferred as `function` pointers with inferred attributes (`pure nothrow @nogc @safe`). A parameter typed as `delegate` cannot accept a `function` even if attributes match.
- **Fix**: Use the explicit `delegate` keyword: `scope binaryCheck = delegate (string name, DepConstraint c) @safe { ... };`. This forces delegate type regardless of captures.
- **Why delegate, not function?** The production caller (install command) must capture `cfg` to call `checkBinaryVersion(name, constraint, cfg)`. A function pointer cannot carry captured state. The `delegate` keyword in tests matches the production usage pattern.
- Contrast with `deps.d` where `resolveDepTree` uses `function` — that works because recipe fetching doesn't need captured state (the production caller uses module-level functions).

### `std.string.indexOf` import
- `indexOf` is NOT in the default namespace. Must explicitly `import std.string : indexOf;` to use on string in assertions. Same pattern as `deps.d`.

### Test cases (8/8 pass)
1. dep libfoo>=2.0, binary 2.1 → binary
2. dep libfoo>=2.0, binary 1.9, recipe in index → recipe
3. dep libfoo>=2.0, binary 1.9, NOT in index → ResolveException ("neither binary nor recipe")
4. unconstrained dep with binary → binary
5. unconstrained dep without binary but in index → recipe
6. multiple constraints on same dep, one unsatisfied → recipe (dedup + strictest wins)
7. root marked recipe (single-node tree)
8. empty tree → empty result

### Build verified
- `dub build` passes with `warningsAsErrors`.
- `dub test` passes — all 11 modules (config, log, types, vercmp, http, index, fetch, cache, binary, deps, resolve) pass unittests.
- No D LSP server configured for `.d` files — diagnostics verified via compiler.
- Evidence logged to `.omo/evidence/task-12-tofu-core.log`.

---

## Task 16 — `tofu.install` (invoke ZETA -LocalProvide)

### Architecture
- Module `tofu.install` depends on: `tofu.config` (Config, builtPackagesDir, zuurUrl, zetaPath), `tofu.log` (logInfo).
- `runLocalProvide(pkgName, cfg)` invokes `zeta -LocalProvide <pkgName> --pass` with per-child environment.
- `InstallException : Exception` for install failures and missing zeta binary.

### Process spawning — `pipeProcess` not `spawnProcess`
- Used `std.process.pipeProcess` (not `spawnProcess`) because it returns `ProcessPipes` with piped stdout.
- Flags: `Redirect.stdout | Redirect.stderrToStdout` — pipes stdout and merges stderr into it.
- This avoids needing a separate thread for stderr capture (unlike build.d's approach).
- **Key difference from build.d**: `pipeProcess` + `Redirect.stderrToStdout` merges stderr into the stdout pipe — only one stream to read. build.d uses `Redirect.stderr` and a reader thread for separate stderr.

### Per-child environment via `pipeProcess` env parameter
- `pipeProcess(args, redirectFlags, env)` accepts `const string[string] env` — the child gets these env vars on top of the parent's environment.
- Simpler than the set-restore pattern on `std.process.environment` (no mutation of parent env, no race conditions).
- Set: `ZETA_LOCAL_PACKAGES` = `cfg.builtPackagesDir()`, `ZETA_REPO` = `cfg.zuurUrl ~ "/binary"`.
- `ZETA_ROOT` intentionally not set — config has no such field.

### Real-time output tee pattern
- Read from `pipes.stdout.byLine` (returns `char[]` with `\n` terminator by default).
- Each line: `.idup` to `string`, `write(s)` to parent stdout, `stdout.flush()` for real-time display.
- Rolling buffer: append to `string[]`, trim to last 20 lines (`rollingBuffer[1..$]`).
- On non-zero exit, concatenate rolling buffer into error message.

### `stderrToStdout` + ProcessPipes caveats
- When `Redirect.stderrToStdout` is used, `pipes.stderr` is **not** piped — accessing it throws `object.Error`.
- Similarly, `pipes.stdin` is not piped when not requested — accessing it throws.
- Must NOT attempt to close `pipes.stdin` or `pipes.stderr` when they weren't redirected.
- ZETA with `--pass` is non-interactive so the inherited stdin doesn't block.

### "already installed" handling
- ZETA `actions.localprovide` (lines 174–178): if `db.is_installed(name)`, prints "already installed -- use -ReProvide" and exits 0.
- Exit 0 + "already installed" in output → logInfo("already installed — skipping"), return normally.
- This is NOT an error — just a note.

### Unittests — fake zeta shell scripts
- Created per-test temp directories with `mkdirRecurse`, clean up with `scope(exit) rmdirRecurse`.
- Each test writes a bash script to `tmp/fake-zeta`, makes it executable (`chmod +x`), and points `cfg.zetaPath` at it.
- 5 test scenarios:
  1. Exit 0 + env dump → returns, no throw (env file written by script, verified with readText)
  2. Exit 1 + stderr → InstallException with last output lines
  3. "already installed" + exit 0 → no throw, logInfo logged
  4. Nonexistent binary path → InstallException "zeta not found"
  5. Env correctness → stdout capture via File-swap, assert ZETA_LOCAL_PACKAGES and ZETA_REPO values

### D heredoc gotcha inside test scripts
- Cannot concatenate D strings inside `q"SCRIPT ... SCRIPT"` heredocs — the content is literal.
- Fix: use `__PLACEHOLDER__` and `std.string.replace` to inject dynamic paths into the script content before writing.

### `File.byLine` + terminator behavior
- `byLine` keeps `\n` terminator by default (`Yes.keepTerminator`).
- Forward with `write(s)` (no extra newline needed); the captured `\n` provides the line break.
- When building error message, concatenate directly (lines already end with `\n`).

### `Config` name conflict avoidance
- `std.process.Config` conflicts with `tofu.config.Config`.
- Selective imports: `import std.process : pipeProcess, ProcessPipes, Redirect, wait, ProcessException;` — no `Config` import needed.
- `pipeProcess`'s `config` parameter has a default value (`Config.none`), so explicit `Config` reference is unnecessary.

### Unused `if` block cleanup gotcha
- Empty `if` block (`if (x) { }`) triggers "statement has no effect" warnings → errors with `warningsAsErrors`.
- Remove completely rather than leaving empty.

### Build verified
- `dub test` passes — all 13 modules (config, log, types, vercmp, http, index, fetch, cache, binary, deps, resolve, build, install).
- `dub build` passes with `warningsAsErrors`.
- Evidence logged to `.omo/evidence/task-16-tofu-core.log`.

---

## Task 13 — `tofu.resolve.generateBuildPlan` (build plan from constrained tree)

### Architecture
- New function `generateBuildPlan(constrained, tree, cfg, fetchRecipe = null)` in `tofu.resolve`.
- Input: `ConstrainedNode[]` from `constrainDepTree` (already in topological order, deps-first, root last). The `DepTree` parameter is preserved for future context but not used for logic — ordering comes from the constrained array.
- Output: `BuildPlan` with `Source.recipe` entries for every recipe-sourced node (binary nodes excluded).
- The `fetchRecipe` delegate (`scope string delegate(string) @safe`) is the testability seam. Default `null` means "no fetcher available" — missing recipes throw `FetchException`.
- Production wires: `delegate (string name) @safe { return tofu.fetch.fetchRecipe(name, cfg); }`.

### Algorithm
1. Iterate constrained nodes in order (preserves topological ordering).
2. Skip nodes with `source == DepSource.binary` — Zeta handles binary deps.
3. For recipe nodes: compute cache path via `cfg.recipesCacheDir(name) ~ "/" ~ name ~ ".recipe"`.
4. Check `std.file.exists(path)` via `@trusted` wrapper.
5. Missing → if `fetchRecipe` delegate provided, call it; else throw `FetchException("recipe not cached and no fetcher provided")`.
6. Add to `BuildPlan` via `plan.add(name, recipePath, Source.recipe)`.
7. Empty plan → log `"nothing to build (all binary)"`.
8. Non-empty → log `"build plan: N packages"` via `logOk`.

### `@safe` / `@trusted` architecture
- `generateBuildPlan` is `@safe`.
- Only `std.file.exists` requires `@trusted` wrapper — inline lambda `() @trusted { return exists(path); }()`.
- Follows existing patterns from `fetch.d`, `cache.d`, `index.d`.

### FetchException reuse
- Uses `tofu.fetch.FetchException` (imported via `import tofu.fetch;`). No new exception class needed — the message `"recipe not cached and no fetcher provided"` is distinct and searchable.

### New imports in resolve.d
- `import tofu.config;` — for `Config` type (parameter in function signature).
- `import tofu.fetch;` — for `FetchException`.
- `import std.file : exists;` — for recipe cache existence check.
- `import tofu.log;` already had `logInfo`; now also uses `logStep`, `logOk`.

### Test strategy (6 new unittests, numbered 9–14)
- Reused the temp-dir + `scope(exit)` cleanup pattern from other modules.
- `version(unittest)` block with `@trusted` helpers: `createCachedRecipe(dir, name)`, `testTempDir(suffix)`, `testRmdir(path)`, `testConfig(cacheDir)`.
- Test (9): [C(recipe), B(binary), A(recipe)] → plan [C, A] — B excluded, order preserved.
- Test (10): missing cache + fetch delegate → fetch called, returned path used in plan.
- Test (11): missing cache + no fetch delegate → FetchException thrown.
- Test (12): all binary → empty plan.
- Test (13): root included even when dep is binary.
- Test (14): recipe files exist in cache → fetch delegate NOT called (delegate throws assert on invocation).

### `scope` on constrained parameter
- `scope const ConstrainedNode[] constrained` — DMD 2.112 requires `scope` on array/class reference parameters for `@safe` inference when the function does not escape them. Same pattern as `constrainDepTree` (which uses `scope const PackageIndex[] index`).

### Build verified
- `dub test` passes — all 11 modules, including 6 new generateBuildPlan unittests.
- `dub build` passes with `warningsAsErrors`.
- Pre-existing breakage in `build.d` (parallel task 15 artefact) excluded via `.skip` rename for testing — NOT caused by task 13.
- Evidence logged to `.omo/evidence/task-13-tofu-core.log`.