Skip to content

fix(score): validate PDF bridge byte arrays without coercion - #750

Open
seonghobae wants to merge 17 commits into
developfrom
fix/score-pdf-byte-validation-clean
Open

fix(score): validate PDF bridge byte arrays without coercion#750
seonghobae wants to merge 17 commits into
developfrom
fix/score-pdf-byte-validation-clean

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What

Harden readScorePdf so plain-array bridge responses are accepted only when every element is an integer in the inclusive 0..255 byte range. Validation uses an indexed, short-circuiting loop before Uint8Array.from, preventing implicit wrapping, truncation, and non-finite coercion.

Why

Uint8Array.from silently converts values such as -1, 256, 1.5, NaN, and Infinity. A malformed or compromised desktop bridge response must fail closed instead of being converted into different bytes. The indexed loop additionally avoids predicate callback overhead while preserving O(1) auxiliary validation memory and early termination.

Exact current scope

Exact head: 3e4b88a664bec74074b615e2f410f8ef192fcaae.

Exactly three files differ from protected develop:

  • apps/desktop/src/features/score/scoreStorage.ts
  • apps/desktop/src/features/score/scoreStorage.test.ts
  • CHANGELOG.md

The unrelated Undici lock update was removed; dependency security remains owned by PR #751. No workflow, permission, dependency, nested lockfile, proxy, filesystem authority, network authority, or IPC command is added.

Verification contract

  • valid boundary bytes 0 and 255 are preserved;
  • strings, negative integers, values above 255, fractional values, NaN, and infinity are rejected;
  • validation stops after the first invalid element;
  • bridge-unavailable behavior remains fail-closed;
  • the buyer-visible security behavior is recorded under CHANGELOG.md Unreleased;
  • desktop lint, strict typecheck, complete measured tests, production build, repository CI, security gates, current-head central coverage/review, zero unresolved actionable threads, and qualifying independent approval remain mandatory before merge.

Supersession

This clean review entry supersedes #729. No predecessor-head check or review is accepted for the current exact head.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 317d1269-bb33-4840-b321-9ad0a7e21dc0

📥 Commits

Reviewing files that changed from the base of the PR and between acdbea6 and 3e4b88a.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • apps/desktop/src/features/score/scoreStorage.test.ts
  • apps/desktop/src/features/score/scoreStorage.ts

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 4 minutes and 15 seconds before sending another message.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64 after required checks complete. Verify the three-file scope, fail-closed integer byte validation, first-invalid-element short circuit, bounded Undici remediation, and absence of workflow, permission, nested-lock, filesystem, network, or IPC expansion. Approve only if no blocking finding remains.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64 after the remaining exact-head checks complete. Verify fail-closed integer byte validation before Uint8Array.from, 0/255 boundary preservation, rejection of negative/out-of-range/fractional/non-finite/non-number values, indexed short-circuit behavior, bridge-unavailable handling, the bounded Undici 7.29.0 root-lock change, absence of nested locks or added authority, and current-head measured coverage. Submit APPROVE only if no blocker remains.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='74bb1fe30dc9c12e41e8ec015a64d084213d7b64'
repo='ContextualWisdomLab/bandscope'
pr='750'

echo '=== Checked-out commit ==='
git rev-parse HEAD
echo '=== Expected commit object ==='
git rev-parse "${expected}^{commit}"

echo '=== PR metadata and current head ==='
gh pr view "$pr" --repo "$repo" --json number,url,headRefName,headRefOid,baseRefName,files,commits,statusCheckRollup \
  --jq '{number,url,headRefName,headRefOid,baseRefName,files:[.files[].path],commits:[.commits[].oid],checks:[.statusCheckRollup[]|{name,conclusion,status,detailsUrl}]}'

echo '=== Changed-file names and stat against develop ==='
git diff --name-status develop..."$expected"
git diff --stat develop..."$expected"

echo '=== Full scoped diff ==='
git diff --no-ext-diff --unified=80 develop..."$expected" -- \
  apps/desktop/src/features/score/scoreStorage.ts \
  apps/desktop/src/features/score/scoreStorage.test.ts \
  package-lock.json

