fix(sop-tier-check): clause splitter strips newlines — every tier:low PR fails (#229) #243

claude-ceo-assistant · 2026-05-10T05:03:56Z

2026-05-10 05:03:56 +00:00

Closes #229.

Phase 1 — root cause

.gitea/scripts/sop-tier-check.sh ~line 289 (introduced in #225, untouched by #231):

_clause=$(echo "$_raw_clause" | tr -d '()' | tr ',' '\n' | tr -d '[:space:]' | grep -v '^$')

tr -d '[:space:]' strips the newlines that tr ',' '\n' just inserted. For the tier:low expression "engineers,managers,ceo" the pipeline collapses to a single nonsense token engineersmanagersceo. The for-loop iterates once; the case pattern *engineersmanagersceo* never matches a real APPROVER_TEAMS entry. Every tier:low PR fails — confirmed against PRs #233/#235/#236/#237/#238 in run logs (token resolves to sop-tier-bot, then immediately FAIL).

Secondary bug at the same site: _clause_names="${_clause_names:+, }${_t}" overwrites the variable on every iteration instead of appending. With the splitter bug, the inner loop only ran once so the overwrite was invisible. The error string teamsengineersmanagersceo is the smoking gun for both bugs simultaneously.

Phase 2 — design

Per-tier policy is UNCHANGED. This is a parser fix, not a policy relaxation. tier:low remains an OR-set across {engineers, managers, ceo}; tier:medium remains AND-of-clauses; tier:high remains ceo-only. The hard gate that internal#189 designed continues to function — it just couldn't function at all before this fix because the clause never tokenised.

Fix shape:

# Splitter: parens out, commas to spaces, let bash word-split iterate.
_no_parens=${_raw_clause//[()]/}
_clause=${_no_parens//,/ }

# Accumulators: prepend prior value before the comma-separator.
_clause_names="${_clause_names}${_clause_names:+, }${_t}"
_passed_clauses="${_passed_clauses}${_passed_clauses:+, }$_label"
_failed_clauses="${_failed_clauses}${_failed_clauses:+, }$_label"

The accumulator bug also existed at lines 314 and 317 (passed/failed clause lists across raw clauses). It was invisible for tier:low (one clause) but would have been wrong for tier:medium. Fixed atomically.

Phase 3 — implementation

.gitea/scripts/sop-tier-check.sh: splitter rewrite + 3 accumulator fixes + comments documenting the trap.
.gitea/scripts/tests/test_sop_tier_check_clause_split.sh: new unit test asserting splitter and accumulators behave as documented.

Commits authored as dev-lead <dev-lead@moleculesai.app> per per-agent-Gitea-identity policy.

Phase 4 — verify (test matrix)

Local bash .gitea/scripts/tests/test_sop_tier_check_clause_split.sh — 7/7 pass:

Case	Input	Expected tokens	Result
tier:low OR-clause splits	`engineers,managers,ceo`	`engineers \| managers \| ceo`	PASS
tier:medium AND-expr (5 raw clauses)	`managers AND engineers AND qa???,security???`	`managers ; AND ; engineers ; AND ; qa??? \| security???`	PASS
tier:high single team	`ceo`	`ceo`	PASS
paren-wrapped OR-set	`(managers,ceo)`	`managers \| ceo`	PASS
`_clause_names` accumulator	iter `engineers managers ceo`	`engineers, managers, ceo`	PASS
`_failed_clauses` accumulator	iter `clauseA clauseB clauseC`	`clauseA, clauseB, clauseC`	PASS
End-to-end OR-gate	APPROVER_TEAMS=`managers` vs `engineers,managers,ceo`	matches `managers`	PASS

Reproduction of the OLD broken splitter for the record:

$ echo 'engineers,managers,ceo' | tr -d '()' | tr ',' '\n' | tr -d '[:space:]' | grep -v '^$'
engineersmanagersceo   # ← single concatenated token instead of three lines

Downstream: once this lands, orchestrator should re-fire sop-tier-check on the 5 stuck PRs (#233/#235/#236/#237/#238). Not touching them in this PR per scope boundary.

Boundaries respected

Per-tier policy untouched. tier:high still requires ceo. tier:medium still AND-composes managers+engineers (+qa/security when teams exist).
No changes to the workflow YAML — fix lives entirely in the script, which is the workflow's documented single source of truth.
Did not modify the 5 stuck PRs (#233/#235/#236/#237/#238).
Did not revert #225 / #231.
BURN-IN window (continue-on-error: true in the workflow until 2026-05-17) left as-is — that's a separate cleanup tracked by the workflow comment.

Tier

tier:medium — CI gate regression that was blocking ALL team productivity on tier:low PRs. Filed by dev-lead per dev-sop Phase 1-4.

Closes #229. ## Phase 1 — root cause `.gitea/scripts/sop-tier-check.sh` ~line 289 (introduced in #225, untouched by #231): ```bash _clause=$(echo "$_raw_clause" | tr -d '()' | tr ',' '\n' | tr -d '[:space:]' | grep -v '^$') ``` `tr -d '[:space:]'` strips the newlines that `tr ',' '\n'` just inserted. For the tier:low expression `"engineers,managers,ceo"` the pipeline collapses to a single nonsense token `engineersmanagersceo`. The for-loop iterates once; the case pattern `*engineersmanagersceo*` never matches a real APPROVER_TEAMS entry. **Every tier:low PR fails** — confirmed against PRs #233/#235/#236/#237/#238 in run logs (token resolves to sop-tier-bot, then immediately FAIL). Secondary bug at the same site: `_clause_names="${_clause_names:+, }${_t}"` overwrites the variable on every iteration instead of appending. With the splitter bug, the inner loop only ran once so the overwrite was invisible. The error string `teamsengineersmanagersceo` is the smoking gun for both bugs simultaneously. ## Phase 2 — design **Per-tier policy is UNCHANGED.** This is a parser fix, not a policy relaxation. tier:low remains an OR-set across {engineers, managers, ceo}; tier:medium remains AND-of-clauses; tier:high remains ceo-only. The hard gate that internal#189 designed continues to function — it just couldn't function at all before this fix because the clause never tokenised. Fix shape: ```bash # Splitter: parens out, commas to spaces, let bash word-split iterate. _no_parens=${_raw_clause//[()]/} _clause=${_no_parens//,/ } # Accumulators: prepend prior value before the comma-separator. _clause_names="${_clause_names}${_clause_names:+, }${_t}" _passed_clauses="${_passed_clauses}${_passed_clauses:+, }$_label" _failed_clauses="${_failed_clauses}${_failed_clauses:+, }$_label" ``` The accumulator bug also existed at lines 314 and 317 (passed/failed clause lists across raw clauses). It was invisible for tier:low (one clause) but would have been wrong for tier:medium. Fixed atomically. ## Phase 3 — implementation - `.gitea/scripts/sop-tier-check.sh`: splitter rewrite + 3 accumulator fixes + comments documenting the trap. - `.gitea/scripts/tests/test_sop_tier_check_clause_split.sh`: new unit test asserting splitter and accumulators behave as documented. Commits authored as `dev-lead <dev-lead@moleculesai.app>` per per-agent-Gitea-identity policy. ## Phase 4 — verify (test matrix) Local `bash .gitea/scripts/tests/test_sop_tier_check_clause_split.sh` — **7/7 pass**: | Case | Input | Expected tokens | Result | |---|---|---|---| | tier:low OR-clause splits | `engineers,managers,ceo` | `engineers \| managers \| ceo` | PASS | | tier:medium AND-expr (5 raw clauses) | `managers AND engineers AND qa???,security???` | `managers ; AND ; engineers ; AND ; qa??? \| security???` | PASS | | tier:high single team | `ceo` | `ceo` | PASS | | paren-wrapped OR-set | `(managers,ceo)` | `managers \| ceo` | PASS | | `_clause_names` accumulator | iter `engineers managers ceo` | `engineers, managers, ceo` | PASS | | `_failed_clauses` accumulator | iter `clauseA clauseB clauseC` | `clauseA, clauseB, clauseC` | PASS | | End-to-end OR-gate | APPROVER_TEAMS=` managers ` vs `engineers,managers,ceo` | matches `managers` | PASS | Reproduction of the OLD broken splitter for the record: ``` $ echo 'engineers,managers,ceo' | tr -d '()' | tr ',' '\n' | tr -d '[:space:]' | grep -v '^$' engineersmanagersceo # ← single concatenated token instead of three lines ``` Downstream: once this lands, orchestrator should re-fire sop-tier-check on the 5 stuck PRs (#233/#235/#236/#237/#238). Not touching them in this PR per scope boundary. ## Boundaries respected - Per-tier policy untouched. tier:high still requires ceo. tier:medium still AND-composes managers+engineers (+qa/security when teams exist). - No changes to the workflow YAML — fix lives entirely in the script, which is the workflow's documented single source of truth. - Did not modify the 5 stuck PRs (#233/#235/#236/#237/#238). - Did not revert #225 / #231. - BURN-IN window (`continue-on-error: true` in the workflow until 2026-05-17) left as-is — that's a separate cleanup tracked by the workflow comment. ## Tier tier:medium — CI gate regression that was blocking ALL team productivity on tier:low PRs. Filed by dev-lead per dev-sop Phase 1-4.

claude-ceo-assistant added 1 commit 2026-05-10 05:03:57 +00:00

fix(sop-tier-check): clause splitter strips newlines, OR-set collapses to one token (#229 )

sop-tier-check / tier-check (pull_request) Failing after 5s

Details

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s

Details

audit-force-merge / audit (pull_request) Successful in 4s

Details

b75187d11c

PR #225 introduced the AND-composition clause evaluator. PR #231
patched the per-team case-pattern matching but did NOT fix the
underlying clause-splitter bug. This PR fixes the actual root cause
behind issue #229.

Root cause (.gitea/scripts/sop-tier-check.sh ~line 289):

    _clause=$(echo "$_raw_clause" \
      | tr -d '()' \
      | tr ',' '\n' \
      | tr -d '[:space:]' \
      | grep -v '^$')

`tr -d '[:space:]'` strips the newlines that `tr ',' '\n'` just
inserted. For tier:low (expression "engineers,managers,ceo") the
intermediate value is:

    engineers\nmanagers\nceo

then `tr -d '[:space:]'` flattens it to:

    engineersmanagersceo

The for-loop iterates ONCE over this single bogus token. The case
pattern `*engineersmanagersceo*` never matches APPROVER_TEAMS values
like " managers ", so EVERY tier:low PR fails:

    ::error::clause [engineers/managers/ceo]: FAIL — no approving
    reviewer belongs to any of these teamsengineersmanagersceo
    ::error::sop-tier-check FAILED for tier:low

(Note: the missing separators in the error string `teamsengineersmanagersceo`
were a SECOND, masked bug — `_clause_names="${_clause_names:+, }${_t}"`
overwrites the variable on every iteration instead of appending. With
the splitter bug, the inner loop only ran once so the overwrite was
invisible. Fixing the splitter unmasks the accumulator bug, so we fix
both atomically.)

Fix:

  _no_parens=${_raw_clause//[()]/}
  _clause=${_no_parens//,/ }   # comma -> space, bash word-split iterates

  # Append, don't overwrite:
  _clause_names="${_clause_names}${_clause_names:+, }${_t}"
  _passed_clauses="${_passed_clauses}${_passed_clauses:+, }$_label"
  _failed_clauses="${_failed_clauses}${_failed_clauses:+, }$_label"

Per-tier policy is UNCHANGED — this is a parser fix, not a policy
relaxation:

  tier:low    — engineers,managers,ceo   (OR-set, ANY ONE suffices)
  tier:medium — managers AND engineers AND qa???,security???
  tier:high   — ceo

Test: .gitea/scripts/tests/test_sop_tier_check_clause_split.sh
asserts the splitter, accumulators, and end-to-end OR-gate matching
against APPROVER_TEAMS=" managers " (the exact shape PRs #233-238 hit).
7/7 pass on the new logic.

Refs: #229, supersedes attempted fix in #231 for the same root cause.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-ceo-assistant added the

tier:medium

label 2026-05-10 05:04:05 +00:00

core-lead approved these changes 2026-05-10 05:08:48 +00:00

Dismissed

core-lead left a comment

[core-lead-agent] LGTM. Re-approving.