Harden diagnostic build finalization

[Soengkit]

Summary

Closes #2 by hardening diagnostic build finalization and follow-up review findings so failed or late diagnostic steps preserve useful encrypted artifacts instead of overwriting metadata with empty values.

Changes

• Preserves generated diagnostic logd artifacts when late metadata or finalization errors occur.
• Cleans stale commit-stable diagnostic chunks before repacking a new bundle.
• Fixes OpenAPI Haskell stubs so optional JSON nulls parse as missing fields and HashMap values encode as objects.
• Uses uintptr range math that avoids cross-object pointer comparisons and end-address overflow in arena_contains.
• Keeps public diagnostic metadata free of local filesystem paths and diagnostic unlock material.

Testing

• python3 -m py_compile build.py
• (cd docs/openapi && ghc -fno-code Types.hs Server.hs Validate.hs Generate.hs)
• make -C frailbox
• python3 build.py -> 10/10 modules passed

Latest diagnostic artifacts:

• diagnostic/build-3a68b6a3.json
• diagnostic/build-3a68b6a3.logd

Checklist

• Relevant modules affected by these changes build locally
• Tests pass locally
• Diagnostic build log is committed in this PR
• Changes are scoped to the PR purpose and avoid unrelated cleanup
• Security, privacy, and error-handling implications have been considered

---

• I would like to request that my diagnostic build log is removed before merging

[coderabbitai]

📝 Walkthrough

Walkthrough

build.py gains new constants, format_diagnostic_exception(), record_diagnostic_failure(), and defensive try/except wrappers in commit_diagnostic_artifacts() and generate_logd() to route all diagnostic finalization errors into a structured JSON report. Five Haskell stub modules are added under docs/openapi/ to satisfy the OpenAPI docs build. frailbox receives OS-conditional linker flags and #ifdef guards for MAP_HUGETLB and prctl.

Changes

build.py Diagnostic Error Handling

Layer / File(s)	Summary
New constants and error-formatting helpers build.py, tests/test_build_logging.py	Adds DIAGNOSTIC_FAILURE_MESSAGE and chunk-size constants, format_diagnostic_exception(), record_diagnostic_failure(), and an active non-zero exit check in check_encryptly_runs(); tests verify both new helpers with mock patches.
commit_diagnostic_artifacts defensive wrappers build.py	Wraps all three git subprocess calls (status, add, commit) in try/except and routes failures through format_diagnostic_exception().
generate_logd failure-path rewiring build.py	Guards JSON metadata writing with try/except; converts encryptly unavailability, pack timeout, generic pack exception, .logd split error, and diagnostic report error into record_diagnostic_failure() early returns; adds a final top-level exception handler.
Generated diagnostic metadata artifacts diagnostic/build-811c1d21.json, diagnostic/build-cb24616a.json	Adds commit-stable JSON reports for commits 811c1d2 and cb24616 with overall pass/fail totals, per-module build results, and a pr_note for reviewers.

OpenAPI Haskell Stub Modules

Layer / File(s)	Summary
Data.Aeson.Key, KeyMap, and Types foundations docs/openapi/Data/Aeson/Key.hs, docs/openapi/Data/Aeson/KeyMap.hs, docs/openapi/Data/Aeson/Types.hs	Introduces Key as a Text alias, KeyMap as a strict Map Key v wrapper, and Parser as Either String with parseMaybe.
Data.Aeson value type, typeclasses, helpers, and instances docs/openapi/Data/Aeson.hs	Defines the Value ADT, FromJSON/ToJSON typeclasses with stub defaults, stubbed encode/decode, and the object, withObject, (.=), (.:?), (.!=) helpers with instance declarations.
Data.HashMap.Strict, Data.Yaml, and System.Random stubs docs/openapi/Data/HashMap/Strict.hs, docs/openapi/Data/Yaml.hs, docs/openapi/System/Random.hs	Adds HashMap as a newtype over Data.Map.Strict, a Data.Yaml stub whose decodeFileEither always returns a Left error, and a System.Random stub returning the lower bound of its range.

