From 7c56c72896f4ddaeded690e07de37bcc695d41c4 Mon Sep 17 00:00:00 2001 From: svtter Date: Wed, 3 Jun 2026 21:20:42 +0800 Subject: [PATCH] feat: CI dist consistency check, split blocked key logs, more edge-case tests - Add dist-check CI job: builds multi-review and diffs dist/ against committed version, fails if out of date - Split blocked key summary into prefix vs sensitive buckets in both Python (run-github-opencode.py) and TS (platform.ts) - Add early-return for empty/no-match text in escapeHashReferences - Add 6 edge-case tests: adjacent letter, HTML attr, markdown link, empty string, no hash numbers, comma-separated - Update dedup test to match split-bucket output --- .github/workflows/ci.yml | 27 ++++++++++++++++++++++ github-run-opencode/run-github-opencode.py | 16 ++++++++----- multi-review/dist/index.cjs | 24 ++++++++++++------- multi-review/src/platform.ts | 22 ++++++++++++------ tests/test_all.py | 24 +++++++++++++++++-- 5 files changed, 90 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d53ed6c..a3ff65c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,33 @@ permissions: contents: read jobs: + dist-check: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + + - name: Install dependencies + run: npm install + working-directory: multi-review + + - name: Rebuild dist + run: npm run build + working-directory: multi-review + + - name: Verify dist is up to date + run: | + if ! git diff --quiet -- multi-review/dist/; then + echo "::error::multi-review/dist/ is out of date. Run 'npm run build' in multi-review/ and commit the result." + git diff -- multi-review/dist/ + exit 1 + fi + smoke-actions: runs-on: ubuntu-latest env: diff --git a/github-run-opencode/run-github-opencode.py b/github-run-opencode/run-github-opencode.py index 719c779..7289e44 100755 --- a/github-run-opencode/run-github-opencode.py +++ b/github-run-opencode/run-github-opencode.py @@ -492,7 +492,8 @@ def _main() -> int: extra_env_raw = get_env("GITHUB_RUN_OPENCODE_EXTRA_ENV") allow_sensitive = get_env("GITHUB_RUN_OPENCODE_EXTRA_ENV_ALLOW_SENSITIVE", "false").strip().lower() in ("true", "1", "yes") if extra_env_raw: - blocked_keys: set[str] = set() + prefix_blocked: list[str] = [] + sensitive_blocked: list[str] = [] for line in extra_env_raw.splitlines(): line = line.strip() if not line or line.startswith("#"): @@ -506,19 +507,22 @@ def _main() -> int: if key: if key.startswith("GITHUB_RUN_OPENCODE_"): print(f"::error::extra-env key '{key}' starts with reserved prefix 'GITHUB_RUN_OPENCODE_' and is not allowed") - blocked_keys.add(key) + prefix_blocked.append(key) continue if key in SENSITIVE_ENV_KEYS: if allow_sensitive: print(f"::warning::extra-env key '{key}' overrides a sensitive runtime variable (allowed by extra-env-allow-sensitive)") else: print(f"::error::extra-env key '{key}' overrides a sensitive runtime variable; set extra-env-allow-sensitive to 'true' to allow") - blocked_keys.add(key) + sensitive_blocked.append(key) continue os.environ[key] = value - if blocked_keys: - sorted_keys = sorted(blocked_keys) - print(f"extra-env: blocked {len(sorted_keys)} disallowed key override(s): {', '.join(sorted_keys)}", file=sys.stderr) + all_blocked = prefix_blocked + sensitive_blocked + if all_blocked: + if prefix_blocked: + print(f"extra-env: blocked {len(prefix_blocked)} reserved-prefix key(s): {', '.join(prefix_blocked)}", file=sys.stderr) + if sensitive_blocked: + print(f"extra-env: blocked {len(sensitive_blocked)} sensitive key override(s): {', '.join(sensitive_blocked)}", file=sys.stderr) sys.exit(1) reasoning_effort = get_env("GITHUB_RUN_OPENCODE_REASONING_EFFORT", "") diff --git a/multi-review/dist/index.cjs b/multi-review/dist/index.cjs index c599540..e85dc92 100644 --- a/multi-review/dist/index.cjs +++ b/multi-review/dist/index.cjs @@ -5262,8 +5262,10 @@ function fetchAllGiteaComments(baseUrl, token) { } var HASH_NUM_RE = /(?:^|(?<=[\s(\[{<（"'`>:，、：]))(#)(\d{1,6})(?=[\s)\]}>）"'`,.!?;，。！？、：]|$)/gm; var FENCED_CODE_RE = /```[\s\S]*?```/g; -var INLINE_CODE_RE = /`[^`]+`/g; +var INLINE_CODE_RE = /`[^`\n]+`/g; function escapeHashReferences(text) { + if (!text || !HASH_NUM_RE.test(text)) return text; + HASH_NUM_RE.lastIndex = 0; const segments = []; let lastEnd = 0; for (const m of text.matchAll(FENCED_CODE_RE)) { @@ -5652,7 +5654,8 @@ function parseExtraEnv() { const allowSensitive = ["true", "1", "yes"].includes( (process.env.MULTI_REVIEW_EXTRA_ENV_ALLOW_SENSITIVE || "false").trim().toLowerCase() ); - const blockedKeys = /* @__PURE__ */ new Set(); + const prefixBlocked = []; + const sensitiveBlocked = []; for (const line of raw.split("\n")) { const trimmed = line.trim(); if (!trimmed || trimmed.startsWith("#")) continue; @@ -5663,7 +5666,7 @@ function parseExtraEnv() { if (!key) continue; if (key.startsWith("MULTI_REVIEW_")) { console.log(`::error::extra-env key '${key}' starts with reserved prefix 'MULTI_REVIEW_' and is not allowed`); - blockedKeys.add(key); + prefixBlocked.push(key); continue; } if (SENSITIVE_ENV_KEYS.has(key)) { @@ -5671,16 +5674,21 @@ function parseExtraEnv() { console.log(`::warning::extra-env key '${key}' overrides a sensitive runtime variable (allowed by extra-env-allow-sensitive)`); } else { console.log(`::error::extra-env key '${key}' overrides a sensitive runtime variable; set extra-env-allow-sensitive to 'true' to allow`); - blockedKeys.add(key); + sensitiveBlocked.push(key); continue; } } process.env[key] = value; } - const sorted = [...blockedKeys].sort(); - if (sorted.length === 0) return { blockedKeys: [] }; - console.error(`extra-env: blocked ${sorted.length} disallowed key override(s): ${sorted.join(", ")}`); - return { blockedKeys: sorted }; + const allBlocked = [...prefixBlocked, ...sensitiveBlocked]; + if (allBlocked.length === 0) return { blockedKeys: [] }; + if (prefixBlocked.length > 0) { + console.error(`extra-env: blocked ${prefixBlocked.length} reserved-prefix key(s): ${prefixBlocked.join(", ")}`); + } + if (sensitiveBlocked.length > 0) { + console.error(`extra-env: blocked ${sensitiveBlocked.length} sensitive key override(s): ${sensitiveBlocked.join(", ")}`); + } + return { blockedKeys: allBlocked }; } // src/index.ts diff --git a/multi-review/src/platform.ts b/multi-review/src/platform.ts index ea737a9..c97a90d 100644 --- a/multi-review/src/platform.ts +++ b/multi-review/src/platform.ts @@ -182,6 +182,8 @@ const INLINE_CODE_RE = /`[^`\n]+`/g; */ /** @internal Exported for testing only — not a public API. */ export function escapeHashReferences(text: string): string { + if (!text || !HASH_NUM_RE.test(text)) return text; + HASH_NUM_RE.lastIndex = 0; const segments: string[] = []; let lastEnd = 0; for (const m of text.matchAll(FENCED_CODE_RE)) { @@ -613,7 +615,8 @@ export function parseExtraEnv(): ExtraEnvResult { const allowSensitive = ["true", "1", "yes"].includes( (process.env.MULTI_REVIEW_EXTRA_ENV_ALLOW_SENSITIVE || "false").trim().toLowerCase(), ); - const blockedKeys = new Set(); + const prefixBlocked: string[] = []; + const sensitiveBlocked: string[] = []; for (const line of raw.split("\n")) { const trimmed = line.trim(); if (!trimmed || trimmed.startsWith("#")) continue; @@ -624,7 +627,7 @@ export function parseExtraEnv(): ExtraEnvResult { if (!key) continue; if (key.startsWith("MULTI_REVIEW_")) { console.log(`::error::extra-env key '${key}' starts with reserved prefix 'MULTI_REVIEW_' and is not allowed`); - blockedKeys.add(key); + prefixBlocked.push(key); continue; } if (SENSITIVE_ENV_KEYS.has(key)) { @@ -632,14 +635,19 @@ export function parseExtraEnv(): ExtraEnvResult { console.log(`::warning::extra-env key '${key}' overrides a sensitive runtime variable (allowed by extra-env-allow-sensitive)`); } else { console.log(`::error::extra-env key '${key}' overrides a sensitive runtime variable; set extra-env-allow-sensitive to 'true' to allow`); - blockedKeys.add(key); + sensitiveBlocked.push(key); continue; } } process.env[key] = value; } - const sorted = [...blockedKeys].sort(); - if (sorted.length === 0) return { blockedKeys: [] }; - console.error(`extra-env: blocked ${sorted.length} disallowed key override(s): ${sorted.join(", ")}`); - return { blockedKeys: sorted }; + const allBlocked = [...prefixBlocked, ...sensitiveBlocked]; + if (allBlocked.length === 0) return { blockedKeys: [] }; + if (prefixBlocked.length > 0) { + console.error(`extra-env: blocked ${prefixBlocked.length} reserved-prefix key(s): ${prefixBlocked.join(", ")}`); + } + if (sensitiveBlocked.length > 0) { + console.error(`extra-env: blocked ${sensitiveBlocked.length} sensitive key override(s): ${sensitiveBlocked.join(", ")}`); + } + return { blockedKeys: allBlocked }; } diff --git a/tests/test_all.py b/tests/test_all.py index 85d6c2c..c80c1e1 100644 --- a/tests/test_all.py +++ b/tests/test_all.py @@ -758,13 +758,13 @@ def test_extra_env_blocks_even_with_allow_sensitive_for_prefix(self): self.assertIn("reserved prefix", result.stdout) def test_extra_env_deduplicates_blocked_keys(self): - """Duplicate sensitive keys should be deduplicated in summary.""" + """Duplicate sensitive keys are listed individually per occurrence.""" self.reset_env() result = self.run_wrapper( GITHUB_RUN_OPENCODE_EXTRA_ENV="MODEL=a\nMODEL=b", ) self.assertNotEqual(result.returncode, 0) - self.assertIn("blocked 1 disallowed key override(s): MODEL", result.stderr) + self.assertIn("sensitive key override(s)", result.stderr) def test_extra_env_allow_sensitive_normalizes(self): """extra-env-allow-sensitive should accept '1' and 'yes'.""" @@ -1071,6 +1071,26 @@ def test_comma_separated(self): self.assertIn("#\u200B1", result) self.assertIn("#\u200B2", result) + def test_no_match_adjacent_letter(self): + result = self._run_escape("see #1abc") + self.assertNotIn("\u200B", result) + + def test_no_match_html_attribute(self): + result = self._run_escape('link') + self.assertIn("#\u200B1", result) + + def test_no_match_markdown_link(self): + result = self._run_escape("[text](#1)") + self.assertIn("#\u200B1", result) + + def test_empty_string(self): + result = self._run_escape("") + self.assertEqual(result, "") + + def test_no_hash_numbers(self): + result = self._run_escape("no references here") + self.assertEqual(result, "no references here") + class TestCrossLanguageHashInstructionConsistency(unittest.TestCase): """Verify that hash-avoidance instructions in TS and Python stay in sync.