[Santiagocetran]

PR #301 moved Full GI to a manual-only workflow. This PR adds a summary that makes those runs easier to investigate by reporting setup completion, pytest results, log capture, runtime and artifact availability separately.

Changes

• Add a small follow-up summary job to full-gi.yml, including reporting after integration failures.
• Capture pytest and log-writing exit codes separately, preserving the original test failure.
• Attempt a bounded upload of available logs after failure or cancellation.
• Report missing evidence as incomplete, and link artifacts only when their availability is confirmed.
• Add focused tests and update the contributor and testing documentation.

Full GI remains manual-only and outside CI OK. Integration, logging, upload and reporting failures retain their real failure status. ci.yml is unchanged from the current base, preserving #308’s merge-queue support.

This replaces the original automatic input-detection and CI dispatch proposal, which was superseded by #301.

Reporting and uploads are best effort: cancellation, timeouts or runner loss can prevent collection. This PR does not repair the Full GI suite or add #241’s harness diagnostics.

Validation

Head: 785f452fe60d86c34f1838b33ad3f5f4603d6af2. Its file tree is identical to the locally tested revision, 54f1d43; the merge preserves the existing PR history and incorporates current develop.

• 42 focused reporting, shell failure-propagation and workflow tests passed.
• Full non-integration suite: 648 passed, 133 integration tests deselected.
• Workflow lint, Python lint/formatting, documentation links and whitespace checks passed.
• Hosted execution of the revised Full GI workflow remains pending.

Related to #270 and #228; closes neither issue.

[Santiagocetran]

I’ve revised this PR to align with #301’s move to manual-only Full GI testing.

The automatic path detection and CI dispatch changes have been removed from the proposed scope. The remaining work improves reporting in full-gi.yml: setup completion, pytest results, log-writing failures and artifact availability are shown separately, with a bounded log-upload attempt after failure or cancellation.

Full GI stays manual-only and outside CI OK. The update also preserves #308’s merge-queue support.

Local validation passed: 648 non-integration tests, including 42 focused tests for this change, plus workflow lint and documentation checks. Hosted Full GI workflow validation is still pending.

The description now reflects this narrower scope and replaces the outdated draft and default-branch rollout notes.

Updated head: 785f452. The branch incorporates current develop and preserves the original PR history.

[umeradl]

Reviewed in an isolated worktree at head 785f452f. It merges cleanly onto current develop 74a845c9 (the merge-base is develop's tip, 0 commits behind). A local git merge-tree shows no conflicts and GitHub agrees (mergeable: MERGEABLE; BLOCKED only while it waits for approval). CI on 785f452f: CI OK green. It covers the 42 new tests (the Python job ran 648 passed, 133 deselected) and the doc links. The Full GI workflow itself is manual-only, so CI doesn't run it.

Claimed: ci.yml is unchanged, so PR No. 308's merge-queue support is kept.

Verified — holds: the PR touches 5 files and ci.yml isn't one of them. The automatic trigger/dispatch scope from the first revision is gone from the final diff.

Claimed: pytest and log-writing exit codes are captured separately, and the original test failure is preserved.

Verified — holds, and the tests catch a regression: the suite step now reads PIPESTATUS under set +e, writes pytest_exit/log_exit/elapsed_seconds to GITHUB_OUTPUT, and exits with pytest's code first, then tee's, then the output-write code. The tests pull the real step script out of full-gi.yml and run it under bash. As a break test, I replaced if [ "$pytest_exit" -ne 0 ]; then exit "$pytest_exit"; fi with true: 5 failed, 37 passed. After restoring it: 42 passed (pytest tests/test_full_gi_summary.py, 0.44 s).

Claimed: a follow-up summary job reports setup, pytest, log capture, runtime and artifact separately, including after integration failures.

Verified — holds (by reading the code and running the unit tests): integration-summary has needs: integration and if: always(). It passes toJSON(needs) to .github/scripts/full_gi_summary.py through an env var, not by interpolating it into the script, so it can't inject commands. Missing or malformed outputs come out as "unknown", never as success (number() accepts digits only). The artifact link is shown only if it matches the canonical https://github.com/<owner>/<repo>/actions/runs/<id>/artifacts/<id> form, so a stray output can't inject Markdown (test_unexpected_artifact_output_is_not_rendered_as_markdown). The workflow still has permissions: contents: read and workflow_dispatch only.

Claimed: there's a bounded attempt to upload logs after failure or cancellation.

Verified — holds as written: the upload step is if: failure() || cancelled() with timeout-minutes: 3.

Not independently re-verified: a hosted Full GI run, as the PR also says. Outputs flowing from the job into needs, and the summary job appearing after a failure, can only be shown by a real dispatch. The suite is currently red on develop (task_061026_21 Part D), so the first manual run after merge will exercise the failure path. That makes it a good check of this PR.

---

Non-blocking observations:

1. The 85-minute suite timeout no longer leaves room for the upload. Before, the comment said the step timeout sits below the job's so a hung suite still gets its logs uploaded. The PR correctly drops that claim and documents it as a limit, because setup also uses the 90-minute budget. If a hung suite does hit the job timeout, the logs we most want are the ones least likely to be uploaded. A follow-up could lower the suite step timeout (or raise the job's) so the 3-minute upload always fits. Not required for this PR.
2. set -o pipefail in the local reproduction snippet (dincli-testing-guide.md) stays on in the reader's interactive shell after they paste it. That's harmless, but a ( … ) subshell or a short note would keep it contained. Trivial.