echo '=== Relevant source outline ==='
ast-grep outline apps/desktop/src/features/score/scoreStorage.ts --items all
ast-grep outline apps/desktop/src/features/score/scoreStorage.test.ts --items all

echo '=== Nested lockfiles and workflow/authority changes ==='
fd -HI '(^|/)(package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml)$' . \
  -E .git -E node_modules | sort
git diff --name-only develop..."$expected" | rg '(^|/)(\.github/workflows/|package-lock\.json$|npm-shrinkwrap\.json$|yarn\.lock$|pnpm-lock\.yaml$)' || true

echo '=== Undici entries in lockfile ==='
rg -n -C 3 '"node_modules/undici"|"undici":' package-lock.json

Length of output: 26746


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
strix_run='30905262853'
strix_job='91978764354'
coverage_run='30905262795'
coverage_job='91983961980'

echo '=== Failed strix job metadata ==='
gh api "repos/${repo}/actions/jobs/${strix_job}" \
  --jq '{id,name,status,conclusion,started_at,completed_at,html_url,steps:[.steps[]|{name,status,conclusion,number}]}'

echo '=== Failed strix job log tail ==='
gh run view "$strix_run" --repo "$repo" --job "$strix_job" --log-failed 2>&1 | tail -n 250

echo '=== Queued coverage job metadata ==='
gh api "repos/${repo}/actions/jobs/${coverage_job}" \
  --jq '{id,name,status,conclusion,started_at,completed_at,html_url,steps:[.steps[]|{name,status,conclusion,number}]}'

echo '=== Latest PR check states ==='
gh pr view 750 --repo "$repo" --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|select(.name=="strix" or .name=="coverage-source-tree")|{name,status,conclusion,detailsUrl}]}'

Length of output: 5019


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
strix_job='91978764354'
coverage_job='91983961980'

echo '=== Exact PR head and selected checks ==='
gh api "repos/${repo}/commits/74bb1fe30dc9c12e41e8ec015a64d084213d7b64/check-runs?per_page=100" \
  --jq '{head_sha,check_runs:[.check_runs[]|select(.name=="strix" or .name=="coverage-source-tree")|{name,status,conclusion,details_url,started_at,completed_at,output}]}'

echo '=== Coverage job ==='
gh api "repos/${repo}/actions/jobs/${coverage_job}" \
  --jq '{id,name,status,conclusion,started_at,completed_at,html_url,steps:[.steps[]|{name,status,conclusion}]}'

echo '=== Download and inspect Strix job log archive ==='
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
gh api "repos/${repo}/actions/jobs/${strix_job}/logs" > "$tmpdir/strix-logs.zip"
unzip -l "$tmpdir/strix-logs.zip"
unzip -p "$tmpdir/strix-logs.zip" | tail -n 300

Length of output: 2958


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
check_run='91978764354'

echo '=== Strix check annotations ==='
gh api "repos/${repo}/check-runs/${check_run}/annotations?per_page=100" \
  --jq '.[] | {path,start_line,end_line,annotation_level,title,message,raw_details}'

echo '=== Strix job-log endpoint response headers ==='
gh api -i "repos/${repo}/actions/jobs/${check_run}/logs" | sed -n '1,80p'

Length of output: 6266


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
strix_job='91978764354'
tmp="$(mktemp)"
trap 'rm -f "$tmp"' EXIT

gh api "repos/${repo}/actions/jobs/${strix_job}/logs" > "$tmp"

echo '=== Run Strix step and following output ==='
rg -n -A160 -i 'Run Strix \(quick\)|::error|error:|exception|traceback|failed|failure|exit code' "$tmp" \
  | tail -n 400

echo '=== Final 180 log lines ==='
tail -n 180 "$tmp"

Length of output: 67235


@seonghobae I reviewed exact head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64.

The three-file scope is correct. No workflow, permission, nested-lockfile, filesystem, network, or IPC authority change exists.