frailbox Cross-Platform Portability

Layer / File(s)	Summary
Makefile LDFLAGS, arena MAP_HUGETLB guard, sandbox prctl guard frailbox/Makefile, frailbox/src/arena.c, frailbox/src/sandbox.c	Makes LDFLAGS OS-dependent via uname, guards MAP_HUGETLB with #ifdef, adds <stdint.h> and fixes arena_contains pointer comparison to use uintptr_t, and wraps prctl/PR_SET_NO_NEW_PRIVS in #ifdef __linux__.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 Hoppity-hop through the error-strewn lane,
I wrapped every failure so none would escape plain.
A JSON report blooms where exceptions once fell,
Haskell stubs whisper a passable spell.
On Linux or Darwin, the flags now align—
This bunny commits diagnostics just fine! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Harden diagnostic build finalization' clearly and concisely summarizes the main change: enhanced error handling for diagnostic finalization in the build process.
Linked Issues check	✅ Passed	The code changes fully address issue #2 requirements: error handling is implemented for encryptly operations and git commits [#2], graceful failure modes preserve diagnostic metadata via record_diagnostic_failure(), structured error reporting follows conventions, and unit tests [test_build_logging.py] provide coverage [#2].
Out of Scope Changes check	✅ Passed	All changes are scoped to the PR purpose: build.py error handling, OpenAPI Haskell stubs enabling macOS compatibility, frailbox platform guards, and supporting test coverage. No unrelated cleanup or scope creep detected.
Description check	✅ Passed	PR description covers summary, detailed changes, testing steps with commands and results, and completes all checklist items with explanations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/openapi/Data/Aeson.hs`:
- Around line 56-58: The (.=) operator in the Data.Aeson module is ignoring the
field value parameter (using _ pattern) and always returning Null, which
corrupts all JSON object construction. Fix this by adding a ToJSON constraint to
the type signature of (.=), then modify the implementation to pattern match on
the actual value parameter instead of discarding it with _, and call toJSON on
that value to properly convert it to a JSON representation.

In `@frailbox/src/arena.c`:
- Around line 179-183: Replace the relational pointer comparisons in the
condition checking whether needle falls within a memory region with uintptr_t
address-based comparisons. Convert needle (which is assigned from ptr) and start
(which is assigned from region->start) to uintptr_t type, then perform the
comparison operations (needle >= start and needle < start + region->size) using
these uintptr_t values instead of pointer comparisons to avoid undefined
behavior when comparing pointers from different memory regions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 886d1214-8cba-4355-879f-7dc63467fdfa

📥 Commits

Reviewing files that changed from the base of the PR and between e5de93b and 067e090.

📒 Files selected for processing (14)

• build.py
• diagnostic/build-cb24616a.json
• diagnostic/build-cb24616a.logd
• docs/openapi/Data/Aeson.hs
• docs/openapi/Data/Aeson/Key.hs
• docs/openapi/Data/Aeson/KeyMap.hs
• docs/openapi/Data/Aeson/Types.hs
• docs/openapi/Data/HashMap/Strict.hs
• docs/openapi/Data/Yaml.hs
• docs/openapi/System/Random.hs
• frailbox/Makefile
• frailbox/src/arena.c
• frailbox/src/sandbox.c
• tests/test_build_logging.py

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

build.py (1)

826-879: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clean stale commit-stable chunks before creating a new diagnostic bundle.

Because artifact names are commit-stable, a rerun for the same commit can leave old build-<commit>-partNNN.logd files behind. If the new bundle has fewer chunks or no chunks, those stale files are not included in the new metadata and their deletions are not staged.

🧹 Suggested fix

+        for stale_path in [logd_path, *DIAGNOSTIC_DIR.glob(f"{logd_path.stem}-part*.logd")]:
+            if stale_path.exists():
+                stale_path.unlink()
+
         try:
             sr = subprocess.run(
                 [
                     str(encryptly_bin),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build.py` around lines 826 - 879, Before executing the encryptly pack
