Skip to content

fix(review): paginate check-runs in findPreviewUrlFromChecks - #7867

Closed
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:fix/preview-url-checks-pagination-7779
Closed

fix(review): paginate check-runs in findPreviewUrlFromChecks#7867
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:fix/preview-url-checks-pagination-7779

Conversation

@bohdansolovie

@bohdansolovie bohdansolovie commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Walk check-runs via findAcrossPages in findPreviewUrlFromChecks, matching getPreviewBuildState / findPreviewUrlFromPrComments.
  • Add regression coverage for a preview URL that only appears on page 2, early-exit when page 1 already has it, failed-run skips, and output.summary / output.text extraction.

Closes #7779

Scope

  • src/review/visual/preview-url.ts
  • test/unit/preview-url.test.ts

Validation

  • vitest run test/unit/preview-url.test.ts
  • git diff --check

Safety

  • Read-only GitHub list pagination; no auth or write-path changes.

@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 21, 2026 15:28
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 21, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.83%. Comparing base (283fb8f) to head (3650857).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7867      +/-   ##
==========================================
- Coverage   88.85%   88.83%   -0.03%     
==========================================
  Files          88       89       +1     
  Lines       21214    21352     +138     
  Branches     3675     3722      +47     
==========================================
+ Hits        18849    18967     +118     
- Misses       2187     2196       +9     
- Partials      178      189      +11     
Flag Coverage Δ
shard-1 44.92% <42.85%> (?)
shard-2 2.17% <0.00%> (?)
shard-3 76.81% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/visual/preview-url.ts 85.50% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Important

🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪

🔍 LoopOver is reviewing…

AI analysis is in progress. This comment will update when the review is complete.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing

@bohdansolovie
bohdansolovie force-pushed the fix/preview-url-checks-pagination-7779 branch from 903e58c to 857bc2e Compare July 21, 2026 15:46
@bohdansolovie
bohdansolovie force-pushed the fix/preview-url-checks-pagination-7779 branch from 857bc2e to fa4f98f Compare July 21, 2026 15:47
Reuse findAcrossPages the same way getPreviewBuildState already does, so a
preview URL on page 2+ of check-runs is still discovered.

Closes JSONbored#7779
@bohdansolovie
bohdansolovie force-pushed the fix/preview-url-checks-pagination-7779 branch from fa4f98f to 3650857 Compare July 21, 2026 15:48

@superagent-security superagent-security 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.

Superagent found 7 security concern(s).

Comment thread scripts/deploy-selfhost-image.sh Outdated
exit 1
fi
mapfile -t compose_args <<< "$compose_args_raw"
mapfile -t compose_args < <(compose_file_args)

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.

P2: Image deploy script reverts error handling for missing compose files

Reverts checked assignment so missing compose file failures are silently swallowed.

Restore the checked assignment pattern to propagate compose file errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="scripts/deploy-selfhost-image.sh">
<violation number="1" location="scripts/deploy-selfhost-image.sh:111">
<priority>P2</priority>
<title>Image deploy script reverts error handling for missing compose files</title>
<evidence>Reverted from a checked assignment pattern (if ! compose_args_raw=&quot;$(compose_file_args)&quot;; then exit 1; fi) back to mapfile -t compose_args &lt; &lt;(compose_file_args). Process substitution runs compose_file_args in a subshell whose non-zero exit is swallowed by mapfile (mapfile always returns 0), so a missing compose file no longer aborts the script. The script continues with an empty compose_args array, causing docker compose to fall back to default files instead of the configured ones.</evidence>
<recommendation>Restore the checked assignment pattern so compose_file_args failures propagate and abort the script before docker compose runs with incorrect arguments.</recommendation>
</violation>
</file>

Comment thread scripts/deploy-selfhost-prebuilt.sh Outdated
exit 1
fi
mapfile -t compose_args <<< "$compose_args_raw"
mapfile -t compose_args < <(compose_file_args)

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.

P2: Prebuilt deploy script reverts error handling for missing compose files

Reverts checked assignment so missing compose file failures are silently swallowed.

Restore the checked assignment pattern to propagate compose file errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="scripts/deploy-selfhost-prebuilt.sh">
<violation number="1" location="scripts/deploy-selfhost-prebuilt.sh:107">
<priority>P2</priority>
<title>Prebuilt deploy script reverts error handling for missing compose files</title>
<evidence>Reverted from a checked assignment pattern (if ! compose_args_raw=&quot;$(compose_file_args)&quot;; then exit 1; fi) back to mapfile -t compose_args &lt; &lt;(compose_file_args). Process substitution runs compose_file_args in a subshell whose non-zero exit is swallowed by mapfile (mapfile always returns 0), so a missing compose file no longer aborts the script. The script continues with an empty compose_args array, causing docker compose to fall back to default files instead of the configured ones.</evidence>
<recommendation>Restore the checked assignment pattern so compose_file_args failures propagate and abort the script before docker compose runs with incorrect arguments.</recommendation>
</violation>
</file>

Comment thread scripts/selfhost-post-update-check.sh Outdated
exit 1
fi
mapfile -t compose_args <<< "$compose_args_raw"
mapfile -t compose_args < <(compose_file_args)

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.

P2: Post-update check script reverts error handling for missing compose files

Reverts checked assignment so missing compose file failures are silently swallowed.

Restore the checked assignment pattern to propagate compose file errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="scripts/selfhost-post-update-check.sh">
<violation number="1" location="scripts/selfhost-post-update-check.sh:25">
<priority>P2</priority>
<title>Post-update check script reverts error handling for missing compose files</title>
<evidence>Reverted from a checked assignment pattern (if ! compose_args_raw=&quot;$(compose_file_args)&quot;; then exit 1; fi) back to mapfile -t compose_args &lt; &lt;(compose_file_args). Process substitution runs compose_file_args in a subshell whose non-zero exit is swallowed by mapfile (mapfile always returns 0), so a missing compose file no longer aborts the script. The script continues with an empty compose_args array, causing docker compose to fall back to default files instead of the configured ones.</evidence>
<recommendation>Restore the checked assignment pattern so compose_file_args failures propagate and abort the script before docker compose runs with incorrect arguments.</recommendation>
</violation>
</file>

exit 1
fi
mapfile -t compose_args <<< "$compose_args_raw"
mapfile -t compose_args < <(compose_file_args)

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.

P2: Regression gate script reverts error handling for missing compose files

Reverts checked assignment so missing compose file failures are silently swallowed.

Restore the checked assignment pattern to propagate compose file errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="scripts/selfhost-post-update-regression-gate.sh">
<violation number="1" location="scripts/selfhost-post-update-regression-gate.sh:44">
<priority>P2</priority>
<title>Regression gate script reverts error handling for missing compose files</title>
<evidence>Reverted from a checked assignment pattern (if ! compose_args_raw=&quot;$(compose_file_args)&quot;; then exit 1; fi) back to mapfile -t compose_args &lt; &lt;(compose_file_args). Process substitution runs compose_file_args in a subshell whose non-zero exit is swallowed by mapfile (mapfile always returns 0), so a missing compose file no longer aborts the script. The script continues with an empty compose_args array, causing docker compose to fall back to default files instead of the configured ones.</evidence>
<recommendation>Restore the checked assignment pattern so compose_file_args failures propagate and abort the script before docker compose runs with incorrect arguments.</recommendation>
</violation>
</file>

Comment thread src/queue/ai-review-orchestration.ts Outdated
console.error(
JSON.stringify({
level: "error",
level: "warn",

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.

P2: AI review orchestration downgrades error log level to warn

Downgrades structured log level from error to warn in a console.error sink.

Revert to level:error so Sentry and error trackers classify these as errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/queue/ai-review-orchestration.ts">
<violation number="1" location="src/queue/ai-review-orchestration.ts:883">
<priority>P2</priority>
<title>AI review orchestration downgrades error log level to warn</title>
<evidence>Changed console.error structured log payload from level:&quot;error&quot; to level:&quot;warn&quot; in the ai_review_failed catch block. The deleted regression test (queue-console-error-level.test.ts) explicitly enforced this rule because Sentry severity follows the explicit level field over the sink default. Downgrading error logs to warnings hides queue processing failures from error alerting.</evidence>
<recommendation>Revert to level:&quot;error&quot; in console.error sinks so error-tracking systems correctly classify queue processing failures and trigger alerts.</recommendation>
</violation>
</file>

Comment thread src/queue/slop-detection.ts Outdated
console.error(
JSON.stringify({
level: "error",
level: "warn",

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.

P2: Slop detection downgrades error log level to warn

Downgrades structured log level from error to warn in a console.error sink.

Revert to level:error so Sentry and error trackers classify these as errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/queue/slop-detection.ts">
<violation number="1" location="src/queue/slop-detection.ts:175">
<priority>P2</priority>
<title>Slop detection downgrades error log level to warn</title>
<evidence>Changed console.error structured log payload from level:&quot;error&quot; to level:&quot;warn&quot; in the ai_slop_failed catch block. The deleted regression test (queue-console-error-level.test.ts) explicitly enforced this rule because Sentry severity follows the explicit level field over the sink default. Downgrading error logs to warnings hides queue processing failures from error alerting.</evidence>
<recommendation>Revert to level:&quot;error&quot; in console.error sinks so error-tracking systems correctly classify queue processing failures and trigger alerts.</recommendation>
</violation>
</file>

Comment thread src/queue/processors.ts Outdated
console.error(
JSON.stringify({
level: "error",
level: "warn",

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.

P2: Queue processors downgrades error log level to warn

Downgrades structured log level from error to warn in a console.error sink.

Revert to level:error so Sentry and error trackers classify these as errors.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/queue/processors.ts">
<violation number="1" location="src/queue/processors.ts:808">
<priority>P2</priority>
<title>Queue processors downgrades error log level to warn</title>
<evidence>Changed 25+ console.error structured log payloads from level:&quot;error&quot; to level:&quot;warn&quot; across the queue processors file, including failure events like decision_pack_login_failed, sweep_mark_regated_failed, pr_public_surface_failed, agent_maintenance_failed, gate_check_summary_upsert_failed, and others. The deleted regression test (queue-console-error-level.test.ts) explicitly enforced this rule because Sentry severity follows the explicit level field over the sink default. Downgrading error logs to warnings hides queue processing failures from error alerting.</evidence>
<recommendation>Revert all console.error sinks in the queue pipeline to level:&quot;error&quot; so error-tracking systems correctly classify failures and trigger alerts.</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jul 21, 2026
@JSONbored

Copy link
Copy Markdown
Owner

Force pushing resets CI, please do not touch the PR after you open it.

@JSONbored JSONbored closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. pr:flagged PR flagged for review by security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

findPreviewUrlFromChecks doesn't paginate check-runs, unlike its sibling functions in the same file

2 participants