Every checkable claim held up. No closing keywords (the body says "Related to … closes neither issue"; the commit messages have none). Mergeable as-is.

[umeradl]

Files changed (5) — as of 785f452f (PR head)

Diffed against merge-base 74a845c9, which is the current develop tip, so none of these files changed on develop since the branch was updated. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (waiting on approval). A local git merge-tree --write-tree dry run is clean.

.github/workflows/full-gi.yml

Field	Value
Change	Modified
Lines	+52/-8
Diff (what exactly is in this PR)	Adds job outputs (setup/suite/upload outcomes, pytest/log exit codes, elapsed time, artifact URL), an Integration setup complete marker step (id: ready), and id: suite with a PIPESTATUS-based exit capture. The upload step becomes failure() || cancelled() with a 3-min timeout. Adds a new integration-summary job. The suite's own GITHUB_STEP_SUMMARY line is removed (the summary job replaces it).
Functionality — how & why	How: the suite step runs pytest piped into tee under set +e, records both pipeline exit codes and the elapsed time in GITHUB_OUTPUT, then exits with pytest's code, else tee's, else the output-write code. The marker step only succeeds once setup has finished, which lets the summary tell "setup failed" apart from "suite failed". integration-summary (needs: integration, if: always()) checks out the repo and runs the summary script with toJSON(needs) passed in through an env var. Why: before this, a failed manual Full GI run reported only one exit line, and a log-capture failure or a cancelled run couldn't be told apart from a test failure.
Diff vs current develop HEAD	None (develop is the merge-base).
Recommended merge proposal	Merged as-is. The exit-code logic is break-tested (5 tests fail when pytest's exit is dropped). Observation No. 1 (the step timeout no longer leaves room for the upload) is a possible follow-up.
Actual merge proposal	Merged as-is through the merge queue in 8f291c11.
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

.github/scripts/full_gi_summary.py + tests/test_full_gi_summary.py

Field	Value
Change	New (both)
Lines	+117/-0 · +281/-0
Diff (what exactly is in this PR)	A summary renderer: summary(needs) maps the integration job's result and outputs to a 6-row Markdown table, and main() appends it to GITHUB_STEP_SUMMARY. The tests cover each outcome mapping, malformed or missing outputs, the artifact-URL allowlist, the real step script run under bash (both pipeline exits, plus an output-write failure that must not hide a pytest failure), the workflow structure (manual-only, independent summary job), and the CLI.
Functionality — how & why	How: pure string mapping. Digits-only parsing means any missing or odd value reads as "unknown". The artifact link is rendered only for the canonical actions/runs/<id>/artifacts/<id> URL. Why: missing evidence must never look like a passing suite, and the summary must not render untrusted outputs as Markdown.
Diff vs current develop HEAD	None (new files).
Recommended merge proposal	Merged as-is. 42 passed locally; CI's Python job ran them (648 passed, 133 deselected).
Actual merge proposal	Merged as-is through the merge queue in 8f291c11.
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Developer/CONTRIBUTING.md + Documentation/technical/testing/dincli-testing-guide.md

Field	Value
Change	Modified (both)
Lines	+1/-1 · +21/-4
Diff (what exactly is in this PR)	The integration-suite paragraph and the guide's "CI integration job" section now describe the summary job, the best-effort upload after failure or cancellation, and the timeout limits. The local reproduction snippet gains set -o pipefail.
Functionality — how & why	How: documentation only. Why: keeps the contributor docs in line with the workflow's new reporting.
Diff vs current develop HEAD	None.
Recommended merge proposal	Merged as-is. Docs CI green. Observation No. 2 (pipefail stays on in the reader's shell) is trivial.
Actual merge proposal	Merged as-is through the merge queue in 8f291c11.
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Verification

See the verification comment above: CI run 41465774312 is green, the 42 focused tests pass locally, and the break-then-restore check failed 5 tests then passed 42. A hosted Full GI run is still pending; the first manual dispatch after merge covers it.

Local vs. GitHub agree: yes. Both report a clean merge.

[umeradl]

Approved on merit at 785f452: CI OK green, 42 focused tests pass locally (break-then-restore confirmed), mergeable as-is. Two non-blocking notes in the verification comment.

[robertocarlous]

Reviewed against develop in an isolated worktree (branch applies cleanly on
top of current tip, 74a845c9, 1 commit, no conflicts — confirmed by both
git merge-tree --write-tree locally (exit 0, no file list) and GitHub's own
mergeable: MERGEABLE / mergeStateStatus: BLOCKED (blocked only on missing
approval, not a conflict)). CI on 785f452f: CI OK green — covers Python (unit tests, lint, formatting, the new test_full_gi_summary.py), Docs (doc-link checks on updated CONTRIBUTING.md and testing guide), Solidity (build and test, no Solidity changes in this PR). No foundry/src/** or dincli live-chain flows changed; the full CI suite is sufficient evidence.

Claimed: 42 focused reporting, shell failure-propagation and workflow tests passed.

Verified: reproduced locally — python3 -m pytest tests/test_full_gi_summary.py -v in the review worktree: 42 passed in 7.98s, 0 failures, 0 errors. Parametrize expansion accounts for: 7 (non-zero exit codes) + 5 (missing-outputs results) + 6 (malformed exit values) + 2 (interrupted-suite outcomes) + 2 (job-result override) + 4 (upload outcomes) + 6 (shell pipeline exit pairs) + 2 (evidence-write failure) + 1 (workflow structure) + 1 (CLI append) + 7 single-case tests = 43 count; the 16-function file produces exactly 42 parametrized cases by pytest's own count.

Claimed: Full non-integration suite: 648 passed, 133 integration tests deselected.

Verified: CI Python check green at 785f452f — run link. Not reproduced locally (full suite is CI's job; the new test file is the only addition and passes independently above).

Claimed: Workflow lint, Python lint/formatting, documentation links and whitespace checks passed.

Verified: CI Docs and Python checks green at 785f452f (links above).

Claimed: ci.yml is unchanged from the current base, preserving PR No. 308's merge-queue support.

Verified: git diff upstream/develop...pr-296-review -- .github/workflows/ci.yml returns empty after fetching the current develop tip (PR No. 308 merged to develop on 2026-11-02). The ci.yml diff that appeared earlier in the review was identical in both sides of the merge — absorbed at the base. Confirmed: 5 files in the final diff, ci.yml is not among them.

Claimed: INTEGRATION_NEEDS env injects the full needs context; missing outputs stay unknown, never success.

Verified: test_missing_outputs_never_claim_suite_or_setup_success passes with 5 parametrized job results including success, failure, cancelled, skipped, and empty string — each produces "no recorded pytest exit" and "no setup completion evidence". test_missing_integration_is_unknown confirms a completely absent integration key is "Incomplete/unknown". Both pass locally.

Claimed: Artifact URL is validated before being rendered as a Markdown link — arbitrary output cannot inject table rows.

Verified: test_unexpected_artifact_output_is_not_rendered_as_markdown feeds "bad)\n| fake | passed |" as the artifact URL and asserts the word "fake" does not appear in the report output. Passes locally. The regex in full_gi_summary.py (re.fullmatch(r"https://github\.com/[\w.-]+/[\w.-]+/actions/runs/[0-9]+/artifacts/[0-9]+", artifact_url)) requires an exact match — no free-form Markdown from GitHub Actions output can be rendered as a link.

Claimed: Shell failure propagation preserves both pytest and tee PIPESTATUS exits; evidence-write failure cannot hide pytest failure.

Verified: test_actual_suite_shell_preserves_both_pipeline_exits spins up fake pytest and tee binaries with configurable exit codes and runs the real suite step script via subprocess.run(bash -e -o pipefail -c <extracted script>). All 6 pairs ((0,0)→0, (1,0)→1, (2,0)→2, (5,0)→5, (0,7)→7, (2,7)→2) pass, confirming pytest's exit takes precedence when both are nonzero. test_evidence_write_failure_cannot_hide_pytest_failure passes a directory as GITHUB_OUTPUT (writes fail) and confirms (pytest_exit=0 → exit 1) and (pytest_exit=2 → exit 2). Both pass locally.

Claimed: Upload runs on failure() || cancelled(), not just failure().

Verified: test_workflow_keeps_manual_policy_and_independent_reporting asserts "if: failure() || cancelled()\n timeout-minutes: 3" in integration — passes locally. Also checked in the raw diff: - if: failure() → + if: failure() || cancelled() with + timeout-minutes: 3 added.

Claimed: Hosted execution of the revised Full GI workflow remains pending.

Not independently re-verified: live Full GI execution requires an active network and is outside CI. The PR author explicitly notes it as pending. This is expected for a manual-only workflow.

---

All checkable claims hold up. No foundry/src/** or mechanism changes; no second key required. The tests are thorough — the shell-level subprocess test (test_actual_suite_shell_preserves_both_pipeline_exits) goes beyond a unit test of the Python reporter and validates the actual bash step extracted from the workflow file. Looks good to merge.

[robertocarlous]

Files changed (5) — as of 785f452f (PR head)

Diffed against merge-base 74a845c9 (develop). develop moved significantly since this branch was cut (PR No. 308, "ci: run CI on merge_group", merged to develop on 2026-11-02), but only .github/workflows/ci.yml overlapped — and that file is byte-for-byte identical on both sides of the merge, so it is not present in the 5-file diff. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (blocked only on missing approval). Confirmed with a local git merge-tree --write-tree dry run: clean (exit 0, no file list).

.github/scripts/full_gi_summary.py

Field	Value
Change	New
Lines	+117/-0
Diff (what exactly is in this PR)	New Python script: number(value) for safe int parsing, summary(needs) to generate a 6-column Markdown table from the workflow's needs context, main() to read INTEGRATION_NEEDS from env, call summary(), and append the result to GITHUB_STEP_SUMMARY. Artifact URL is validated with re.fullmatch(r"https://github\.com/[\w.-]+/[\w.-]+/actions/runs/[0-9]+/artifacts/[0-9]+", ...) before being rendered as a link.
Functionality — how & why	How: The integration-summary job (new job, below) passes toJSON(needs) as the INTEGRATION_NEEDS env var, which this script reads and parses. summary() inspects keys result, outputs.setup_outcome, outputs.suite_outcome, outputs.pytest_exit, outputs.elapsed_seconds, outputs.upload_outcome, outputs.artifact_url on the integration entry and maps them to a human-readable markdown table row per field; missing or malformed values become explicit "unknown"/"no recorded…" strings rather than silently claiming success. Why: Before this PR the Full GI workflow produced no structured job summary on failure or cancellation, leaving maintainers with nothing but raw log links to triage a run.
Diff vs current develop HEAD	None — new file, not present on develop
Recommended merge proposal	Merged as-is — 42/42 tests pass locally, CI Python check green at 785f452f, URL injection guard in place
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

.github/workflows/full-gi.yml

Field	Value
Change	Modified
Lines	+52/-8
Diff (what exactly is in this PR)	Adds 7 outputs: to the integration job (setup_outcome, suite_outcome, pytest_exit, log_exit, elapsed_seconds, upload_outcome, artifact_url). Adds id: ready to the Set up environment step. Replaces simple status=$? in the main pytest step with PIPESTATUS capture (set +e; pytest … | tee …; pipeline_status=("${PIPESTATUS[@]}"); set -e; pytest_exit=…; log_exit=…). Extends upload step from if: failure() to if: failure() || cancelled(), adds id: upload and timeout-minutes: 3. Adds new integration-summary job (needs: integration, if: always(), timeout-minutes: 5, checkout fetch-depth: 1, runs full_gi_summary.py with INTEGRATION_NEEDS: ${{ toJSON(needs) }}).
Functionality — how & why	How: The integration job now exports granular exit status via job outputs (using per-step id: + echo "key=val" >> $GITHUB_OUTPUT). PIPESTATUS captures pytest and tee exits independently, preventing tee's exit masking pytest's. The upload step runs on both failure and cancellation now. The new integration-summary job reads needs.integration.outputs via toJSON(needs) and drives full_gi_summary.py. Why: Before this PR, a cancelled or failed run gave no structured output — maintainers couldn't distinguish "pytest failed" from "log capture failed" from "artifact upload failed" from a cancelled workflow. The if: always() on the summary job ensures the report appears even when the integration job is cancelled.
Diff vs current develop HEAD	None — the only develop-side change to full-gi.yml in the merge window came from develop itself (no other PRs touched it); ci.yml overlap with PR No. 308 is the only overlap, and that file is identical.
Recommended merge proposal	Merged as-is — workflow structural test (test_workflow_keeps_manual_policy_and_independent_reporting) passes; if: failure() || cancelled() and output keys verified in test; PIPESTATUS propagation verified by shell subprocess test
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Developer/CONTRIBUTING.md

Field	Value
Change	Modified
Lines	+1/-1
Diff (what exactly is in this PR)	Single line updated in the "Running the integration tests" section to describe the new Full GI outcome reporting (reference to the Step Summary table).
Functionality — how & why	How: Documentation update only — tells contributors where to find the structured outcome report when a Full GI run produces one. Why: Keeps the contributing guide accurate now that Step Summary output exists.
Diff vs current develop HEAD	None — CONTRIBUTING.md not touched on develop in the merge window
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

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

Field	Value
Change	Modified
Lines	+24/-12
Diff (what exactly is in this PR)	Updates the Full GI section of the testing guide: describes the 7 new outputs the integration job exposes, explains the Step Summary table and what each column means, and notes the PIPESTATUS capture for exit-code preservation.
Functionality — how & why	How: Documentation update only — adds a reference table for the 7 job outputs and a brief explanation of how the summary job consumes them. Why: Without updated docs, contributors wouldn't know what the new Step Summary fields mean or how to interpret exit-code combinations in the table.
Diff vs current develop HEAD	None — not touched on develop in the merge window
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

tests/test_full_gi_summary.py

Field	Value
Change	New
Lines	+281/-0
Diff (what exactly is in this PR)	New test file: 16 test functions covering the reporter (test_missing_outputs_never_claim_suite_or_setup_success, test_missing_integration_is_unknown, test_nonzero_pytest_exit_propagated, etc.), shell failure-propagation (test_actual_suite_shell_preserves_both_pipeline_exits — spins up fake pytest/tee binaries and runs the real bash step), workflow structure (test_workflow_keeps_manual_policy_and_independent_reporting), and injection guard (test_unexpected_artifact_output_is_not_rendered_as_markdown).
Functionality — how & why	How: Pytest parametrize expands the file to 42 test cases. Shell tests use subprocess.run with a PATH prepended with a temp bin dir containing fake pytest and tee scripts with configurable exit codes; the real suite-step bash script is extracted from the workflow YAML at test time. The workflow-structure test reads full-gi.yml directly with yaml.safe_load and asserts on-event triggers, continue-on-error absence, output key names, and summary-job shape. Why: The PIPESTATUS capture and artifact URL guard are subtle bash/regex behaviours that could silently regress — a unit test of the Python logic alone wouldn't catch them. The shell subprocess test provides the strongest evidence that the bash step actually preserves both exits.
Diff vs current develop HEAD	None — new file, not present on develop
Recommended merge proposal	Merged as-is — 42/42 pass locally and CI Python check green
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

---

Verification

Deep verification detail in the separate verification comment. Summary: python3 -m pytest tests/test_full_gi_summary.py -v — 42 passed in the review worktree. CI Python check green at 785f452f. All 4 CI checks pass.

Local vs. GitHub agree: git merge-tree --write-tree exit 0 (no file list) matches GitHub mergeable: MERGEABLE.

[robertocarlous]

Verified at 785f452: 42/42 tests pass locally, CI all green (Python, Docs, Solidity, CI OK), clean merge, PIPESTATUS capture and artifact URL guard confirmed by shell subprocess and regex tests respectively.

[umeradl]

Actual outcome — PR No. 296 merged as-is through the merge queue

This replaces the predictions in the pre-merge review comment with what actually happened when the PR landed.

One commit on origin/develop:

1. 8f291c11 — the merge of this PR (a merge commit, not a squash), path A. I approved head 785f452f and added it to the develop merge queue. This was the first PR through the queue, which went live today (Discussion 208). The queue merged it at 2026-11-02 15:16 UTC with parents 74a845c9 (develop) and 785f452f (PR head), and there were no conflicts, as predicted. GitHub shows the PR as MERGED.

Files unchanged from the PR (5 of 5)

.github/workflows/full-gi.yml, .github/scripts/full_gi_summary.py, tests/test_full_gi_summary.py, Developer/CONTRIBUTING.md, Documentation/technical/testing/dincli-testing-guide.md. Every file's "Actual merge proposal" is merged as-is.

Verification

• PR CI on 785f452f: run 41465774312, green.
• Merge-queue CI on gh-readonly-queue/develop/pr-296-74a845c9…: run 41468689673. Python, Docs, Solidity and CI OK all passed. This confirms PR No. 308's merge_group trigger reports CI OK in the queue.
• develop push CI on 8f291c11: run 41469936970, green.
• A hosted Full GI run is still pending, as noted in the review. The first manual dispatch covers it.

Local vs. GitHub agree: yes. Both predicted a clean merge, and that's what happened.