fix: prepare dbt review action for v0.8.4#900
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR exposes runner.manifestAvailable(), seeds orchestrator anyManifest from that signal, centralizes the GitHub App install URL, adds asset integrity tests, updates manifest-related tests, and updates docs/examples/changelog to reference v0.8.4. Changes0.8.4 Release Readiness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f683f50 to
b0d0a8d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/opencode/src/altimate/review/orchestrate.ts`:
- Line 1006: Awaiting the optional hook input.runner.manifestAvailable() is
currently unprotected and can reject and crash runReview; wrap the call in a
try/catch (while still honoring the optional nature) so that if
input.runner.manifestAvailable throws or rejects you set anyManifest = false (or
a safe default) and optionally log the error, instead of letting the exception
bubble; update the code around the anyManifest assignment (the await of
input.runner.manifestAvailable) to perform this guarded call.
In `@packages/opencode/test/altimate/review-runner.test.ts`:
- Around line 2-17: Replace the manual temp-dir lifecycle (mkdtempSync, tempDirs
array and afterEach cleanup) with the test fixture helper: import tmpdir from
"fixture/fixture" and use the await using pattern to create the directory for
the test; specifically, remove tempDirs and the afterEach block and change the
mkdtempSync usage in the test to an await using (const tmp =
tmpdir("altimate-review-manifest-")) { const dir = tmp.path; const manifestPath
= path.join(dir, "manifest.json"); ... } so the fixture automatically cleans up
when the scope exits; keep existing references to createDispatcherRunner and
other test logic unchanged.
In `@packages/opencode/test/altimate/review.test.ts`:
- Around line 1236-1241: The test currently sets manifestAvailable() to true but
leaves impact().hasManifest true, so it doesn't validate the decoupling; change
the stubbed impact() return to set hasManifest: false while keeping
manifestAvailable() === true so env.summary.degraded will reflect the new
contract. Update the same change in the other test seed that mirrors this
behavior (the second occurrence of these stubs) so both cases assert
manifestAvailable() is respected independently of impact().hasManifest.
In `@packages/opencode/test/install/repository-symlinks.test.ts`:
- Line 13: The test currently only asserts the symlink target exists; update the
assertion to also verify the resolved target is inside the repository root by
computing the absolute target (using readlinkSync and path.resolve) and then
checking path.relative(repoRoot, target) does not start with '..' (and is not
equal to '') or by asserting target.startsWith(path.resolve(repoRoot) ) before
asserting existsSync(target) is true; reference the variables/functions link,
readlinkSync, path.resolve, path.relative, and existsSync when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 47534ee8-5ec9-4572-9b02-cd16b0ee76e2
⛔ Files ignored due to path filters (6)
packages/identity/mark-192x192.pngis excluded by!**/*.pngpackages/identity/mark-512x512-light.pngis excluded by!**/*.pngpackages/identity/mark-512x512.pngis excluded by!**/*.pngpackages/identity/mark-96x96.pngis excluded by!**/*.pngpackages/identity/mark-light.svgis excluded by!**/*.svgpackages/identity/mark.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
CHANGELOG.mdREADME.mddocs/docs/usage/dbt-pr-review.mdgithub/README.mdgithub/review/examples/altimate-ingestion.ymlpackages/opencode/src/altimate/review/orchestrate.tspackages/opencode/src/altimate/review/runner.tspackages/opencode/src/cli/cmd/github.tspackages/opencode/test/altimate/review-runner.test.tspackages/opencode/test/altimate/review.test.tspackages/opencode/test/cli/github-action.test.tspackages/opencode/test/install/repository-symlinks.test.ts
There was a problem hiding this comment.
2 issues found across 18 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
dev-punia-altimate
left a comment
There was a problem hiding this comment.
Multi-Persona Review — Verdict: skipped
Multi-persona review completed.
0/0 agents completed · 2s · 0 findings (0 critical, 0 high, 0 medium)
Multi-Persona Review · vllm:qwen3-next-80b (waves) + vllm-fallback (synth) ·
Summary
github/reviewcomposite action archivev0.8.4tagThis PR prepares the patch release but does not create or publish the
v0.8.4tag.Validation
bun test --timeout 30000 test/altimate/review.test.ts test/altimate/review-runner.test.ts(68 passed)bun test --timeout 30000 test/install/repository-symlinks.test.ts test/cli/github-action.test.ts(19 passed)bun test --timeout 30000 test/install/(120 passed, 5 compiled-binary tests skipped because no local release build exists)bun test --timeout 30000 test/install/repository-symlinks.test.ts test/branding/upstream-guard.test.ts test/branding/upstream-merge-guard.test.ts(215 passed)bun run typecheckgit archive HEADand verified allsdks/vscode/images/*assets resolveThe initial CI run exposed the repository guard against restoring the removed upstream
packages/identitypackage. The follow-up keeps that package deleted and embeds only the three required image assets.Summary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests