Feat/forge templates - #36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR extends the Forge API contract with PR and repo metadata, implements per-file diff expansion controls in ForgePanel with PR link display, and updates CI pre-push checklist guidance for GitHub Actions version pinning detection. ChangesForge API and Diff Viewer Enhancement
CI Checklist Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/lib/components/ForgePanel.svelteOops! Something went wrong! :( ESLint: 10.4.1 The requested operation requires ESLint to serialize configuration data, Please double-check your configuration for errors. If you still have problems, please stop by https://eslint.org/chat/help to chat Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/components/ForgePanel.svelte (1)
102-104: ⚡ Quick winConsider resetting
expandedFileswhen the diff data or selected task changes.The expansion state currently persists across task switches. If a user expands
src/app.tsin task A and then selects task B (which also modifiessrc/app.ts), the file will start expanded in task B. This could be confusing.♻️ Suggested fix to reset expansion state
Add a
$effectto resetexpandedFileswhen the selected task or diff data changes:function toggleFile(path: string) { expandedFiles[path] = !expandedFiles[path]; } + +$effect(() => { + // Reset expansion state when task or diff changes + if ($forgeSelectedId || $forgeDiffData) { + expandedFiles = {}; + } +});🤖 Prompt for 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. In `@src/lib/components/ForgePanel.svelte` around lines 102 - 104, The expansion state in expandedFiles persists across task switches; add a Svelte reactive statement that resets expandedFiles to an empty object whenever the selected task or the diff data changes so expandedFiles no longer carries over between tasks—update the component to watch the selectedTask and diffData variables and clear expandedFiles (used by toggleFile) when either changes.
🤖 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 `@src/lib/components/ForgePanel.svelte`:
- Around line 324-333: The if-block in ForgePanel.svelte currently checks
{$forgeDetail.pr_number} truthily which treats 0 as falsy; update the condition
to explicitly test for null (e.g., $forgeDetail.pr_number !== null) while still
verifying $forgeDetail.repo so PR number 0 is correctly rendered; locate the
{`#if` $forgeDetail.pr_number && $forgeDetail.repo} block and replace the truthy
check with an explicit null comparison.
---
Nitpick comments:
In `@src/lib/components/ForgePanel.svelte`:
- Around line 102-104: The expansion state in expandedFiles persists across task
switches; add a Svelte reactive statement that resets expandedFiles to an empty
object whenever the selected task or the diff data changes so expandedFiles no
longer carries over between tasks—update the component to watch the selectedTask
and diffData variables and clear expandedFiles (used by toggleFile) when either
changes.
🪄 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 Plus
Run ID: 628fc90d-24d8-4c48-a7bd-9b5c4220ad5d
📒 Files selected for processing (3)
src/lib/api/forgeClient.tssrc/lib/components/ForgePanel.sveltesrc/routes/todo/+page.svelte
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/components/ForgePanel.svelte (1)
106-110: 💤 Low valueConsider unconditionally resetting
expandedFilesin the effect.The current condition
if ($forgeSelectedId || $forgeDiffData)meansexpandedFilesis only cleared when there's a selected task or diff data. When both become null (e.g., task deselected), the state isn't cleared, leaving stale expansion state in memory. While harmless (the diff viewer won't render when no task is selected), unconditionally resetting on every dependency change would be more explicit:$effect(() => { const _ = $forgeSelectedId ?? $forgeDiffData; expandedFiles = {}; });The current implementation is correct and acceptable; this is a minor clarity improvement.
🤖 Prompt for 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. In `@src/lib/components/ForgePanel.svelte` around lines 106 - 110, In the $effect block that currently checks `if ($forgeSelectedId || $forgeDiffData)` before clearing `expandedFiles`, change it to unconditionally reset `expandedFiles = {}` on every effect run so stale expansion state is never retained; update the `$effect` callback (the one referencing `$forgeSelectedId` and `$forgeDiffData`) to always assign `expandedFiles = {}` rather than only doing so inside the conditional.
🤖 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.
Nitpick comments:
In `@src/lib/components/ForgePanel.svelte`:
- Around line 106-110: In the $effect block that currently checks `if
($forgeSelectedId || $forgeDiffData)` before clearing `expandedFiles`, change it
to unconditionally reset `expandedFiles = {}` on every effect run so stale
expansion state is never retained; update the `$effect` callback (the one
referencing `$forgeSelectedId` and `$forgeDiffData`) to always assign
`expandedFiles = {}` rather than only doing so inside the conditional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c5fd83e0-f1f7-4992-9ce6-cbb4520b78d0
📒 Files selected for processing (1)
src/lib/components/ForgePanel.svelte
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.opencode/agent/helios-cli.md:
- Line 4: Update the grep command to use the correct file glob and regex: change
the file pattern from the literal "_.yml" to "*.yml" to match all workflow YAML
files, and change the regex from "uses:._@v[0-9]" to "uses:.*`@v`[0-9]" so it
matches typical GitHub Actions lines like "uses: actions/checkout@v4"; apply
this edit to the grep invocation shown in the diff so subsequent pipelines
search all .yml files and correctly detect uses:...@vN patterns.
🪄 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 Plus
Run ID: d64f1ce6-561d-499f-ae76-2826dbf106fc
📒 Files selected for processing (2)
.opencode/agent/helios-cli.mdsrc/lib/components/ForgePanel.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/components/ForgePanel.svelte
| Only then report "ready for merge." | ||
|
|
||
| 1. npm run lint && npm run test | ||
| 2. grep -n 'uses:._@v[0-9]' .github/workflows/_.yml | grep -v '# pinned' |
There was a problem hiding this comment.
Fix the file glob and regex pattern in the grep command.
The grep command has two critical issues that prevent it from working:
-
File glob
_.yml: The underscore is a literal character, not a wildcard. This pattern will only match a file literally named_.yml, notci.ymlor other workflow files. Should be*.ymlto match all YAML workflow files. -
Regex pattern
uses:._@v[0-9]: This matches "uses:" + any char + literal underscore + "@v" + digit (e.g.,uses:x_@v4), but will NOT match standard GitHub Actions syntax likeuses: actions/checkout@v4. Should beuses:.*@v[0-9]to match any characters betweenuses:and@v.
🔧 Proposed fix
-grep -n 'uses:._@v[0-9]' .github/workflows/_.yml | grep -v '# pinned'
+grep -n 'uses:.*`@v`[0-9]' .github/workflows/*.yml | grep -v '# pinned'🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...m run test 2. grep -n 'uses:.@v[0-9]' .github/workflows/.yml | grep -v '# pinned' ...
(GITHUB)
🤖 Prompt for 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.
In @.opencode/agent/helios-cli.md at line 4, Update the grep command to use the
correct file glob and regex: change the file pattern from the literal "_.yml" to
"*.yml" to match all workflow YAML files, and change the regex from
"uses:._@v[0-9]" to "uses:.*`@v`[0-9]" so it matches typical GitHub Actions lines
like "uses: actions/checkout@v4"; apply this edit to the grep invocation shown
in the diff so subsequent pipelines search all .yml files and correctly detect
uses:...@vN patterns.
66d5fa9 to
da5cba7
Compare
fix: POST /forge/{id}/gate ignores approve body field
1 participant
Summary by CodeRabbit