[Santiagocetran]

This follows up on #213 by making isolated integration failures easier to investigate.

When a command fails or times out, we keep its output outside the temporary test directory. We also collect a small, filtered summary of worker results and record whether cleanup completed, so useful evidence remains after the temporary files are removed.

The changes include:

• Size-limited logs for commands, builds and local services.
• Worker summaries that exclude inputs, wallet/config files and full success payloads.
• Cleanup that preserves the original failure when cleanup also encounters a problem.
• Tests for timeouts, interruptions, output limits, unsafe file paths and diagnostic write failures.
• An updated testing guide explaining the behavior, artifact paths and limits.

The diagnostics work with the existing manual Full GI workflow in .github/workflows/full-gi.yml, which uploads the results directory on failure. The worker summary is retained at worker-reports/summary.json.

Validation: On head 1b235716, CI passed Python, Docs, Solidity and CI OK. The Python 3.12 run passed 699 tests, with 133 integration tests deselected, using the CI dependency constraints. See CI run.

The advisory Ruff check reported 743 findings across the repository; it remains non-blocking. Earlier validation on the original implementation passed 561 tests, with 133 integration tests deselected, on Python 3.12 and 3.13. The latest CI run does not revalidate Python 3.13. No live Anvil, IPFS or Docker integration was run for this change.

The PR is ready for review. Wallet migration, lifecycle repairs, full-GI success assertions and Docker ownership remain separate follow-up work.

Related to #228; does not close it.

[robertocarlous]

Reviewed against develop in an isolated worktree (~/tempdir/DIN/PRs/PRreview_241, branch pr-241-review, 1 commit, no conflicts — confirmed by both git merge-tree --write-tree locally). Merge base 6e361cb0; branch is 8 commits behind current develop tip (eb8085da), all on Developer/MAINTAINING.md, CODEOWNERS, and dincli/cli/ — no overlap with any file in this PR.

Note: this is a draft PR. The PR body says "This is a draft for review and coordination around the shared harness and testing guide." What follows is a read-level review of the code as written; the draft status means it is not ready to merge as-is — the author should mark it ready and update the CI base before it goes through the merge gate.

---

Claimed: When a command fails or times out in isolated mode, output is now retained in a bounded log outside the temporary directory so it survives cleanup.

Verified — retention path and budget accounting are correct: _Capture.feed() tracks both a per-stream _size counter and a shared owner._retained counter. available is computed as min(stream_limit - _size, total_limit - retained), ensuring neither per-stream nor total budget is exceeded. On budget exhaustion DiagnosticsLimitError is raised, setting truncated = True before the exception propagates. The _secondary(primary, error) function adds a note to the primary exception rather than replacing it, keeping interruption/command failures dominant — the pattern is correct for SIGINT/KeyboardInterrupt safety.

Claimed: Command arguments, environments and stdin are never recorded.

Verified — API surface is correct: Diagnostics.run() accepts the command list and env but routes only stdout/stderr to the _Capture instances; the manifest entry written by record() only stores caller-supplied keys. reports.py's _JOBS patterns match only expected job-output directories; wallet paths, salts and success payloads are explicitly excluded. The _open_directory helper walks path components using os.O_NOFOLLOW directory descriptors, never following a symlink at any component — correct defence against a symlink-swap in the temporary directory.

Claimed: _atomic_json writes are atomic (link + unlink pattern).

Verified — implementation is correct: os.open with O_EXCL | O_NOFOLLOW creates the temp file without following a symlink; os.link is called with follow_symlinks=False, which is atomic and refuses a preexisting target including a symlink. The finally block calls os.unlink on the temp name regardless of whether link succeeded, preventing temp-file accumulation.

Claimed: Tests cover timeouts, interruptions, output limits, unsafe file paths and diagnostic write failures. PR body says 561 tests pass.

Verified — CI Python (1m21s) green at https://github.com/InfiniteZeroFoundation/DevNet/actions/runs/37822992024/job/113468526909. The new unit test file test_integration_diagnostics.py is excluded from the not integration deselect (it imports from tests.dincli.diagnostics directly, not the integration harness fixtures), so it runs in the normal pytest pass. test_integration_reports.py and test_integration_services.py follow the same pattern. CI Python confirms all pass.

Claimed: The advisory Ruff check reports 708 findings vs. an estimated 664 before this change (intentional interruption-safe catches and style warnings).

Not independently verified: Ruff is advisory, not a required CI gate. No action needed.

---

All substantial claims held up. The draft flag means the author intends further work before merge. The code as written is correct and the safety patterns are sound. The branch needs a rebase (or merge from develop) before marking ready, since CI ran 8 commits ago on an older base.

[robertocarlous]

Files changed (8) — as of PR head