readScorePdf now validates every plain-array element before Uint8Array.from. The indexed loop rejects non-number values, non-integers, negative values, values above 255, NaN, and infinity. The tests cover 0, 255, invalid values, first-invalid-element short circuit, and bridge-unavailable handling.

The root lockfile changes undici only from 7.28.0 to 7.29.0.

I cannot approve yet:

  • coverage-source-tree is still queued.
  • strix failed with a MEDIUM finding about frontend validation of projectId, songId, and scoreId. The direct IPC forwarding predates this PR, but the failed required check must be triaged and resolved or formally accepted, then rerun.

The successful build, test, audit, and other security checks are insufficient while these two conditions remain unresolved.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. Repository CI, security, SBOM, audit, Bandit, Semgrep, and release workflows for this head have succeeded. Do not reuse approval evidence from superseded PR #729 or another SHA.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. Repository-local CI, build, release, SAST, Bandit, secret scan, security audit, security scan, and SBOM runs are successful; approve only if the same-head coverage and review contracts are satisfied.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64.

  • Head SHA: 74bb1fe30dc9c12e41e8ec015a64d084213d7b64

  • Workflow run: 30981152078

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 3e4b88a664bec74074b615e2f410f8ef192fcaae
  • Workflow run: 31193392189
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 3e4b88a664bec74074b615e2f410f8ef192fcaae.

  • Head SHA: 3e4b88a664bec74074b615e2f410f8ef192fcaae

  • Workflow run: 31193392189

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 5, 2026 09:20

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. Repository CI, npm audit, release, SBOM, SAST, secret, Bandit, and security workflows all succeeded on this head. Focus on fail-closed integer byte validation, first-invalid-element short circuiting, bridge-unavailable behavior, and the bounded Undici 7.29.0 root-lock change.

@opencode-agent Please rerun review for this exact head after the central npm-workspace coverage materializer is available; do not reuse the failed predecessor coverage-evidence result.

@cwl-noema-review Please independently review this exact head. Merge remains subject to protected current-head checks and a qualifying non-author approval.

@seonghobae
seonghobae enabled auto-merge (squash) August 5, 2026 12:02
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
sha='74bb1fe30dc9c12e41e8ec015a64d084213d7b64'

echo '=== Local and requested commit ==='
git rev-parse HEAD
git rev-parse "${sha}^{commit}"

echo '=== PR metadata and exact-head checks ==='
gh pr view 750 --repo "$repo" --json number,url,headRefOid,baseRefName,files,statusCheckRollup \
  --jq '{number,url,headRefOid,baseRefName,files:[.files[].path],checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'
gh api "repos/${repo}/commits/${sha}/check-runs?per_page=100" \
  --jq '{head_sha,checks:[.check_runs[]|{name,status,conclusion,details_url}]}'

echo '=== Changed-file scope against develop ==='
git diff --name-status "develop...${sha}"
git diff --stat "develop...${sha}"

echo '=== Scoped diff ==='
git diff --no-ext-diff --unified=100 "develop...${sha}" -- \
  apps/desktop/src/features/score/scoreStorage.ts \
  apps/desktop/src/features/score/scoreStorage.test.ts \
  package-lock.json

echo '=== Relevant file outlines ==='
ast-grep outline apps/desktop/src/features/score/scoreStorage.ts --items all
ast-grep outline apps/desktop/src/features/score/scoreStorage.test.ts --items all

echo '=== Nested lockfiles ==='
fd -HI '(^|/)(package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml)$' . \
  -E .git -E node_modules | sort

echo '=== Lockfile undici dependency context ==='
rg -n -C 5 '"node_modules/undici"|"undici":' package-lock.json

Length of output: 36079


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
sha='74bb1fe30dc9c12e41e8ec015a64d084213d7b64'
strix_job='91978764354'
coverage_job='91987916426'

echo '=== Strix annotations on the exact-head check ==='
gh api "repos/${repo}/check-runs/${strix_job}/annotations?per_page=100" \
  --jq '.[] | {path,start_line,end_line,annotation_level,title,message,raw_details}'

