chore: remove silent-failure workarounds; add forbid-suppressions guard#549
Merged
Merged
Conversation
Ports the regression guard from HomericIntelligence/Odysseus#280 and refactors all 27 || true occurrences plus 10 continue-on-error: true suppressions per Bucket A–E classification. - Bucket A (masks real failures): pip-audit, benchmarks, gh-api milestone creation, cmake graphviz configure, clang-tidy build, semgrep, codeql c-cpp build deps + build — surfaced as explicit warnings/gates instead of silent. - Bucket B (best-effort cleanup): pkill, kill, wait, docker-compose down, clang-tidy per-file aggregator — wrapped in kill -0 guards or rc capture with stderr warnings for unexpected (rc>=2) failures. - Bucket C ((counter++) under set -e): converted TESTS_PASSED++ / TESTS_FAILED++ in test_docker.sh and verify_install.sh to $((var + 1)) so the first increment from 0 does not trip set -e. - Bucket D (pipeline-tail grep): replaced grep -c with awk-based counters in run_static_analysis.sh; replaced grep | grep filter in clang-tidy gate with a single awk script (always exits 0). - Bucket E (continue-on-error: true): removed all 10 occurrences. Trivy steps now rely on exit-code: "0"; docker build / CodeQL deps / CodeQL build / Semgrep capture rc into step outputs so downstream steps gate explicitly; gitleaks already used --exit-code 0 so the suppression was pure redundancy. Adds .pre-commit-config.yaml hooks forbid-or-true / forbid-continue-on-error and a forbid-suppressions job at the top of .github/workflows/_required.yml so the pattern cannot return. Local verification: - bash -n on every modified .sh: pass - shellcheck -S error on every modified .sh: pass (only SC2329/SC2155 info/warn) - python3 -c "import yaml; yaml.safe_load(...)" on every modified YAML: pass - pre-commit run forbid-or-true --all-files: pass - pre-commit run forbid-continue-on-error --all-files: pass - grep for || true and continue-on-error: true post-refactor: zero hits Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the regression guard from HomericIntelligence/Odysseus#280 and refactors all 27
|| trueoccurrences plus 10continue-on-error: truesuppressions across this repo per Bucket A–E classification.Two pygrep hooks are added to
.pre-commit-config.yamland aforbid-suppressionsjob is prepended to.github/workflows/_required.ymlso the pattern cannot return.Refactors
|| true(27 occurrences, 6 shell files + 2 workflow files)examples/multi_process_agents/scripts/launch_system.sh:9-12pkillloop with rc capture; warn on rc>=2 (rc=1 = no match, expected)examples/multi_process_agents/scripts/launch_system.sh:54killloop guarded bykill -0; stderr warn on real failureexamples/multi_process_agents/scripts/launch_system.sh:55waitloop with rc capture; signal-termination acceptedscripts/create_phase_issues.sh:164gh api ... || true→if ! gh api ...; then warn; return 1; fi(root cause: must report API failure)scripts/hmas-server.sh:120-121killloop guarded bykill -0; stderr warn on real failurescripts/run_static_analysis.sh:85clang-tidyper-file aggregator: rc captured, warn only on rc>1;grep -c || echo 0→ awk counters (Bucket D)scripts/test_docker.sh:145docker-compose down 2>/dev/null || true→if ! docker-compose down 2>/dev/null; then warn; fiscripts/test_docker.sh:177-184(5 occurrences)run_test ... || true→run_test ... && _rc=0 || _rc=$?soset -edoes not abort the aggregatorscripts/test_docker.sh:45,49((TESTS_PASSED++))/((TESTS_FAILED++))→TESTS_PASSED=$((TESTS_PASSED + 1))(pre-increment from 0 returns 1 underset -e)scripts/verify_install.sh:265cd "$PROJECT_ROOT" || true→ guard with[[ -n PROJECT_ROOT && -d $PROJECT_ROOT ]]beforecdscripts/verify_install.sh:280-286(7 occurrences)run_check ... || true→run_check ... && _rc=0 || _rc=$?aggregator patternscripts/verify_install.sh:43,47((TESTS_PASSED++))/((TESTS_FAILED++))→$((var + 1)).github/workflows/_required.yml:65(CMake graphviz)cmake ... || true→if ! cmake ...; then warn; fi(next step gates on[ -f deps.dot ]).github/workflows/_required.yml:177(clang-tidy grep tail)grep -E | grep -v ... || true→ singleawkfilter (always rc=0).github/workflows/_required.yml:601(pip-audit)pip-audit --strict || true→if ! pip-audit --strict; then warn; fi.github/workflows/extras.yml:69(benchmarks)make benchmark.native || true→if ! make benchmark.native; then warn; ficontinue-on-error: true(10 occurrences, all in.github/workflows/_required.yml)clang-tidy-build.rc; next step inspects build_rc and emits a warning if non-zero but no source-file errorsexit-code: "0"; the gate step downstream decides job pass/fail from the JSONdocker buildwrapped inif-then; writesbuilt=true/falseto$GITHUB_OUTPUT; downstreamif: steps.docker_build.outputs.built == 'true'instead ofoutcome == 'success'exit-code: "0"and removedcontinue-on-error(was masking, now gated bybuilt == 'true')--exit-code 0; central gate step decides pass/failpip install + semgrep scanthat captures rc into$GITHUB_OUTPUT; warning emitted on non-zerocodeql_deps.outputs.rc == '0'; warnings on non-zeroLint guard
.pre-commit-config.yaml: appendedforbid-or-trueandforbid-continue-on-errorpygrep hooks (preserving all existing hooks)..github/workflows/_required.yml: addedforbid-suppressionsjob at the top (runs beforelint); self-exemption for_required.ymland (future)docs/runbooks/no-silent-failures.md.Verification
bash -non every modified.sh: pass.shellcheck -S erroron every modified.sh: pass (only SC2329 info and SC2155 warn, both pre-existing).python3 -c "import yaml; yaml.safe_load(...)"on every modified YAML: pass.pre-commit run forbid-or-true --all-files: pass.pre-commit run forbid-continue-on-error --all-files: pass.\|\|\s*true(\s*$|\s+#)and^\s*continue-on-error:\s*true\s*$across.sh/.bash/.yml/.yaml/.hcl/Dockerfile*/justfile: zero hits.Full
pre-commit run --all-filescould not be executed locally because this repo pinsdefault_language_version: python: python3.12and onlypython3.9.2is available in the local environment — the new pygrep hooks themselves do not need a Python interpreter and were exercised individually with success above. The remoteforbid-suppressionsCI job is the authoritative backstop.Pre-existing issues observed (out of scope)
.yamllint.yamlwarns on several pre-existing long lines (grep -c ... \|\| echo 0style jq one-liners on lines 795-800 of_required.yml). These are formatting warnings, not silent failures (|| echo 0is not in scope of the lint guard's regex). Left as-is.scripts/test_docker.shandscripts/verify_install.shboth definerun_test/run_checkhelpers that increment counters via((var++))— fixed as part of Bucket C since they sit underset -e.Test plan
forbid-suppressionsjob passes on the PR (proves no regression).lintjob still passes (CMake graphviz cycle check, clang-tidy gate, mypy).security-dependency-scanjob still passes (pip-audit warning, Trivy gating intact).security/secrets-scanjob still passes (gitleaks gate, semgrep SARIF upload, CodeQL c-cpp + python).🤖 Generated with Claude Code