chore(ci): enforce scoped ShellCheck warning policy - #47
Conversation
Project 2 review checkpoint: blocked pending scope decisionPinned ShellCheck v0.10.0 inventory: https://github.com/LucaLin233/Linux/actions/runs/34174487781 . Diagnostic workflow intentionally reports warning failures; this is not the final warning gate. The SC2034 diagnostic at modules/system-customize.sh:2828 exposes a parser concern requiring a separate behavior/security review: xanmod_pending_allocation_proof_trusted reads two required lines, then rejects only when a third read succeeds. Bash read returns nonzero at EOF even when it assigns an unterminated nonempty tail to extra_line. That tail is never inspected. tools/xanmod-install.sh:727 has the same pattern. Call chain includes xanmod_pending_allocation_owned, xanmod_finish_pending_allocation, and pending allocation cleanup. This proves a static validation gap, NOT an arbitrary-deletion exploit; no behavior test has been run for this case. Minimal proposed separate fix: reject both a successful third read and a nonempty extra_line after EOF; add inert fixtures for exact two lines, newline-terminated third line, unterminated third-line bytes, and ownership cleanup fail-closed behavior. Do not execute production scripts or APT. Requires scope decision before implementation; no suppression added. Other observations: tests/test-push.sh has a file-wide SC2034 suppression; some inline suppressions lack nearby reasons. Review is incomplete. No production source changed. Current diff is only temporary diagnostic workflow (+17 lines); existing required jobs unchanged. Draft is NOT ready for merge. Projects 3/4/history not started. Await LENS review and user scope decision. |
Continued independent reviewRemoved 21 redundant SC2034 directives from error-gated worker fixtures, in addition to the file-wide directive removed earlier. Replaced bounded brace-list retry loops with explicit arithmetic counters in both mirrored XanMod implementations; retry count remains 64. Added README coverage/exception policy. Local diff check and bash -n passed for all four changed Shell files; no local behavior tests or ShellCheck ran. Unresolved: extra_line parser finding remains intentionally unsuppressed and outside lint-only behavior scope; BUILDING state warnings still need semantic review. No green-CI claim. Coverage regression tests and remaining production exception rationale audit are still outstanding. Current PR is incomplete/draft, not for merge. Do not advance projects 3/4/history. |
|
CI investigation: run 34220440919 attempt 1 failed on Debian in cleanup_failed/after-rename/HUP (expected 129, actual 2), with tools/push.sh trap parsing EOF diagnostic. Ubuntu and actual-workflow lint probes passed. Read-only comparison confirms tools/push.sh is unchanged against main and the worker-registration test is unchanged since green HEAD 86e7f70. This does not prove environmental flakiness or rule out latent signal races. Preserve this run as evidence; rerun only the failed Debian job once without code, assertion, timeout or suite changes. A successful rerun will not establish root cause. No merge. |
|
Review decision superseding the prior conditional-merge recommendation: BLOCKED, do not merge. Attempt 1 signal/trap parse failure is unexplained; attempt 2 success does not resolve it. Read-only call-chain review: publication fixture waits for worker hook marker then sends HUP to main; that does not synchronize main to a specific instruction. Main can be in wait_for_all_workers -> cleanup_active_failed_worker_sessions / prune_active_workers. runtime_signal_handler is registered as a constant string (runtime_signal_handler HUP 129), not an interpolated trap command. The EOF diagnostic alone does not identify a malformed generated trap, Bash defect, or fixture race. No root cause proven. Recommended separate bounded diagnostic branch/PR from latest main, with explicit Bash/package version capture, case/phase/PID diagnostics and inert reproduction. No global xtrace or real credentials; no production execution. Do not weaken assertions, extend timeouts, or repeatedly rerun to obtain green. Preserve both attempts. Further new diagnostic PR requires scope confirmation. |
|
Latest verification: HEAD 5961fb2 passed both required full=true jobs (11 suites each), run 34237117645. However main merge run 34237080747 FAILED Debian test-push.sh: partial-server-failure unexpectedly entered batch lifecycle failure; following all-failure case hit old barrier and recorded 0 instead of 2 failures. This is not the prior trap EOF or stage-ready TERM residue. Keep PR blocked; do not rerun to green or accept PR success as main verification. Source review shows partial-failure test asserts only nonzero and one failed record, so it does not distinguish expected transfer failure from lifecycle failure. Root cause of the earlier lifecycle barrier remains unproven; do not clear barriers or relax assertions as a workaround. |
Latest validation — still blocked on the batch investigationNormal merge synchronized #52/main d776297. Current HEAD: f7febbf. https://github.com/LucaLin233/Linux/actions/runs/34249483148 passed both required environments, full=true with all 11 suites each. The workflow-derived lint spy passed all four production/test success-failure combinations in both environments. Ubuntu production warning lint and test error lint passed. Complete diff remains 10 files, +94/-54; local diff checks and changed-script bash -n passed. Main post-#52 run 34248695868 passed both full suites. Synchronized diagnostic PR #51 run 34249483414 also passed both full suites and all eight partial/all-failure rounds. However, original main run 34237080747 batch lifecycle failure has not been causally explained. No rerun-to-green or speculative production fix was performed. These new green runs supersede old CI status only, not the unresolved investigation finding. Assessment: lint changes are reviewed and tested, but overall acceptance remains blocked. Keep Draft; no merge requested for #47 at this point. Do not interpret historical conditional-merge comments as current approval. |
1 participant
Summary
Project 2: ShellCheck v0.10.0 gates all tracked production .sh outside tests at warning, tests at error. Both groups execute and failures accumulate. Remove redundant test SC2034 suppressions; document retained local exceptions; remove write-only XanMod flags while retaining consumed snapshot flags, mirrored implementations and 64-attempt bounds.
Scope
Base f4418fb; HEAD 5e5e60f; branch luno/chore-shellcheck-warning-policy. Complete diff: 10 files, +94/-54. #51 and #54 are synchronized by ordinary merge. No production tools/push.sh modification, no #41 changes, no retired tool restoration.
Validation
PR run https://github.com/LucaLin233/Linux/actions/runs/34253836537 passed Ubuntu 24.04 lint and tests and Debian 13 tests: full=true, all 11 suites in each environment, all eight batch diagnostic rounds. Workflow-derived lint spy passed all four production/test result combinations in both environments. Actual Ubuntu production warning/test error gates passed.
Main post-#54 run https://github.com/LucaLin233/Linux/actions/runs/34253827734 independently passed both environments, full=true, 10 suites each including the batch diagnostics.
Local git diff --check and changed-script bash -n passed. No host ShellCheck or behavior tests. Complete diff and merge-parent readback reviewed again after synchronization; no unrelated changes. Same-assistant second-pass review, not third-party review.
Risks
Original batch failure remains unresolved in issue #53; user explicitly accepted deferral, not a fix claim. #54 retains known failing substitution controls as opt-in diagnostics, with default plain/pipeline exact signal assertions unchanged. Green finite timing coverage is not proof of Bash immunity. Ubuntu full CI is near the existing 15-minute job limit; no timeout increase or rerun-to-green. Test fixtures still exercise generic allocation building outputs; removed production flags have no consumers.
Rollback and authorization
Recommend merge for this lint-policy scope with the explicitly accepted deferred issue. Revert the eventual squash commit through a separate PR if needed. No production services, configuration or data changed and no real credentials supplied to tests. PR remains Draft pending explicit merge confirmation; no merge performed. Projects 3/4/history remain pending.