echo '=== Strix failed-log excerpts ==='
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
gh api "repos/${repo}/actions/jobs/${strix_job}/logs" > "$tmpdir/strix.log"
rg -n -i -C 12 '::error|medium|projectId|songId|scoreId|finding|failed|failure|exit code' \
  "$tmpdir/strix.log" | tail -n 320

echo '=== Coverage-evidence job steps and log excerpts ==='
gh api "repos/${repo}/actions/jobs/${coverage_job}" \
  --jq '{name,status,conclusion,started_at,completed_at,steps:[.steps[]|{name,status,conclusion}]}'
gh api "repos/${repo}/actions/jobs/${coverage_job}/logs" > "$tmpdir/coverage.log"
rg -n -i -C 8 'scoreStorage|coverage|test|pass|fail|threshold' \
  "$tmpdir/coverage.log" | tail -n 320

echo '=== Exact-head commit parents ==='
gh api "repos/${repo}/commits/${sha}" --jq '{sha,parents:[.parents[].sha]}'

Length of output: 1845


🏁 Script executed:

#!/bin/bash
set -u

repo='ContextualWisdomLab/bandscope'
strix_job='91978764354'
coverage_job='91987916426'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

echo '=== Strix job metadata ==='
gh api "repos/${repo}/actions/jobs/${strix_job}" \
  --jq '{name,status,conclusion,started_at,completed_at,steps:[.steps[]|{number,name,status,conclusion}]}'

echo '=== Strix failed-log excerpts ==='
if gh api "repos/${repo}/actions/jobs/${strix_job}/logs" > "$tmpdir/strix.log"; then
  rg --allow-escape-sequences -n -i -C 12 \
    '::error|medium|projectId|songId|scoreId|finding|failed|failure|exit code|strix' \
    "$tmpdir/strix.log" | tail -n 400 || true
else
  echo 'Could not download Strix logs.'
fi

echo '=== Coverage-evidence job metadata ==='
gh api "repos/${repo}/actions/jobs/${coverage_job}" \
  --jq '{name,status,conclusion,started_at,completed_at,steps:[.steps[]|{number,name,status,conclusion}]}'

echo '=== Coverage-evidence log excerpts ==='
if gh api "repos/${repo}/actions/jobs/${coverage_job}/logs" > "$tmpdir/coverage.log"; then
  rg --allow-escape-sequences -n -i -C 8 \
    'scoreStorage|coverage|test|pass|fail|threshold|materializ' \
    "$tmpdir/coverage.log" | tail -n 400 || true
else
  echo 'Could not download coverage-evidence logs.'
fi

Length of output: 5735


🏁 Script executed:

#!/bin/bash
set -u

repo='ContextualWisdomLab/bandscope'
coverage_run='30905262795'
coverage_job='91983961980'
ci_run='30905263479'
ci_job='91978765682'

echo '=== Exact-head coverage-source-tree job ==='
gh api "repos/${repo}/actions/jobs/${coverage_job}" \
  --jq '{name,status,conclusion,started_at,completed_at,steps:[.steps[]|{number,name,status,conclusion}]}'

echo '=== Exact-head build-and-test job ==='
gh api "repos/${repo}/actions/jobs/${ci_job}" \
  --jq '{name,status,conclusion,started_at,completed_at,steps:[.steps[]|{number,name,status,conclusion}]}'

echo '=== coverage-source-tree log excerpts ==='
gh run view "$coverage_run" --repo "$repo" --job "$coverage_job" --log 2>&1 \
  | rg --allow-escape-sequences -n -i -C 6 \
      'scoreStorage|coverage|test|pass|fail|threshold|materializ' \
  | tail -n 300 || true

echo '=== build-and-test log excerpts ==='
gh run view "$ci_run" --repo "$repo" --job "$ci_job" --log 2>&1 \
  | rg --allow-escape-sequences -n -i -C 6 \
      'scoreStorage|vitest|test files|tests|passed|failed' \
  | tail -n 300 || true