subprocess.run command, clean up any stale commit-stable chunk files that match
the pattern build-<commit_id>-partNNN.logd in the workspace. This prevents old
partial files from persisting when a rerun occurs for the same commit with
potentially fewer or different chunks. Locate and delete these stale files using
Path.glob() to find matches and unlink() to remove them before the
subprocess.run call that packs the logd.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@build.py`:
- Around line 661-690: The issue is that when record_diagnostic_failure() is
called from an outer exception handler, it lacks the logd_relpaths, password,
and logd_files parameters that were populated during successful processing,
causing metadata to be overwritten with null values and losing the encryption
details. Initialize the variables safe_pw, logd_files, and logd_relpaths before
the outer try block (not inside it) so they remain in scope in the exception
handler, then pass these populated values to record_diagnostic_failure() when
calling it from the outer except block to preserve the already-created .logd
artifacts.

In `@docs/openapi/Data/Aeson.hs`:
- Line 113: The `ToJSON (HM.HashMap k a)` instance on line 113 lacks an
implementation and defaults to the class default which returns Null, causing all
HashMap fields to be silently dropped during JSON encoding. Modify the instance
declaration to restrict the key type to `Text` (matching the usage patterns in
the codebase) and add a `where` clause that implements the `toJSON` method to
properly serialize the HashMap as a JSON object instead of relying on the
default Null implementation.
- Around line 59-64: The `(.:?)` operator does not handle explicit `Null` values
in JSON objects as missing fields. Currently, when a key exists with a `Null`
value, it attempts to parse the `Null` via `parseJSON`, which fails. Modify the
implementation to check if the looked-up value is a JSON `Null` before
attempting to parse it. If the value is `Null`, return `pure Nothing` (same as
when the key doesn't exist). Only proceed with `parseJSON` when the value is
something other than `Null`. This ensures that optional fields with explicit
`null` values are treated as missing and can fall back to defaults like `A..!=
HM.empty`.

---

Outside diff comments:
In `@build.py`:
- Around line 826-879: Before executing the encryptly pack subprocess.run
command, clean up any stale commit-stable chunk files that match the pattern
build-<commit_id>-partNNN.logd in the workspace. This prevents old partial files
from persisting when a rerun occurs for the same commit with potentially fewer
or different chunks. Locate and delete these stale files using Path.glob() to
find matches and unlink() to remove them before the subprocess.run call that
packs the logd.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b8e7b326-095d-403e-a117-c3b66bb85b62

📥 Commits

Reviewing files that changed from the base of the PR and between 067e090 and 1c1d624.

📒 Files selected for processing (16)

• build.py
• diagnostic/build-811c1d21.json
• diagnostic/build-811c1d21.logd
• diagnostic/build-cb24616a.json
• diagnostic/build-cb24616a.logd
• docs/openapi/Data/Aeson.hs
• docs/openapi/Data/Aeson/Key.hs
• docs/openapi/Data/Aeson/KeyMap.hs
• docs/openapi/Data/Aeson/Types.hs
• docs/openapi/Data/HashMap/Strict.hs
• docs/openapi/Data/Yaml.hs
• docs/openapi/System/Random.hs
• frailbox/Makefile
• frailbox/src/arena.c
• frailbox/src/sandbox.c
• tests/test_build_logging.py

✅ Files skipped from review due to trivial changes (2)

• docs/openapi/Data/Aeson/KeyMap.hs
• diagnostic/build-811c1d21.json

🚧 Files skipped from review as they are similar to previous changes (9)

• docs/openapi/Data/Aeson/Key.hs
• docs/openapi/Data/Yaml.hs
• docs/openapi/Data/Aeson/Types.hs
• frailbox/src/sandbox.c
• frailbox/Makefile
• docs/openapi/System/Random.hs
• docs/openapi/Data/HashMap/Strict.hs
• diagnostic/build-cb24616a.json
• tests/test_build_logging.py