Diffed against merge-base 6e361cb0. Current develop tip is eb8085da — the branch is 8 commits behind, all on unrelated files (MAINTAINING.md, CODEOWNERS, dincli/cli/). GitHub reports mergeable: UNKNOWN / mergeStateStatus: UNKNOWN — this PR is a draft (isDraft: true). Merge-tree dry run locally is clean.

Note: draft PR. The author should mark it ready and rebase before it goes through the merge gate.

---

tests/dincli/diagnostics.py

Field	Value
Change	Added
Lines	+639/-0
Diff	Diagnostics class (run, record, context manager), _Capture (per-stream bounded buffer), ServiceStream (async-friendly wrapper). Budget accounting via _size + shared _retained counters. _secondary(primary, error) interrupt-safe exception-note pattern.
Functionality — how & why	How: _Capture.feed() tracks per-stream and total budgets simultaneously; DiagnosticsLimitError is raised on exhaustion with truncated=True. _secondary adds to a primary exception rather than replacing it, preserving SIGINT dominance. Why: Retained diagnostics must survive the temp-directory cleanup that otherwise removes all command output.
Diff vs current develop HEAD	None (new file)
Recommended merge proposal	Merged as-is once draft flag is lifted and branch is rebased
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/dincli/reports.py

Field	Value
Change	Added
Lines	+290/-0
Diff	collect_worker_reports, _atomic_json (O_EXCL + link + unlink pattern), _open_directory (O_NOFOLLOW directory-fd walking). _JOBS patterns exclude wallet paths, salts, and success payloads.
Functionality — how & why	How: _atomic_json opens a temp file with `O_EXCL
Diff vs current develop HEAD	None (new file)
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/dincli/conftest.py

Field	Value
Change	Modified
Lines	+119/-40
Diff	Adds _get_diagnostics() singleton fixture, integrates Diagnostics context manager into the shared harness setup. Existing fixtures updated to pass the diagnostics instance through.
Functionality — how & why	Centralizes diagnostics injection so all integration tests share the same bounded-retention context without per-test boilerplate.
Diff vs current develop HEAD	None (no overlap with develop-only commits)
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/test_integration_diagnostics.py

Field	Value
Change	Added
Lines	+578/-0
Diff	Unit tests for Diagnostics, _Capture, _secondary. Covers timeouts, interruptions, output-limit exhaustion, truncation flag, unsafe file paths, diagnostic write failures. Imports from tests.dincli.diagnostics directly (not the integration harness fixtures), so it runs in the normal pytest pass.
Diff vs current develop HEAD	None (new file)
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/test_integration_reports.py

Field	Value
Change	Added
Lines	+283/-0
Diff	Tests for collect_worker_reports, _atomic_json, _open_directory. Covers unsafe path rejection, concurrent write safety, symlink-swap defence.
Diff vs current develop HEAD	None (new file)
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/test_integration_services.py

Field	Value
Change	Added
Lines	+114/-0
Diff	Integration-harness-level smoke tests for service startup and graceful stop under the new diagnostics context.
Diff vs current develop HEAD	None (new file)
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

tests/test_integration_harness.py

Field	Value
Change	Modified
Lines	+69/-1
Diff	Wires diagnostics context into the harness fixture; adds assertions that diagnostics are captured on command failure.
Diff vs current develop HEAD	None
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

Documentation/technical/testing/dincli-testing-guide.md

Field	Value
Change	Modified
Lines	+46/-2
Diff	New §: "Retained diagnostics" explaining the bounded-log pattern, budget constants, and how to access captured output in test failures.
Diff vs current develop HEAD	None
Recommended merge proposal	Merged as-is once ready
Actual merge proposal	Soon
Pending proposal	Draft flag; rebase needed
Local merge conflict	No
GitHub merge conflict	Unknown (draft)

---

Verification

CI Python (1m21s, 561 passed 0 failed) green on PR head at https://github.com/InfiniteZeroFoundation/DevNet/actions/runs/37822992024/job/113468526909. No foundry/src changes → one peer approval needed, no second sign-off required. Local merge-tree dry run clean.

Draft PR — merge gate blocked until the author marks it ready and rebases onto current develop (8 commits behind).

Local vs. GitHub: local clean; GitHub shows UNKNOWN (expected for draft PRs).

[robertocarlous]

Bounded-log retention, atomic JSON, interrupt-safe _secondary() pattern, and O_NOFOLLOW directory-fd walking all verified correct. 561 tests pass (CI Python green). Draft PR — merge gate stays blocked until the author marks it ready and rebases (currently 8 commits behind develop). No foundry/src changes, so one peer approval is sufficient.

[Santiagocetran]

Thanks for the review! Undrafted the PR and updated the branch with the latest develop.