Commit Graph

4 Commits

Author SHA1 Message Date
c74c0a0283 fix(ci): add jq install to review-check-tests workflow + fix /tmp/jq hardcode
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 14s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 14s
CI / Detect changes (pull_request) Successful in 25s
review-check-tests / review-check.sh regression tests (pull_request) Successful in 15s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 32s
E2E API Smoke Test / detect-changes (pull_request) Successful in 34s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 34s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 17s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 30s
qa-review / approved (pull_request) Failing after 17s
security-review / approved (pull_request) Failing after 16s
sop-tier-check / tier-check (pull_request) Successful in 18s
gate-check-v3 / gate-check (pull_request) Successful in 27s
CI / Platform (Go) (pull_request) Successful in 7s
CI / Canvas (Next.js) (pull_request) Successful in 7s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 5s
CI / Python Lint & Test (pull_request) Successful in 6s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 6s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 9s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 6s
CI / all-required (pull_request) Successful in 3s
Two fixes found during first CI run:

1. Workflow missing jq installation step — T12 jq-filter test needs jq
   which is not in the Gitea Actions ubuntu-latest runner image.
   Add the same install dance as sop-tier-check.yml (apt-get first,
   GitHub binary download fallback, infra#241 belt-and-suspenders).

2. test_review_check.sh hardcodes /tmp/jq in T12. In CI jq gets
   installed to /usr/bin/jq via apt-get. Fix: use `command -v jq` to
   resolve from PATH first, fall back to /tmp/jq for local dev.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-12 01:24:24 +00:00
43cc27ade5 test(ci): add bats-style integration tests for review-check.sh (#540)
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 23s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 13s
CI / Detect changes (pull_request) Successful in 1m6s
gate-check-v3 / gate-check (pull_request) Successful in 26s
E2E API Smoke Test / detect-changes (pull_request) Successful in 1m6s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 1m2s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 1m3s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 1m0s
qa-review / approved (pull_request) Failing after 20s
security-review / approved (pull_request) Failing after 17s
sop-tier-check / tier-check (pull_request) Successful in 23s
CI / Canvas (Next.js) (pull_request) Successful in 9s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 6s
CI / Python Lint & Test (pull_request) Successful in 6s
CI / Platform (Go) (pull_request) Successful in 7s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 15s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 11s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 10s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 8s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 5s
Add 13 test cases (22 assertions) covering all key paths:
- open/closed PR handling
- non-author APPROVED review detection
- dismissed review exclusion
- team membership probe (204 member, 404 not-member, 403 fail-closed)
- missing GITEA_TOKEN exits 1
- CURL_AUTH_FILE mode 600 and header format
- jq filter correctness

Uses a Python HTTP fixture server that reads scenario from a temp
state dir, with a curl shim rewriting https://fixture.local/* to
http://127.0.0.1:{port}/*.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 20:33:14 +00:00
claude-ceo-assistant
2d096aa7ae feat(ci): sop-tier-check refire workflow via issue_comment (internal#292)
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 28s
Harness Replays / detect-changes (pull_request) Failing after 15s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 14s
Harness Replays / Harness Replays (pull_request) Has been skipped
CI / Detect changes (pull_request) Successful in 59s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 18s
E2E API Smoke Test / detect-changes (pull_request) Successful in 1m5s
sop-tier-check / tier-check (pull_request) Successful in 19s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 59s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 1m10s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 54s
CI / Platform (Go) (pull_request) Successful in 11s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 10s
CI / Python Lint & Test (pull_request) Successful in 8s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 10s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 7s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 9s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 9m10s
CI / Canvas (Next.js) (pull_request) Failing after 10m31s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
## Why

Gitea 1.22.6's `pull_request_review` event doesn't refire workflows
(go-gitea/gitea#33700). The existing sop-tier-check workflow subscribes
to the review event, but the subscription is silently dead. When an
approving review lands AFTER tier-check ran on PR-open/synchronize, the
PR's `sop-tier-check / tier-check (pull_request)` status stays at
failure forever, forcing the orchestrator down the admin force-merge
path (audited via audit-force-merge.yml, but the audit trail keeps
growing — see feedback_never_admin_merge_bypass).

## What

New `.gitea/workflows/sop-tier-refire.yml` listening on `issue_comment`
events. When a repo MEMBER/OWNER/COLLABORATOR comments
`/refire-tier-check` on a PR, the workflow re-invokes the canonical
sop-tier-check.sh and POSTs the resulting status directly to the PR
head SHA (no empty commit, no git history bloat, no cascade re-fire of
every other workflow).

## Security model

Three gates in the workflow `if:` expression — all required:

1. `github.event.issue.pull_request != null` — comment is on a PR, not
   a plain issue.
2. `author_association` ∈ {MEMBER, OWNER, COLLABORATOR} — only repo
   collaborators+ can flip the status (per the internal#292 core-security
   review#1066 ask).
3. Comment body contains `/refire-tier-check` — slash-command-shaped,
   not just any word in normal review prose.

Workflow does NOT check out PR HEAD; only HTTP-calls the Gitea API.
Same trust boundary as sop-tier-check.yml's `pull_request_target`.

## DRY: re-uses sop-tier-check.sh

Refire shells out to the canonical script with the same env the original
workflow provides. We get the EXACT AND-composition gate, not a
watered-down approving-count check.

## Rate-limit

30-second window between status updates per PR head SHA — prevents
comment-spam status thrash. Override via SOP_REFIRE_RATE_LIMIT_SEC or
disable for tests via SOP_REFIRE_DISABLE_RATE_LIMIT=1.

## Tests

`.gitea/scripts/tests/test_sop_tier_refire.sh` — 23 assertions across
T1-T7 covering: success POST, failure POST, no-op on closed, rate-limit
skip, plus YAML-level checks of all three security gates. Real script
runs against a local-fixture HTTP server (`_refire_fixture.py`) with a
mock tier-check (`_mock_tier_check.sh`) — the latter sidesteps the
known bash 3.2 (macOS dev) parser bug on `declare -A`; Linux Gitea
runners (bash 4/5) use the real sop-tier-check.sh in production.

Hostile self-review verified:
- Tests FAIL on absent code (exit 1, FAIL=2 PASS=0 in existence-block).
- Tests FAIL on swapped success/failure label (exit 1).
- Tests PASS on correct code (exit 0, 23/23).

## Brief-falsification log

(a) Keep using force_merge — no, this is the issue being closed.
(b) Empty-commit re-trigger — no, status-POST is cleaner + faster +
    doesn't bloat git history.
(c) author_association check in the script not the workflow — both work
    but workflow-level short-circuits faster (saves runner spin).
(d) Re-implement a watered-down tier-check inside refire — no, that's a
    security regression (skips team-membership AND-composition).
    Refire shells out to the canonical script.

Tier: tier:high (unblocks approved-PR-backlog drain class).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 02:44:31 -07:00
dev-lead
b75187d11c fix(sop-tier-check): clause splitter strips newlines, OR-set collapses to one token (#229)
Some checks failed
sop-tier-check / tier-check (pull_request) Failing after 5s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
audit-force-merge / audit (pull_request) Successful in 4s
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>
2026-05-09 22:03:12 -07:00