Merge dev into main#245
Conversation
* chore: add project cleanup config * Fix compact diff baseline icon * Add chat message copy controls * Fix chat markdown wrapping * Fix macOS window chrome and quit path * Add sidebar project folder browser * Refine project folder browser controls * Fix composer textarea alignment jump * Align composer textarea scrollbar * Measure composer textarea before paint * Stop composer text resize from rerendering timeline * Anchor thread composer to bottom while typing * Stabilize composer dictation adornment layout * Revert "Stabilize composer dictation adornment layout" This reverts commit 00c4586. * Reserve composer dictation adornment space * Pad composer text around dictation adornment * Hide composer adornment at tiny widths * Keep composer text wrapping vertically * Avoid reserving dictation space on multiline composer * Keep dictation adornment and refresh composer resize state * Preserve session composer model choices * Polish compact composer controls * Close terminal drawer when entering compact mode * Align takeover sidebar toggle * Fix dictation setup prompt routing * Fix project folder browser edge cases * Update changelog for 0.1.64 * Address PR review edge cases
* Improve Pi message status details * Address Pi status review edge cases
* Support dashboard terminal and sidebar toggle * Polish project dashboard narrow layout * Refine composer and dashboard narrow chrome * Add about link to landing page * Add beta update branch setting * Fix beta updater channel edge cases * Fix release notes and landing settings state
* Fix launcher download timeout * Bump launcher package version
* Add React Doctor and tidy low-risk issues * Continue React Doctor cleanup * Polish diff baseline popover * Align GitOps composer sizing * Expose GitOps origin settings * Restore accessible composer placeholder
* Polish sidebar and artifact interactions * Add stacked PR workflow guidance * Address artifact and diff review findings
* Fix Codex review follow-ups * Ignore semantic grep cache files * Restore dev workflow validation triggers * Document workflow release conventions * Clarify docs-only validation guidance * Address updater and inbox review feedback * Clarify validation command guidance
* Fix composer controls and project terminals * Polish sidebar favorite indicators * Close status model picker on selection * Refine composer mic and sidebar icon alignment * Restore project favorite icon alignment * Limit left mic placement to empty composer * Align empty composer mic with placeholder * Nudge empty composer mic left * Move empty composer mic farther left * Set empty composer mic midpoint offset * Tighten empty composer mic spacing * Move updater channels to prefixed release tags * Bump npm launcher version * Stop release artifact builds on PRs * Address updater and terminal review findings * Fix gitops terminal restore and archive cleanup
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
![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: a2c26b0e01
ℹ️ 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".
| } else { | ||
| setErrorMessage(result.errorMessage) | ||
| } |
There was a problem hiding this comment.
When openDirectory fails, the component keeps the previous currentPath in parent state, so the create action can still submit using an old folder even though navigation just failed. This can add/create a project in the wrong directory after an error (for example, permission denied or missing path). Resetting the current-path selection on error avoids stale-path submissions.
Useful? React with 👍 / 👎.
| .catch((error) => { | ||
| if (!cancelled) { | ||
| setArtifactLoadError( | ||
| error instanceof Error ? error.message : 'Could not load artifacts.', | ||
| ) |
There was a problem hiding this comment.
A transient failure in listArtifactsQuery sets artifactLoadError, but this error is only cleared on conversation change. If artifacts later arrive via desktop artifact-update events, the panel still renders the error screen and hides real data. Clearing artifactLoadError when artifacts are successfully populated prevents the panel from getting stuck in an error state.
Useful? React with 👍 / 👎.
| onClick={() => setPinned((current) => !current)} | ||
| onPointerDown={loadUsageTotals} | ||
| aria-label={label} |
There was a problem hiding this comment.
Usage totals are loaded on onPointerDown only. If a user opens the context popover via keyboard (Enter/Space), usageTotals is never populated, so the detailed usage section is missing even when message usage exists. Triggering the same data load from keyboard activation path keeps behavior consistent and accessible.
Useful? React with 👍 / 👎.
| const loadUsageTotals = useCallback(() => { | ||
| setUsageTotals(getMessageUsageTotals(messages)) | ||
| }, [messages]) | ||
|
|
||
| const openHoverPreview = useCallback(() => { | ||
| clearHoverTriangle() | ||
| loadUsageTotals() | ||
| setHovered(true) | ||
| }, [clearHoverTriangle]) | ||
| }, [clearHoverTriangle, loadUsageTotals]) |
There was a problem hiding this comment.
Totals are computed only when opening/hovering, so a pinned-open popover shows stale token/cost numbers as new assistant messages stream in. Since this panel is used for live usage visibility, it should recompute while open (or on message changes) to avoid outdated figures.
Useful? React with 👍 / 👎.
| if (!(selectedProject && projectUsageQuery.data?.archivedUsageRefreshing)) return | ||
| const timeout = window.setTimeout(() => { | ||
| void projectUsageQuery.refetch() | ||
| }, 750) |
There was a problem hiding this comment.
This effect refetches every 750ms while archivedUsageRefreshing is true. Each refetch re-runs full usage summary loading, including active-session scans, which can cause repeated disk-heavy work on large projects. This should be event-driven or throttled more conservatively to avoid unnecessary churn.
Useful? React with 👍 / 👎.
| label="Spend" | ||
| loading={usageLoading} | ||
| value={formatCost(usageSummary?.costTotal)} | ||
| detail={`${formatAverageCost(usageSummary?.costTotal, sessionCount)}/session`} |
There was a problem hiding this comment.
The spend-per-session detail divides by total sessions, including sessions with zero assistant turns. For projects with many empty/draft sessions, this underreports average spend and can be misleading. Using sessionsWithUsageCount would better match what users expect from a spend average.
Useful? React with 👍 / 👎.
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0ef9ab31e
ℹ️ 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".
| if (versionDiff < 0) return false | ||
|
|
||
| const runningRelease = getRunningReleaseFingerprint() | ||
| if (release.channel === 'main') return false |
There was a problem hiding this comment.
Returning false for every main release when versionDiff === 0 blocks hash-only channel updates from ever being offered. This is a regression with moving channel-main assets: if main is republished with the same semver but a new archive hash, clients remain stuck on the old build because available is never reached.
Useful? React with 👍 / 👎.
| if (command.source === 'app' && command.name === 'new') { | ||
| setDraft('') | ||
| onStartNewSession?.() | ||
| return |
There was a problem hiding this comment.
The /new branch clears the draft and returns even when onStartNewSession is undefined. In composers that don't pass this callback (for example inbox composer), submitting /new silently discards user input and performs no action, which is a user-visible behavior break.
Useful? React with 👍 / 👎.
| const canSubmit = | ||
| draft.trim().length > 0 && | ||
| !busy && | ||
| (Boolean(defaultLocation) || (browseOpen && Boolean(currentFolderPath))) |
There was a problem hiding this comment.
When browse mode is open, canSubmit still becomes true solely from defaultLocation. If the folder browser has not resolved a path (or failed), clicking add sends parentPath: null and falls back to the default location, which contradicts the visible “browse folder” flow and can create projects in the wrong place.
Useful? React with 👍 / 👎.
| <button | ||
| type="button" | ||
| className="sidebar-project-create-submit sidebar-project-browse-toggle" | ||
| onClick={() => setBrowseOpen((current) => !current)} |
There was a problem hiding this comment.
Toggling the browser only flips browseOpen; it does not reset currentFolderPath. Reopening the browser can leave an old path active until the async directory load completes, so an immediate submit can target the previous folder rather than the currently shown one.
Useful? React with 👍 / 👎.
| @@ -17,7 +18,11 @@ import type { PiRuntime } from './types.ts' | |||
| const builtinCommandNames = new Set([compactSlashCommand.name]) | |||
There was a problem hiding this comment.
Builtin filtering still reserves only compact, even though app-level settings and new commands are now injected. Extensions/prompt templates can therefore register the same names and appear as conflicting duplicates, while exact /new handling is hardcoded to app behavior, making extension variants ambiguous or unreachable.
Useful? React with 👍 / 👎.
|
|
||
| async function listProjectDirectoryEntries(request: { path?: string | null | undefined }) { | ||
| const homePath = os.homedir() | ||
| const requestedPath = request.path?.trim() ? request.path : homePath |
There was a problem hiding this comment.
The dev bridge checks request.path?.trim() but still uses the untrimmed request.path value. A path with accidental leading/trailing whitespace resolves as a different filesystem path and can fail directory listing in dev mode, diverging from the Electron handler behavior.
Useful? React with 👍 / 👎.
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
There was a problem hiding this comment.
💡 Codex Review
Submit /new while slash commands are loading
When Enter is pressed with /new before slash commands finish loading, this branch prevents default submission but only calls submit() for /settings. In that state /new is swallowed and does nothing, so users can hit Enter and get no action until they retry after loading completes.
ℹ️ 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".
| } | ||
| onOpenSettingsView={onOpenSettingsView} | ||
| onStartNewSession={() => | ||
| void onAction('thread.new', { projectId: thread.projectId, composerMode: 'code' }) |
There was a problem hiding this comment.
The new-session action from the inbox always sends composerMode: 'code', so running /new from an inbox thread that came from chat will open a code draft instead of a chat draft. This regresses mode consistency for chat inbox items and can route the user into the wrong workflow unless they manually switch views first.
Useful? React with 👍 / 👎.
| } else { | ||
| onProjectSelect(project.id) | ||
| } |
There was a problem hiding this comment.
The project row “create session” handler no longer calls thread.new in the code view path and now only selects the project. This changes the button from an action that creates a fresh session into a no-op for already-selected projects, so users can no longer reliably start a new code thread from that control.
Useful? React with 👍 / 👎.
| sessionPath: thread.sessionPath, | ||
| lastModifiedMs: thread.lastModifiedMs, | ||
| }) | ||
| threadUsageCache.set(thread.sessionPath, { summary, signature: cacheKey }) |
There was a problem hiding this comment.
Per-session and per-project usage summaries are inserted into process-wide Maps without any eviction or cleanup path, so long-running desktop sessions that touch many projects/threads will retain stale entries indefinitely. This can steadily increase memory usage over time even after projects/threads are deleted.
Useful? React with 👍 / 👎.
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f403dd0e1a
ℹ️ 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".
| .finally(() => { | ||
| cleanupFinished = true | ||
| app.quit() | ||
| app.exit(0) |
There was a problem hiding this comment.
After before-quit is prevented, calling app.exit(0) terminates immediately and skips Electron’s normal app.quit() flow, including renderer beforeunload/unload handling. That can drop in-flight UI state or bypass window-level cleanup whenever runtime shutdown takes the async path (which is now every quit). Electron’s app docs note app.exit bypasses before-quit/will-quit semantics, so this changes shutdown behavior in a user-visible way.
Useful? React with 👍 / 👎.
| const baseUrl = RELEASE_BASE_URL.replace(trailingSlashesPattern, '') | ||
| if (baseUrl.includes('{releaseTag}')) | ||
| return baseUrl.replace(releaseTagPlaceholderPattern, releaseTag) | ||
| if (baseUrl.includes('{channel}')) return baseUrl.replace(channelPlaceholderPattern, releaseTag) |
There was a problem hiding this comment.
{channel} placeholder mapped to actual channel name
Replacing {channel} with channel-main/channel-dev instead of main/dev breaks existing custom mirror URLs that use {channel} literally for branch/channel folders. For deployments configured as .../{channel}/..., update checks will start requesting .../channel-main/... and fail unless mirrors are restructured, which is a regression for externally hosted update endpoints.
Useful? React with 👍 / 👎.
| const channelPrefixedMatch = channelReleaseKeyPattern.exec(runningReleaseKey) | ||
| if (channelPrefixedMatch?.[2] && channelPrefixedMatch[3]) { | ||
| return { | ||
| version: channelPrefixedMatch[2], | ||
| hash: channelPrefixedMatch[3].toLowerCase(), | ||
| } | ||
| } | ||
|
|
||
| return null |
There was a problem hiding this comment.
The new running-release fingerprint parser only accepts keys prefixed with main-/dev-. Older installed caches use <version>-<hash>, so getRunningReleaseFingerprint() returns null for those users. In dev channel mode with equal semver, isUpdateCandidate() will then keep returning true and repeatedly offer/reinstall the same build because hash equality is never detected.
Useful? React with 👍 / 👎.
| moveTranscript(input.record.transcriptPath, nextTranscriptPath) | ||
| input.record.transcriptPath = nextTranscriptPath | ||
| input.record.snapshot = { |
There was a problem hiding this comment.
If transcript rename fails (e.g. destination already exists), the code still rewires record.transcriptPath to the new file. That silently detaches the active terminal record from its existing transcript and can lose already collected shell history for this terminal session. This is especially likely when rebinding to a session ID with an existing transcript file.
Useful? React with 👍 / 👎.
| if (entry && isDisplayableEntry(entry)) { | ||
| count += 1 | ||
| if (modelChangeConsumesNextEntry(entry, entries[index + 1])) { | ||
| index += 1 | ||
| } |
There was a problem hiding this comment.
countDisplayableEntriesInRange now counts all model_change entries as visible, but appendModelChangeMessage emits nothing when provider/model is missing. For legacy or partial session logs, the “previous message count” can be inflated relative to actual rendered rows, causing incorrect history-divider counts and compaction UX mismatch.
Useful? React with 👍 / 👎.
| case 'model_change': | ||
| return appendModelChangeMessage(messages, entry, nextEntry) | ||
| case 'thinking_level_change': | ||
| appendThinkingLevelChangeMessage(messages, entry) |
There was a problem hiding this comment.
display:false for new model/reasoning history entries
The new model_change and thinking_level_change branches bypass the display guard that is used for hidden custom entries. If persisted history marks these events with display: false, they will still be surfaced as system rows, leaking entries that callers explicitly marked as non-displayable.
Useful? React with 👍 / 👎.
1 participant
Summary
devbranch intomainfor the next stable update.Validation
devtomainwith no local code changes.