fix(ci): pin comm to LC_ALL=C in reborn crate discovery - #6992
Merged
Merged
Conversation
discover-reborn-package-crates.sh sorts both comm inputs with LC_ALL=C but ran comm itself in the ambient locale. Under a UTF-8 collation (which orders ironclaw_events before ironclaw_event_streams, unlike C) comm rejected the C-sorted input with "comm: input is not in sorted order", killing the pre-push coverage ratchet for any contributor with a UTF-8 locale. Regression test scripts/ci/test-ci-comm-locale-pin.sh asserts every comm invocation in scripts/ci and .githooks is LC_ALL=C-pinned and exercises the failing fixture pair under a UTF-8 locale; wired into the code_style static-check self-tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-6992 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6992
August 1, 2026 02:43
Destroyed
github-actions
Bot
added
scope: ci
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change pins the dependency-closure ChangesCI locale determinism
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a locale-dependent CI failure in Reborn crate discovery by ensuring comm uses the same collation (LC_ALL=C) as its already-LC_ALL=C-sorted inputs, and adds a self-test to prevent regressions.
Changes:
- Pin
commtoLC_ALL=Cinscripts/ci/discover-reborn-package-crates.shto avoid “input is not in sorted order” under UTF-8 collations. - Add a regression test (
scripts/ci/test-ci-comm-locale-pin.sh) that (a) enforces locale-pinnedcommusage in CI scripts/hooks and (b) reproduces the historical ordering case under a UTF-8 locale. - Wire the new regression test into the
code_styleworkflow static-check self-tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/ci/discover-reborn-package-crates.sh | Pins comm collation to match LC_ALL=C-sorted inputs, fixing locale-sensitive failures. |
| scripts/ci/test-ci-comm-locale-pin.sh | Adds a regression guard for locale-pinned comm usage and a UTF-8-locale fixture check. |
| .github/workflows/code_style.yml | Runs the new regression script as part of static-check self-tests in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/ci/test-ci-comm-locale-pin.sh`:
- Around line 27-31: Update the regression test around unpinned to normalize
multiline shell commands before inspection, then detect complete comm
invocations with or without options while allowing the valid LC_ALL=C
continuation form. Change the dynamic locale check to select only a non-C locale
whose unpinned comm rejects the C-sorted fixture, and remove any silent fallback
to C.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 495d405f-1d3f-4a98-9cd4-26d534063b4e
📒 Files selected for processing (3)
.github/workflows/code_style.ymlscripts/ci/discover-reborn-package-crates.shscripts/ci/test-ci-comm-locale-pin.sh
Contributor
🔎 Review · PR #6992
Submitted review →The locale pin is correct and consistently matches the existing C-sorted inputs. The regression test is appropriately wired into fast checks and covers both repository-wide pinning and the collation-sensitive fixture. No actionable findings. Automatic · PR opened · attempt 1 of 3 · completed in 58s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #6992
✅ No actionable findings
The locale pin is correct and consistently matches the existing C-sorted inputs. The regression test is appropriately wired into fast checks and covers both repository-wide pinning and the collation-sensitive fixture. No actionable findings.
Validation and technical details
- Verified trusted base fe8f5c2 and head 6cf311d.
- Inspected the complete base-to-head diff and surrounding workflow and crate-discovery code.
bash -npassed for both changed shell scripts.scripts/ci/test-ci-comm-locale-pin.shpassed: 2 passed, 0 failed.- Confirmed the new test script is executable and invoked by the code-style fast-check job.
- The end-to-end crate-discovery script could not run in this review environment because
cargois unavailable. - Base:
main - Head:
fix/ci-comm-locale-pinat6cf311d - Run:
ca956fe4-1983-426e-ba3f-1bdd4533ae00
Address CodeRabbit review on #6992: - Join backslash-newline continuations before inspection so multiline comm invocations are caught and the valid "LC_ALL=C \" + "comm" continuation form is not false-flagged; match comm with or without options; prune __pycache__ and follow .githooks symlinks. - Select the case-2 locale by proving the mismatch (unpinned comm must reject the C-sorted fixture under it) instead of silently falling back to a C-compatible collation like C.UTF-8; skip explicitly when no installed locale disagrees. A "zzz" sentinel second file forces comm's order check, which never fires on two identical files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6992
August 1, 2026 04:42
Destroyed
There was a problem hiding this comment.
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 `@scripts/ci/test-ci-comm-locale-pin.sh`:
- Around line 35-45: Update the `hits` detection in `test-ci-comm-locale-pin.sh`
to inspect each `comm` invocation independently rather than excluding an entire
logical line when any invocation is pinned, including commands separated by `;`
or `&&`. Preserve multiline command handling, and add a negative regression test
covering a pinned `comm` followed by an unpinned `comm` in a compound command.
- Around line 52-83: Update the locale-selection loop around mismatch_locale to
first verify that the sentinel “zzz” sorts after both fixture entries under each
candidate locale, and only select a candidate that passes this ordering check
and rejects the C-sorted fixture. Preserve the existing skip behavior when no
qualifying locale is found, and keep the pinned LC_ALL=C assertion for the
selected locale.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a83a2f19-5e2a-4979-9170-1c7ccf4304d7
📒 Files selected for processing (1)
scripts/ci/test-ci-comm-locale-pin.sh
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/ci/test-ci-comm-locale-pin.sh:66
- Temporary files created with mktemp are only removed at the end of the script, so a failure/early exit (due to set -e) will leak /tmp files. Add an EXIT trap immediately after creating them so cleanup runs even when a case fails.
fixture="$(mktemp "${TMPDIR:-/tmp}/comm-locale.XXXXXX")"
sentinel="$(mktemp "${TMPDIR:-/tmp}/comm-locale.XXXXXX")"
printf 'ironclaw_event_streams\nironclaw_events\n' > "$fixture"
printf 'zzz\n' > "$sentinel"
…n test The regression-test enforcement gate recognizes shell regression tests by their assertion idiom (assert_success/assert_failure among them); the custom report-only form was invisible to it. Restructure the fixture case into explicit assert_failure (unpinned comm rejects the C-sorted fixture) and assert_success (pinned comm accepts it), which also states the red side of the regression explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two review findings on the locale-pin regression test: - The scan excluded a whole joined logical line once it contained one pinned comm, so 'LC_ALL=C comm a b; comm c d' passed with the second invocation unpinned. Compound commands are now split at ;, &, and | before the pin check, and scanner self-checks pin the compound, multiline-continuation, and pinned-clean cases. - The mismatch-locale selection now verifies the zzz sentinel still sorts after both fixture entries under the candidate locale; without that, a locale ordering the sentinel early would never exercise the fixture-order path comm is being tested on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Reviewed this alongside #6997, #7001 and #5981. The production fix is correct and minimal, and the sweep is clean — repo-wide there's exactly one other 1. The guard checks The invariant is comm's collation must match its inputs', but Case 1 only audits one side of it. I removed Worth extending Case 1 to assert that any 2. Case 2 doesn't execute on dev machines or in CI.
3. The failure message points at the wrong line. The description says it fails "pointing at the exact line". I stashed the fix and ran it — it reported line 31 for the violation at line 43. The Minor: a line containing both a pinned and an unpinned None of this blocks the fix itself — it's a one-line change that's clearly right. |
This was referenced Aug 2, 2026
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6992
August 2, 2026 03:35
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/ci/test-ci-comm-locale-pin.sh`:
- Around line 45-76: Update scan_unpinned_comm so each shell command, including
nested process substitutions, is evaluated independently rather than allowing an
outer LC_ALL=C comm match to exempt an inner invocation. If nested constructs
cannot be parsed safely, reject them explicitly; add the specified
process-substitution fixture to the scanner self-checks and assert it fails.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3992d46d-fce8-4d0b-bda8-f63cf16120ea
📒 Files selected for processing (1)
scripts/ci/test-ci-comm-locale-pin.sh
Comment on lines
+45
to
+76
| # Case 1: every comm invocation in CI scripts and tracked hooks must pin | ||
| # LC_ALL=C so its collation matches the LC_ALL=C-sorted inputs. Backslash- | ||
| # newline continuations are joined first so multiline invocations — both the | ||
| # offending `comm \` form and a valid `LC_ALL=C \` + `comm` form — are | ||
| # inspected as whole commands; comment lines are then dropped, and compound | ||
| # commands are split at `;`, `&`, and `|` so each invocation is inspected | ||
| # independently (a pinned comm must not excuse an unpinned one on the same | ||
| # logical line). `find -L` follows the .githooks symlinks to their tracked | ||
| # targets; build caches are pruned. This file is excluded: it deliberately | ||
| # runs unpinned comm to prove the failure mode. | ||
| scan_unpinned_comm() { | ||
| sed -e ':a' -e '/\\$/{N; s/\\\n[[:space:]]*/ /; ba' -e '}' "$1" \ | ||
| | grep -av '^[[:space:]]*#' \ | ||
| | tr ';&|' '\n\n\n' \ | ||
| | grep -anE '(^|[^-=[:alnum:]_.])comm([[:space:]]|$)' \ | ||
| | grep -avE 'LC_ALL=C[[:space:]]+comm([[:space:]]|$)' \ | ||
| || true | ||
| } | ||
|
|
||
| # Scanner self-checks: each invocation in a compound command is judged on its | ||
| # own, and the pinned forms (single, compound, multiline continuation) stay | ||
| # clean. | ||
| scanner_fixture="$(mktemp "${TMPDIR:-/tmp}/comm-locale.XXXXXX")" | ||
| printf 'LC_ALL=C comm -12 a b; comm -12 c d\n' > "$scanner_fixture" | ||
| assert_failure "scanner flags an unpinned comm after a pinned one in a compound command" \ | ||
| test -z "$(scan_unpinned_comm "$scanner_fixture")" | ||
| printf 'comm \\\n -12 a b\n' > "$scanner_fixture" | ||
| assert_failure "scanner flags an unpinned multiline comm continuation" \ | ||
| test -z "$(scan_unpinned_comm "$scanner_fixture")" | ||
| printf 'LC_ALL=C comm -12 a b && LC_ALL=C \\\n comm -12 c d\n' > "$scanner_fixture" | ||
| assert_success "scanner accepts pinned comm in compound and multiline forms" \ | ||
| test -z "$(scan_unpinned_comm "$scanner_fixture")" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
input='LC_ALL=C comm -12 a b < <(comm -12 c d)'
hits="$(
printf '%s\n' "$input" \
| grep -av '^[[:space:]]*#' \
| tr ';&|' '\n\n\n' \
| grep -anE '(^|[^-=[:alnum:]_.])comm([[:space:]]|$)' \
| grep -avE 'LC_ALL=C[[:space:]]+comm([[:space:]]|$)' \
|| true
)"
test -n "$hits"Repository: nearai/ironclaw
Length of output: 153
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== file =="
fd -a 'test-ci-comm-locale-pin\.sh' . || true
echo "== relevant source =="
file="$(fd 'test-ci-comm-locale-pin\.sh' . | head -n 1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,120p' "$file" | cat -n
fi
echo "== invariant docs references =="
for f in CLAUде.md AGENTS.md .claude/rules/* .claude/rules 2>/dev/null; do
[ -e "$f" ] || continue
name="$(printf '%s' "$f" | sed 's#^\./##')"
echo "--- $name ---"
sed -n '1,220p' "$name" 2>/dev/null | rg -n "Guardrails|hooks|regression|multiline|commit|secrets|migration|docs/documentation" | head -n 40 || true
done
echo "== deterministic scanner on nested case =="
python3 - <<'PY'
from pathlib import Path
import re
# Reproduce scanner lines 45-76 exactly as comments describe.
def scan_unpinned_comm(text):
# Simulate sed continuation join without line numbers.
joined=[]
for line in text.splitlines():
if joined and joined[-1].endswith('\\'):
joined[-1]=joined[-1]+' '+line.lstrip() if len(line)>0 else joined[-1]
else:
joined.append(line)
# Simulate grep -av regex line filter and tr.
step='\n'.join(l for l in joined if not re.match(r'^[[:space:]]*#', l))
compound_lines=step.replace(';', '\n').replace('&', '\n').replace('|', '\n').splitlines()
hits=[]
for i,line in enumerate(compound_lines,1):
if re.search(r'(^|[^-=[:alnum:]_.])comm([[:space:]]|$)', line):
hits.append((i,line))
return [(i,line) for i,line in hits if not re.search(r'LC_ALL=C[[:space:]]+comm([[:space:]]|$)', line)]
case='LC_ALL=C comm -12 a b < <(comm -12 c d)'
print(scan_unpinned_comm(case))
cases=[
'LC_ALL=C comm -12 a b < <(comm -12 c d)',
'LC_ALL=C comm -12 a b && comm -12 c d',
'comm \\\n -12 a b',
'LC_ALL=C comm -12 a b && LC_ALL=C \\\n comm -12 c d',
]
for case in cases:
print(repr(case), scan_unpinned_comm(case))
PYRepository: nearai/ironclaw
Length of output: 6048
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== invariant docs references =="
for f in CLAUDE.md AGENTS.md; do
if [ -e "$f" ]; then
name="$(printf '%s' "$f" | sed 's#^\./##')"
echo "--- $name ---"
sed -n '1,220p' "$name" | rg -n "Guardrails|hooks|regression|multiline|commit|secrets|migration|documentation" | head -n 40 || true
fi
done
if [ -d .claude/rules ]; then
for f in .claude/rules/*; do
[ -e "$f" ] || continue
echo "--- $f ---"
sed -n '1,220p' "$f" | rg -n "Guardrails|hooks|regression|multiline|commit|secrets|migration|documentation" | head -n 40 || true
done
fi
echo "== deterministic scanner-like behavior on nested case =="
python3 - <<'PY'
import subprocess
input='LC_ALL=C comm -12 a b < <(comm -12 c d)'
scanner_fixture='/tmp/coderabbit-comm-scanner-fixture'
with open(scanner_fixture, 'w') as f:
f.write(input + '\n')
hits = subprocess.run(
['sed', '-e', ':a', '-e', '/\\$/{N; s/\\\n[[:space:]]*/ /; ba', '-e', '}', scanner_fixture],
stdout=subprocess.PIPE,
text=True,
check=True,
).stdout | subprocess.run(['grep', '-av', '^[[:space:]]*#'], input=subprocess.STDOUT, text=True, check=True).stdout | subprocess.run(['tr', ';&|', '\n\n\n'], input=subprocess.STDOUT, text=True, check=True).stdout | subprocess.run(['grep', '-anE', '(^|[^-=[:alnum:]_.])comm([[:space:]]|$)'], input=subprocess.STDOUT, text=True) or '' | subprocess.run(['grep', '-avE', 'LC_ALL=C[[:space:]]+comm([[:space:]]|$)'], input=hits.stdout, text=True) or ''
print(repr(input))
print('hits', repr(hits))
print('empty', hits == '')
cases = [
'LC_ALL=C comm -12 a b < <(comm -12 c d)',
'LC_ALL=C comm -12 a b && comm -12 c d',
'comm \\\n -12 a b',
'LC_ALL=C comm -12 a b && LC_ALL=C \\\n comm -12 c d',
]
PY
echo "== targeted command to emulate grep chain =="
test_file="$(mktemp)"
printf '%s\n' 'LC_ALL=C comm -12 a b < <(comm -12 c d)' > "$test_file"
tmp1="$(sed -e ':a' -e '/\\$/{N; s/\\\n[[:space:]]*/ /; ba' -e '}' "$test_file")"
echo "sed_output:"
printf '%s\n' "$tmp1"
tmp2="$(printf '%s\n' "$tmp1" | grep -av '^[[:space:]]*#')"
echo "comment_filter:"
printf '%s\n' "$tmp2"
tmp3="$(printf '%s\n' "$tmp2" | tr ';&|' '\n\n\n')"
echo "compound_split:"
printf '%s\n' "$tmp3"
tmp4="$(printf '%s\n' "$tmp3" | grep -anE '(^|[^-=[:alnum:]_.])comm([[:space:]]|$)' || true)"
echo "comm_hits:"
printf '%s\n' "$tmp4"
tmp5="$(printf '%s\n' "$tmp4" | grep -avE 'LC_ALL=C[[:space:]]+comm([[:space:]]|$)' || true)"
echo "final_hits:"
printf '%s\n' "$tmp5"
rm -f "$test_file"Repository: nearai/ironclaw
Length of output: 6327
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== deterministic scanner-like behavior on nested case =="
input='LC_ALL=C comm -12 a b < <(comm -12 c d)'
test_file="$(mktemp)"
printf '%s\n' "$input" > "$test_file"
step1="$(sed -e ':a' -e '/\\$/{N; s/\\\n[[:space:]]*/ /; ba' -e '}' "$test_file")"
echo "sed/output:"
printf '%s\n' "$step1"
step2="$(printf '%s\n' "$step1" | grep -av '^[[:space:]]*#')"
echo "after comment filter:"
printf '%s\n' "$step2"
step3="$(printf '%s\n' "$step2" | tr ';&|' '\n\n\n')"
echo "after compound split:"
printf '%s\n' "${step3:-<empty>}"
step4="$(printf '%s\n' "$step3" | grep -anE '(^|[^-=[:alnum:]_.])comm([[:space:]]|$)' || true)"
echo "after comm matcher:"
printf '%s\n' "${step4:-<empty>}"
final="$(printf '%s\n' "$step4" | grep -avE 'LC_ALL=C[[:space:]]+comm([[:space:]]|$)' || true)"
echo "final hits:"
printf '%s\n' "${final:-<empty>}"
echo "non-empty? $([ -n "$final" ] && echo yes || echo no)"
rm -f "$test_file"
echo "== minimal grep regex behavior =="
python3 - <<'PY'
import re
line = 'LC_ALL=C comm -12 a b < <(comm -12 c d)'
for pat in ['LC_ALL=C[[:space:]]+comm([[:space:]]|$)', r'(?:^|(?<=[^-=[:alnum:]_.]))comm(?=[[:space:]]|$)']:
print(pat, re.search(pat, line))
PYRepository: nearai/ironclaw
Length of output: 797
Reject nested comm inside a pinned logical line.
The scanner treats pinning at the LC_ALL=C[[:space:]]+comm line filter as a line-level exemption, while the invariant at .claude/rules/review-discipline.md:136 requires checks/hooks to handle multiline syntax and keep guarantees aligned with tests. Handle nested shell command forms independently, or reject unsupported nested constructs outright, and add LC_ALL=C comm -12 a b < <(comm -12 c d) as a failing self-check.
🤖 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 `@scripts/ci/test-ci-comm-locale-pin.sh` around lines 45 - 76, Update
scan_unpinned_comm so each shell command, including nested process
substitutions, is evaluated independently rather than allowing an outer LC_ALL=C
comm match to exempt an inner invocation. If nested constructs cannot be parsed
safely, reject them explicitly; add the specified process-substitution fixture
to the scanner self-checks and assert it fails.
Source: Coding guidelines
BenKurrek
approved these changes
Aug 2, 2026
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* fix(ci): pin comm to LC_ALL=C in reborn crate discovery discover-reborn-package-crates.sh sorts both comm inputs with LC_ALL=C but ran comm itself in the ambient locale. Under a UTF-8 collation (which orders ironclaw_events before ironclaw_event_streams, unlike C) comm rejected the C-sorted input with "comm: input is not in sorted order", killing the pre-push coverage ratchet for any contributor with a UTF-8 locale. Regression test scripts/ci/test-ci-comm-locale-pin.sh asserts every comm invocation in scripts/ci and .githooks is LC_ALL=C-pinned and exercises the failing fixture pair under a UTF-8 locale; wired into the code_style static-check self-tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): harden comm locale-pin regression test per review Address CodeRabbit review on nearai#6992: - Join backslash-newline continuations before inspection so multiline comm invocations are caught and the valid "LC_ALL=C \" + "comm" continuation form is not false-flagged; match comm with or without options; prune __pycache__ and follow .githooks symlinks. - Select the case-2 locale by proving the mismatch (unpinned comm must reject the C-sorted fixture under it) instead of silently falling back to a C-compatible collation like C.UTF-8; skip explicitly when no installed locale disagrees. A "zzz" sentinel second file forces comm's order check, which never fires on two identical files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): use assert_success/assert_failure helpers in comm locale-pin test The regression-test enforcement gate recognizes shell regression tests by their assertion idiom (assert_success/assert_failure among them); the custom report-only form was invisible to it. Restructure the fixture case into explicit assert_failure (unpinned comm rejects the C-sorted fixture) and assert_success (pinned comm accepts it), which also states the red side of the regression explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): per-invocation comm scan and sentinel-order guard per review Two review findings on the locale-pin regression test: - The scan excluded a whole joined logical line once it contained one pinned comm, so 'LC_ALL=C comm a b; comm c d' passed with the second invocation unpinned. Compound commands are now split at ;, &, and | before the pin check, and scanner self-checks pin the compound, multiline-continuation, and pinned-clean cases. - The mismatch-locale selection now verifies the zzz sentinel still sorts after both fixture entries under the candidate locale; without that, a locale ordering the sentinel early would never exercise the fixture-order path comm is being tested on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
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
scripts/ci/discover-reborn-package-crates.shsorts bothcomminputs withLC_ALL=Cbut rancommitself in the ambient locale. Under a UTF-8 collation — which ordersironclaw_eventsbeforeironclaw_event_streams, the opposite of C order (_0x5f <s0x73) —commrejects the C-sorted input withcomm: input is not in sorted order, killing the pre-push coverage ratchet for any contributor whose shell has a UTF-8 locale. Hit in practice while pushing #6991 (noted in that PR's body).Fix: one line —
LC_ALL=C comm -12so comm's collation matches its inputs.Regression test
scripts/ci/test-ci-comm-locale-pin.sh(wired into the code_style static-check self-tests):comminvocation inscripts/ci/and.githooks/isLC_ALL=C-pinned — fails on the pre-fix tree pointing at the exact line.ironclaw_event_streams/ironclaw_events) under an available UTF-8 locale with the pinned form.Verified red-then-green: test fails with the fix stashed, passes with it applied;
discover-reborn-package-crates.shnow runs end-to-end underen_US.UTF-8and emits the crate closure (previously exit 1).🤖 Generated with Claude Code