Length of output: 3550


🏁 Script executed:

#!/bin/bash
set -u

repo='ContextualWisdomLab/bandscope'
ci_run='30905263479'
ci_job='91978765682'

echo '=== Candidate workflow files ==='
fd -HI -t f .github | sort

echo '=== quickcheck workflow definition ==='
rg -n -i -C 12 'quickcheck|build-and-test|vitest|scoreStorage' .github 2>/dev/null || true

echo '=== Text-only CI log excerpts ==='
gh run view "$ci_run" --repo "$repo" --job "$ci_job" --log 2>&1 \
  | sed -E $'s/\x1B\\[[0-?]*[ -\\/]*[`@-`~]//g' \
  | grep -a -i -E -C 6 'quickcheck|scoreStorage|vitest|test files|tests.*passed|tests.*failed' \
  | tail -n 300 || true

Length of output: 17271


@seonghobae I reviewed exact head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64.

I found no blocking defect in the three changed files.

  • The scope is limited to the two score-storage files and the root package-lock.json.
  • readScorePdf validates each plain-array value before Uint8Array.from.
  • The indexed loop rejects non-integers, negative values, values above 255, NaN, infinity, and non-number values.
  • The loop stops at the first invalid value.
  • The tests preserve 0 and 255, cover all specified rejected values, verify short-circuiting, and retain bridge-unavailable failure behavior.
  • Exact-head CI ran scoreStorage.test.ts: 9 tests passed.
  • Exact-head desktop coverage reports scoreStorage.ts at 100% statements, branches, functions, and lines.
  • The lockfile changes only root undici, from 7.28.0 to 7.29.0.
  • No nested lockfile or workflow, permission, filesystem, network, or IPC-authority expansion exists.

I cannot approve while the exact-head strix check remains failed. Its available annotation reports only “Process completed with exit code 1.” Resolve, formally accept, or rerun that check under repository policy before approval.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@opencode-agent
opencode-agent Bot disabled auto-merge August 5, 2026 13:24

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review

Please perform a fresh exact-head review of 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. Every repository-owned exact-head workflow is successful, including CI, measured tests, production build, security-audit/npm audit, Security Scan, Semgrep, Bandit, secret scan, SBOM, release, and build-baseline; there are zero inline review threads. Verify strict integer 0..255 byte-domain validation, early exit after the first invalid element, preservation of valid boundary bytes, bridge-unavailable fail-closed behavior, and the bounded Undici 7.29.0 root-lock security update. The prior coverage-evidence review was an infrastructure result, not a product-source finding; do not reuse it without re-evaluating the current central coverage path.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently re-review exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. All repository-local CI, build, release, npm/Python/Rust security audit, Bandit, Semgrep, Security Scan, secret scan, and SBOM workflows succeeded on this SHA; there are no unresolved inline threads. The remaining central coverage failure is the npm-workspace lock-owner prerequisite tracked in ContextualWisdomLab/.github#748.

@seonghobae
seonghobae enabled auto-merge (squash) August 6, 2026 08:59
@opencode-agent
opencode-agent Bot disabled auto-merge August 6, 2026 10:02

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review
@cwl-noema-review
@opencode-agent

Please review the exact live head only. Verify PDF bridge arrays accept only integers 0..255 before Uint8Array construction, reject strings/negative/out-of-range/fractional/NaN/infinite values, short-circuit after the first invalid element, preserve valid boundary bytes and bridge-unavailable fail-closed behavior, and retain the bounded root-lock Undici update without nested locks. Require current desktop lint, strict typecheck, complete measured tests and coverage, production build, npm audit, security gates, and zero unresolved threads. Review only; do not update the branch, merge, release, or alter protections.

@seonghobae
seonghobae enabled auto-merge (squash) August 7, 2026 00:31
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
pr='750'

echo '=== Pull request live head, changed files, and checks ==='
gh pr view "$pr" --repo "$repo" \
  --json number,url,headRefOid,baseRefName,files,statusCheckRollup,reviews \
  --jq '{
    number,url,headRefOid,baseRefName,
    files:[.files[].path],
    checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}],
    reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}]
  }'

sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
echo "=== Live SHA: ${sha} ==="
git rev-parse HEAD
git rev-parse "${sha}^{commit}"

echo '=== Diff scope against develop ==='
git diff --name-status "develop...${sha}"
git diff --stat "develop...${sha}"

echo '=== Changed source and test diff ==='
git diff --no-ext-diff --unified=100 "develop...${sha}" -- \
  apps/desktop/src/features/score/scoreStorage.ts \
  apps/desktop/src/features/score/scoreStorage.test.ts

echo '=== Lockfile diff and Undici context ==='
git diff --no-ext-diff --unified=25 "develop...${sha}" -- package-lock.json
rg -n -C 4 '"node_modules/undici"|"undici":' package-lock.json

echo '=== Changed-file outlines ==='
ast-grep outline apps/desktop/src/features/score/scoreStorage.ts --items all
ast-grep outline apps/desktop/src/features/score/scoreStorage.test.ts --items all

echo '=== Lockfiles in repository ==='
fd -HI '(^|/)(package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml)$' . \
  -E .git -E node_modules | sort

echo '=== Open review-thread count ==='
gh api graphql -F owner='ContextualWisdomLab' -F name='bandscope' -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes { isResolved }
      }
    }
  }
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)] | length'

echo '=== Exact-head check runs ==='
gh api "repos/${repo}/commits/${sha}/check-runs?per_page=100" \
  --jq '{head_sha,checks:[.check_runs[]|{name,status,conclusion,details_url}]}'

Length of output: 31722


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/bandscope'
pr='750'
sha='74bb1fe30dc9c12e41e8ec015a64d084213d7b64'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

echo '=== Pull request reviews and review comments ==='
gh pr view "$pr" --repo "$repo" --json reviews,comments \
  --jq '{
    reviews:[.reviews[]|{author:.author.login,state,body,submittedAt,commit:.commit.oid}],
    comments:[.comments[]|{author:.author.login,body,createdAt}]
  }'

echo '=== Strix annotations and job steps ==='
gh api "repos/${repo}/check-runs/91978764354/annotations?per_page=100" \
  --jq '.[]|{path,start_line,end_line,annotation_level,title,message,raw_details}'
gh api "repos/${repo}/actions/jobs/91978764354" \
  --jq '{name,status,conclusion,steps:[.steps[]|{number,name,status,conclusion}]}'

echo '=== Strix failure excerpts ==='
gh api "repos/${repo}/actions/jobs/91978764354/logs" > "$tmpdir/strix.log"
rg --text -n -i -C 10 \
  '::error|error:|warning:|medium|high|critical|finding|projectId|songId|scoreId|exit code|failed|failure' \
  "$tmpdir/strix.log" | tail -n 350 || true

for job in \
  91978765682 \
  91983961980 \
  91987916426 \
  91978766074 \
  91978765893 \
  91978765874 \
  91978765928 \
  91978765827 \
  91978765562
do
  echo "=== Job ${job}: metadata ==="
  gh api "repos/${repo}/actions/jobs/${job}" \
    --jq '{name,status,conclusion,steps:[.steps[]|{number,name,status,conclusion}]}'
  gh api "repos/${repo}/actions/jobs/${job}/logs" > "$tmpdir/${job}.log"
  echo "=== Job ${job}: relevant log lines ==="
  rg --text -n -i -C 4 \
    'lint|typecheck|type check|vitest|test files|tests.*pass|coverage|scoreStorage|build|audit|vulnerabilit|npm error|fail|error' \
    "$tmpdir/${job}.log" | tail -n 180 || true
done

Length of output: 34151


🏁 Script executed:

#!/bin/bash
set -u -o pipefail

repo='ContextualWisdomLab/bandscope'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

