feat: show code change diff panel during init onboarding step 5#580
Conversation
Step 4 previously flashed "Code added to <file> ✅" which disappeared
before the user could read it. Capture the before/after content when
injecting the CapacitorUpdater import + notifyAppReady() call, then
render a bordered Ink panel in step 5 that shows 5 lines of context
above and below the change with line numbers and +-markers.
The diff is persisted into the onboarding state file so resuming the
session at step 5 still shows the panel. It stays visible throughout
step 5 and is only cleared once the user moves on to step 6.
Also silences two stray @clack/prompts writes in step 4 that were
corrupting the Ink UI:
- findMainFile now takes a silent flag; addCodeStep calls it silently
- addCodeStep calls findProjectType({ quiet: true }) (the existing
quiet option was not being passed)
📝 WalkthroughWalkthroughAdds init-time code-diff support: new diff types and runtime state, UI panel to render per-line diffs, diff-generation and persistence integrated into onboarding command flow, plus minor helper and logging-suppression updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Command as Onboarding Command
participant Runtime as Init Runtime
participant State as Init State
participant UI as Ink UI
User->>Command: Trigger onboarding (code injection step)
Command->>Command: detect project type / locate main file
Command->>Command: compute before/after contents
Command->>Command: buildCodeDiffLines() -> InitCodeDiff
Command->>Runtime: setInitCodeDiff(InitCodeDiff)
Runtime->>State: updateState({ codeDiff })
State->>UI: re-render with codeDiff
UI->>User: render CodeDiffPanel (lines, notes, gutter)
Note over Command,Runtime: on resume/step transitions clear or persist diff
Command->>Runtime: setInitCodeDiff(undefined) -- when diff cleared
Runtime->>State: updateState({ codeDiff: undefined })
State->>UI: re-render without panel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/init/command.ts (1)
2605-2610: Minor: Explicitundefinedis acceptable here despite static analysis warning.SonarCloud flags line 2609 as redundant because
globalCodeDiffis alreadyundefinedafter line 2608. While technically you could writesetInitCodeDiff(globalCodeDiff), the explicitsetInitCodeDiff(undefined)clearly communicates the intent to clear the diff from the UI state. This is a stylistic choice that favors readability over minimal redundancy.If you prefer to address the warning:
Optional: Address static analysis warning
if (globalCodeDiff) { globalCodeDiff = undefined - setInitCodeDiff(undefined) + setInitCodeDiff(globalCodeDiff) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 2605 - 2610, The code clears the diff UI by setting globalCodeDiff = undefined then calling setInitCodeDiff(undefined); to avoid the SonarCloud "redundant assignment" warning either keep the explicit intent (no change) or change the call to setInitCodeDiff(globalCodeDiff) so you don't pass a redundant literal; locate the two symbols globalCodeDiff and setInitCodeDiff in the if-block and either leave the explicit undefined for readability or replace the second call to use the current variable (setInitCodeDiff(globalCodeDiff)) to satisfy static analysis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/init/command.ts`:
- Around line 2605-2610: The code clears the diff UI by setting globalCodeDiff =
undefined then calling setInitCodeDiff(undefined); to avoid the SonarCloud
"redundant assignment" warning either keep the explicit intent (no change) or
change the call to setInitCodeDiff(globalCodeDiff) so you don't pass a redundant
literal; locate the two symbols globalCodeDiff and setInitCodeDiff in the
if-block and either leave the explicit undefined for readability or replace the
second call to use the current variable (setInitCodeDiff(globalCodeDiff)) to
satisfy static analysis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5a71091-9277-45c0-b765-5f0de7a5f73e
📒 Files selected for processing (4)
src/init/command.tssrc/init/runtime.tsxsrc/init/ui/app.tsxsrc/utils.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c15ccd7d25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {snapshot.codeDiff && ( | ||
| <CodeDiffPanel diff={snapshot.codeDiff} width={contentWidth} /> |
There was a problem hiding this comment.
Rendering CodeDiffPanel adds a multi-line block to the frame, but log truncation still uses the old fixed budget (rows - 14 in src/init/ui/app.tsx). In typical 24-row terminals, showing a 10+ line diff plus the previous log budget can push the active prompt/spinner out of view during step 5, which makes the encryption question hard to see or interact with. Please reduce the visible log budget when snapshot.codeDiff is present (or otherwise account for panel height) so prompts remain visible.
Useful? React with 👍 / 👎.
…anup - Prefix displayed file path with the project directory name so `capgo init` run from a parent folder shows `CLI/src/main.tsx` instead of `src/main.tsx`. - Drop the trailing newline from the main-file injection template so the diff no longer reports a spurious blank `+` line at the end of the block. - Estimate the rendered diff panel height (including wrapped lines) and subtract it from the log viewport clamp so the encryption prompt and spinner stay visible on short terminals. - Clear the restored code diff whenever a resumed onboarding state is discarded (start-over, org validation failure, role loss, 2FA block, or read error) so a fresh manual step 4 never re-shows stale content.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/init/command.ts`:
- Around line 1598-1616: The current logic overwrites an existing Nuxt plugin
file when it lacks 'CapacitorUpdater.notifyAppReady()', risking data loss;
change the behavior in the branch that detects existsSync(nuxtFilePath) and
!currentContent.includes('CapacitorUpdater.notifyAppReady()') to perform a safe,
non-destructive update: either attempt a minimal injection (search for an
appropriate client-init block or export default/ready hook and insert only the
notify call/snippet) and writeFileSync(nuxtFilePath) only if a safe insertion
point is found, or else do not call writeFileSync at all and instead set
globalCodeDiff to indicate manual action required (use
nuxtFilePath/nuxtDisplayPath, currentContent, nuxtFileContent,
buildCodeDiffLines and CODE_DIFF_CONTEXT_LINES to generate the diff/note).
Ensure readFileSync, existsSync and writeFileSync usages remain but avoid
wholesale replacement of the file content.
- Around line 2519-2527: The discardResumedState function only clears in-memory
state (stepToSkip, globalCodeDiff, setInitCodeDiff) but doesn't remove the
on-disk resume/temp record, so make the discard durable by invoking
cleanupStepsDone() inside discardResumedState; update discardResumedState to
call cleanupStepsDone() (and keep existing resets) so that the temp file created
for resumed onboarding is cleared and the stale resume won't be offered again
before markStepDone() runs.
In `@src/init/ui/app.tsx`:
- Around line 54-68: The current visibleLogs calculation uses slice(-Math.max(0,
rows - 14 - diffPanelHeight)) which incorrectly treats -0 as 0 and returns the
full array; instead compute a variable n = Math.max(0, rows - 14 -
diffPanelHeight) and set visibleLogs = n === 0 ? [] : snapshot.logs.slice(-n) so
when the clamp is zero you get an empty array (not the entire log list); update
the code referencing visibleLogs and diffPanelHeight accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 350125b9-63f2-4612-934a-0a4fd21314b4
📒 Files selected for processing (2)
src/init/command.tssrc/init/ui/app.tsx
| if (existsSync(nuxtFilePath)) { | ||
| const currentContent = readFileSync(nuxtFilePath, 'utf8') | ||
| if (currentContent.includes('CapacitorUpdater.notifyAppReady()')) { | ||
| s.stop('Code already added to capacitorUpdater.client.ts file inside plugins directory ✅') | ||
| pLog.info('Plugins directory and capacitorUpdater.client.ts file already exist with required code') | ||
| s.stop() | ||
| globalCodeDiff = { | ||
| filePath: nuxtDisplayPath, | ||
| created: false, | ||
| lines: [], | ||
| note: 'Already contains CapacitorUpdater.notifyAppReady() — no change needed', | ||
| } | ||
| } | ||
| else { | ||
| writeFileSync(nuxtFilePath, nuxtFileContent, 'utf8') | ||
| s.stop('Code added to capacitorUpdater.client.ts file inside plugins directory ✅') | ||
| pLog.info('Updated capacitorUpdater.client.ts file with required code') | ||
| s.stop() | ||
| globalCodeDiff = { | ||
| filePath: nuxtDisplayPath, | ||
| created: false, | ||
| lines: buildCodeDiffLines(currentContent, nuxtFileContent, CODE_DIFF_CONTEXT_LINES), | ||
| } |
There was a problem hiding this comment.
Don't overwrite an existing Nuxt updater plugin wholesale.
If plugins/capacitorUpdater.client.ts/js already exists but doesn't contain notifyAppReady(), this branch replaces the entire file with the template. That can silently delete user code during onboarding. Prefer a safe merge or fall back to manual instructions instead of overwriting.
🛠️ Safer fallback
if (existsSync(nuxtFilePath)) {
const currentContent = readFileSync(nuxtFilePath, 'utf8')
if (currentContent.includes('CapacitorUpdater.notifyAppReady()')) {
s.stop()
globalCodeDiff = {
filePath: nuxtDisplayPath,
created: false,
lines: [],
note: 'Already contains CapacitorUpdater.notifyAppReady() — no change needed',
}
}
else {
- writeFileSync(nuxtFilePath, nuxtFileContent, 'utf8')
- s.stop()
- globalCodeDiff = {
- filePath: nuxtDisplayPath,
- created: false,
- lines: buildCodeDiffLines(currentContent, nuxtFileContent, CODE_DIFF_CONTEXT_LINES),
- }
+ s.stop('Nuxt updater plugin already exists', 'neutral')
+ pLog.warn(`❌ ${nuxtDisplayPath} already exists and would be overwritten.`)
+ pLog.info('💡 Merge the updater plugin code manually, then continue onboarding.')
+ return
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init/command.ts` around lines 1598 - 1616, The current logic overwrites
an existing Nuxt plugin file when it lacks 'CapacitorUpdater.notifyAppReady()',
risking data loss; change the behavior in the branch that detects
existsSync(nuxtFilePath) and
!currentContent.includes('CapacitorUpdater.notifyAppReady()') to perform a safe,
non-destructive update: either attempt a minimal injection (search for an
appropriate client-init block or export default/ready hook and insert only the
notify call/snippet) and writeFileSync(nuxtFilePath) only if a safe insertion
point is found, or else do not call writeFileSync at all and instead set
globalCodeDiff to indicate manual action required (use
nuxtFilePath/nuxtDisplayPath, currentContent, nuxtFileContent,
buildCodeDiffLines and CODE_DIFF_CONTEXT_LINES to generate the diff/note).
Ensure readFileSync, existsSync and writeFileSync usages remain but avoid
wholesale replacement of the file content.
| // Whenever a resume is aborted (org no longer available, role lost, 2FA | ||
| // required, lookup failed) we restart from step 0. Drop any diff that | ||
| // `tryResumeOnboarding` restored so the freshly walked step 4 doesn't see | ||
| // stale content from an earlier run. | ||
| const discardResumedState = () => { | ||
| stepToSkip = 0 | ||
| globalCodeDiff = undefined | ||
| setInitCodeDiff(undefined) | ||
| } |
There was a problem hiding this comment.
Discarded resume state should also clear the temp file.
discardResumedState() only resets in-memory fields. If the user exits before the next markStepDone(), the same invalid/org-blocked resume record is still on disk and will be offered again on the next run. Calling cleanupStepsDone() here would make the discard durable.
🛠️ Proposed fix
const discardResumedState = () => {
+ cleanupStepsDone()
stepToSkip = 0
globalCodeDiff = undefined
setInitCodeDiff(undefined)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Whenever a resume is aborted (org no longer available, role lost, 2FA | |
| // required, lookup failed) we restart from step 0. Drop any diff that | |
| // `tryResumeOnboarding` restored so the freshly walked step 4 doesn't see | |
| // stale content from an earlier run. | |
| const discardResumedState = () => { | |
| stepToSkip = 0 | |
| globalCodeDiff = undefined | |
| setInitCodeDiff(undefined) | |
| } | |
| const discardResumedState = () => { | |
| cleanupStepsDone() | |
| stepToSkip = 0 | |
| globalCodeDiff = undefined | |
| setInitCodeDiff(undefined) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init/command.ts` around lines 2519 - 2527, The discardResumedState
function only clears in-memory state (stepToSkip, globalCodeDiff,
setInitCodeDiff) but doesn't remove the on-disk resume/temp record, so make the
discard durable by invoking cleanupStepsDone() inside discardResumedState;
update discardResumedState to call cleanupStepsDone() (and keep existing resets)
so that the temp file created for resumed onboarding is cleared and the stale
resume won't be offered again before markStepDone() runs.
| const diffPanelHeight = (() => { | ||
| const diff = snapshot.codeDiff | ||
| if (!diff) | ||
| return 0 | ||
| const innerWidth = Math.max(1, contentWidth - 4) | ||
| const wrappedLineRows = diff.lines.reduce((sum, line) => { | ||
| const rendered = ` ${String(line.lineNumber)} │ ${line.text}` | ||
| return sum + Math.max(1, Math.ceil(rendered.length / innerWidth)) | ||
| }, 0) | ||
| const noteRows = diff.note !== undefined ? 1 : 0 | ||
| const linesBlockRows = diff.lines.length > 0 ? wrappedLineRows + 1 : 0 | ||
| // 1 (panel marginTop) + 2 (borders) + 1 (title) + noteRows + linesBlockRows | ||
| return 4 + noteRows + linesBlockRows | ||
| })() | ||
| const visibleLogs = snapshot.logs.slice(-Math.max(0, rows - 14 - diffPanelHeight)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the specific lines
if [ -f "src/init/ui/app.tsx" ]; then
echo "=== File exists, reading lines 54-68 ==="
sed -n '54,68p' "src/init/ui/app.tsx" | cat -n
else
echo "File not found at src/init/ui/app.tsx"
fiRepository: Cap-go/CLI
Length of output: 908
🌐 Web query:
JavaScript Array.prototype.slice(-0) behavior behavior
💡 Result:
arr.slice(-0) behaves exactly like arr.slice(0): it returns a (shallow) copy of the entire array.
Reason: the spec converts start using ToIntegerOrInfinity. If start is -0, then relativeStart is -0, and the check relativeStart < 0 is false, so the algorithm takes the “else” branch and sets k = min(relativeStart, len) (which is still -0, effectively 0 for indexing). Therefore slicing starts at index 0. [1]
MDN’s description (“if start is negative, it’s treated as length + start”) also implies -0 won’t be treated as negative in practice here. [2]
Example
[1,2,3].slice(-0) // [1,2,3]
[1,2,3].slice(0) // [1,2,3]Sources:
[1] ECMAScript spec, Array.prototype.slice algorithm (steps using relativeStart < 0) https://tc39.es/ecma262/multipage/indexed-collections.html
[2] MDN, Array.prototype.slice() https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice
🏁 Script executed:
# Check for skills/ directory and related content
if [ -d "skills/" ]; then
echo "=== Skills directory exists ==="
find skills/ -type f -name "*.md" -o -name "*.ts" -o -name "*.tsx" | head -20
else
echo "Skills directory not found"
fi
# Check git history for this file to understand context
echo "=== Recent commits touching src/init/ui/app.tsx ==="
git log --oneline -10 -- "src/init/ui/app.tsx" 2>/dev/null || echo "No git history available"Repository: Cap-go/CLI
Length of output: 378
🏁 Script executed:
# Check the skills files for references to "init" or "diff"
echo "=== Searching skill docs for init/diff references ==="
grep -r "init\|diff" skills/ --include="*.md" -l 2>/dev/null || echo "No matches found"
# Check what files were changed in the commit that touched app.tsx
echo -e "\n=== Files in the commit ==="
git show 6f16c59 --name-only --pretty=format: | head -20
# Look for any CLI workflow related to init
echo -e "\n=== Checking if init is a CLI command ==="
grep -r "init" src/commands --include="*.ts" -l 2>/dev/null | head -5Repository: Cap-go/CLI
Length of output: 683
🏁 Script executed:
# Check the actual diff of the commit to see if skill docs were updated
echo "=== Changes in skills/ from commit ==="
git show 6f16c59 -- skills/ 2>/dev/null | head -100 || echo "No changes to skills/ in this commit"Repository: Cap-go/CLI
Length of output: 4486
Fix zero-log clamp to prevent off-screen prompt.
When Math.max(0, rows - 14 - diffPanelHeight) evaluates to 0, slice(-0) in JavaScript returns the entire log array (equivalent to slice(0)), defeating the short-terminal protection. This allows the prompt/spinner to be pushed off-screen.
Proposed fix
- const visibleLogs = snapshot.logs.slice(-Math.max(0, rows - 14 - diffPanelHeight))
+ const maxVisibleLogs = Math.max(0, rows - 14 - diffPanelHeight)
+ const visibleLogs = maxVisibleLogs === 0
+ ? []
+ : snapshot.logs.slice(-maxVisibleLogs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const diffPanelHeight = (() => { | |
| const diff = snapshot.codeDiff | |
| if (!diff) | |
| return 0 | |
| const innerWidth = Math.max(1, contentWidth - 4) | |
| const wrappedLineRows = diff.lines.reduce((sum, line) => { | |
| const rendered = ` ${String(line.lineNumber)} │ ${line.text}` | |
| return sum + Math.max(1, Math.ceil(rendered.length / innerWidth)) | |
| }, 0) | |
| const noteRows = diff.note !== undefined ? 1 : 0 | |
| const linesBlockRows = diff.lines.length > 0 ? wrappedLineRows + 1 : 0 | |
| // 1 (panel marginTop) + 2 (borders) + 1 (title) + noteRows + linesBlockRows | |
| return 4 + noteRows + linesBlockRows | |
| })() | |
| const visibleLogs = snapshot.logs.slice(-Math.max(0, rows - 14 - diffPanelHeight)) | |
| const diffPanelHeight = (() => { | |
| const diff = snapshot.codeDiff | |
| if (!diff) | |
| return 0 | |
| const innerWidth = Math.max(1, contentWidth - 4) | |
| const wrappedLineRows = diff.lines.reduce((sum, line) => { | |
| const rendered = ` ${String(line.lineNumber)} │ ${line.text}` | |
| return sum + Math.max(1, Math.ceil(rendered.length / innerWidth)) | |
| }, 0) | |
| const noteRows = diff.note !== undefined ? 1 : 0 | |
| const linesBlockRows = diff.lines.length > 0 ? wrappedLineRows + 1 : 0 | |
| // 1 (panel marginTop) + 2 (borders) + 1 (title) + noteRows + linesBlockRows | |
| return 4 + noteRows + linesBlockRows | |
| })() | |
| const maxVisibleLogs = Math.max(0, rows - 14 - diffPanelHeight) | |
| const visibleLogs = maxVisibleLogs === 0 | |
| ? [] | |
| : snapshot.logs.slice(-maxVisibleLogs) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/init/ui/app.tsx` around lines 54 - 68, The current visibleLogs
calculation uses slice(-Math.max(0, rows - 14 - diffPanelHeight)) which
incorrectly treats -0 as 0 and returns the full array; instead compute a
variable n = Math.max(0, rows - 14 - diffPanelHeight) and set visibleLogs = n
=== 0 ? [] : snapshot.logs.slice(-n) so when the clamp is zero you get an empty
array (not the entire log list); update the code referencing visibleLogs and
diffPanelHeight accordingly.
discardResumedState only cleared in-memory state — the on-disk tmp file persisted, so the next `capgo init` invocation would re-offer the stale resume before markStepDone() had a chance to overwrite it. Call cleanupStepsDone() inside discardResumedState so the discard is durable.
|
1 participant
Summary
Changes
Test plan
Summary by CodeRabbit
New Features
Bug Fixes