fetch_log() {
  local job="$1"
  gh api "repos/${repo}/actions/jobs/${job}/logs" > "${tmpdir}/${job}.raw" || return 0
  sed -E $'s/\x1B\\[[0-?]*[ -\\/]*[`@-`~]//g' "${tmpdir}/${job}.raw" > "${tmpdir}/${job}.log"
}

echo '=== Strix failed step output ==='
fetch_log '91978764354'
grep -a -i -E -C 12 \
  'Run Strix \(quick\)|projectId|songId|scoreId|finding|medium|high|critical|error|failed|exit code' \
  "${tmpdir}/91978764354.log" | tail -n 450 || true

for job in 91978765682 91983961980 91987916426 91978766074 91978765562; do
  echo "=== Job ${job}: status and relevant evidence ==="
  gh api "repos/${repo}/actions/jobs/${job}" \
    --jq '{name,status,conclusion,steps:[.steps[]|{name,conclusion}]}' || true
  fetch_log "$job"
  grep -a -i -E -C 5 \
    'lint|typecheck|type check|vitest|test files|tests.*passed|coverage|scoreStorage|production build|build complete|npm audit|vulnerabilit|found 0|fail|error' \
    "${tmpdir}/${job}.log" | tail -n 260 || true
done

Length of output: 6127


@seonghobae I reviewed live head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64.

I found no blocking defect in the three changed files.

  • readScorePdf validates each plain-array element before Uint8Array.from.
  • The indexed loop accepts only integer values from 0 through 255.
  • It rejects strings, negative values, values above 255, fractional values, NaN, and infinity.
  • The loop exits at the first invalid element.
  • The tests preserve 0 and 255.
  • The tests verify invalid-value rejection, early exit, and bridge-unavailable fail-closed behavior.
  • The root package-lock.json changes undici only from 7.28.0 to 7.29.0.
  • No nested lockfile exists.
  • No workflow, permission, filesystem, network, or IPC-authority change exists.
  • The pull request has zero unresolved review threads.
  • The exact-head build-and-test, audit, and listed security jobs report success.

I cannot approve this head.

  • The exact-head strix check is failed at Run Strix (quick). Its available annotation reports: Process completed with exit code 1.
  • The successful coverage-source-tree and coverage-evidence jobs do not contain measured coverage output. Their steps only delegate coverage execution or preserve a branch-protection status context.

Resolve or formally accept the failed strix check under repository policy. Provide current-head measured coverage evidence before approval.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@opencode-agent
opencode-agent Bot disabled auto-merge August 7, 2026 03:16

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The byte-validation implementation and short-circuit regression are appropriately fail-closed. Before the current-head review rerun, remove the unrelated package-lock.json change because dedicated PR #751 owns the Undici security update, and add an Unreleased CHANGELOG entry describing the bridge-byte validation. This should finish as a focused three-file product slice: production code, direct tests, and CHANGELOG. Rerun exact-head desktop lint, strict typecheck, measured tests, build, repository CI/security, and central evidence after the central validator repair; predecessor-head results are not transferable.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent address

Clean exact current head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64; stop without writing if the live head differs.

Keep the verified readScorePdf integer/range validation and its direct short-circuit tests. Restore package-lock.json exactly from protected develop because #751 is the sole Undici security dependency PR. Add one concise Unreleased CHANGELOG.md entry for rejecting coercible or malformed plain-array bridge bytes. The final base-to-head paths must be exactly apps/desktop/src/features/score/scoreStorage.ts, apps/desktop/src/features/score/scoreStorage.test.ts, and CHANGELOG.md.

Run desktop lint, strict typecheck, complete measured tests, production build, and repository quickcheck before committing normally to this branch. Do not create another branch, PR, workflow, helper, patch artifact, dependency update, or unrelated refactor. Fresh central current-head review is required after the central npm-lock validator repair.

@seonghobae
seonghobae enabled auto-merge (squash) August 7, 2026 10:17

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 3e4b88a664bec74074b615e2f410f8ef192fcaae.

  • Head SHA: 3e4b88a664bec74074b615e2f410f8ef192fcaae

  • Workflow run: 31193392189

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant