Merge upstream main with upstream repo routing and stability fixes#1028
Closed
JerkyTreats wants to merge 45 commits intopingdotgg:mainfrom
Closed
Merge upstream main with upstream repo routing and stability fixes#1028JerkyTreats wants to merge 45 commits intopingdotgg:mainfrom
JerkyTreats wants to merge 45 commits intopingdotgg:mainfrom
Conversation
Policy-Ref: governance/commit_policy.md
- add `promote` stacked action to shared git contracts and inputs - implement server promote flow to commit, push backup, merge, push target, and delete branch - return conflict and cleanup status through git manager and git core delete branch API - update web logic and panel UI with Promote action, confirmation dialog, and promote toasts - remove deprecated `--ephemeral` flag from codex text generation command
- Read AGENTS guidance files from cwd to filesystem root - Follow linked commit policy docs when commit lines reference markdown files - Add safe fallback guidance when no local commit instructions are found - Cover fallback and linked guidance behavior with new tests
- detect Omarchy output files and clipboard updates more reliably - infer Hyprland signature and resolve screenshot output directory from env or user dirs - add desktop tests for Omarchy capture edge cases - add GitHub workflow plus script to open or update external dependency watch issues
- Reuse shared commit plus push execution in `GitManager` for promote paths - Preserve generated commit messaging before merge and assert behavior in server tests - Forward `targetBranch` from web mutation options and cover it with a client test - Remove legacy GitHub Actions CI workflow file
- Add ordered WebSocket push and readiness infrastructure across server and web - Introduce runtime receipt bus and queue-backed orchestration worker flow - Add CI and PR vouch workflows plus release pipeline hardening - Refresh architecture and glossary docs to match new runtime model
…tegration T3code/opinionated upstream integration
- Apply repo-wide formatting updates across staged TypeScript and Markdown files - Normalize line wrapping and spacing with no intended behavior change
- Replace hardcoded terminal palette values with CSS variable lookups - Add Nord-inspired light and dark theme tokens for page, sidebar, selection, and terminal colors - Update chat sidebar to use sidebar-specific background, border, and foreground tokens
Adopt upstream fixes and refactors through upstream/main. Preserve fork owned GitHub panel and Omarchy aligned workflow seams where upstream and fork diverge.
Define how this fork adopts, adapts, or rejects upstream changes while preserving Omarchy specific product and design decisions. Policy-Ref: governance/commit_policy.md Discussion: user request on 2026-03-13
Prevent the thread route from redirecting away during the gap between the first local draft send and the next snapshot sync. Add browser regression coverage for the first-send path.
- Null stale persisted worktree paths during snapshot reads - Keep new thread route stable after first send before snapshot sync
- Add shared GitHub repository selector parsing and resolution logic - Prefer `upstream` over `origin` when choosing the target repo for PR operations - Pass explicit `--repo` values to `gh pr list`, `gh pr view`, `gh pr checkout`, `gh pr create`, and default branch lookup - Add manager and repository tests covering fork plus upstream behavior
- Validate PNG structure before accepting screenshot bytes - Keep polling while Omarchy file is still partial - Add coverage for partial file then completed file flow
github-actions
bot
added
size:XXL
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
Author
|
wrong repo, apologies. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Comment on lines
+59
to
+69
| export function watchDesktopSystemTheme( | ||
| onChange: (theme: DesktopSystemTheme | null) => void, | ||
| ): () => void { | ||
| if (process.platform !== "linux") { | ||
| onChange(null); | ||
| return () => undefined; | ||
| } | ||
|
|
||
| let closed = false; | ||
| let lastSerializedTheme = serializeTheme(readDesktopSystemTheme()); | ||
| let reloadTimer: ReturnType<typeof setTimeout> | null = null; |
Contributor
There was a problem hiding this comment.
🟢 Low src/omarchyTheme.ts:59
On Linux, watchDesktopSystemTheme stores the initial theme in lastSerializedTheme but never calls onChange with it, so callers never receive the starting theme value. Only subsequent file changes trigger the callback. Consider emitting the initial theme immediately so the behavior matches the non-Linux path.
+ let lastSerializedTheme = "";
+ emitIfChanged();
+ lastSerializedTheme = serializeTheme(readDesktopSystemTheme());🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/src/omarchyTheme.ts around lines 59-69:
On Linux, `watchDesktopSystemTheme` stores the initial theme in `lastSerializedTheme` but never calls `onChange` with it, so callers never receive the starting theme value. Only subsequent file changes trigger the callback. Consider emitting the initial theme immediately so the behavior matches the non-Linux path.
Evidence trail:
apps/desktop/src/omarchyTheme.ts (commit REVIEWED_COMMIT):
- Lines 60-62: non-Linux path calls `onChange(null)` immediately
- Line 66: Linux path stores initial theme in `lastSerializedTheme = serializeTheme(readDesktopSystemTheme())`
- Lines 69-74: `emitIfChanged()` is the only place `onChange` is called
- Lines 76-82: `scheduleReload()` is the only place `emitIfChanged()` is called
- Lines 88-93: `scheduleReload()` is only called from file watcher callbacks
- No call to `onChange` or `emitIfChanged()` exists in the Linux initialization path
Comment on lines
+297
to
+301
| const configuredDirectory = | ||
| picturesDirectoryMatch?.[1] ?? picturesDirectoryMatch?.[2] ?? picturesDirectoryMatch?.[3]; | ||
| if (configuredDirectory) { | ||
| return Path.resolve(expandUserPath(configuredDirectory.trim())); | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Low src/screenshotCapture.ts:297
When user-dirs.dirs contains XDG_PICTURES_DIR= (whitespace only), the regex captures the space character which is truthy, so line 299 passes the check but line 300's trim() produces an empty string. Path.resolve("") then returns the current working directory instead of falling back to ~/Pictures. Consider trimming before the truthiness check.
const configuredDirectory =
picturesDirectoryMatch?.[1] ?? picturesDirectoryMatch?.[2] ?? picturesDirectoryMatch?.[3];
if (configuredDirectory) {
- return Path.resolve(expandUserPath(configuredDirectory.trim()));
+ return Path.resolve(expandUserPath(configuredDirectory).trim());
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/src/screenshotCapture.ts around lines 297-301:
When `user-dirs.dirs` contains `XDG_PICTURES_DIR= ` (whitespace only), the regex captures the space character which is truthy, so line 299 passes the check but line 300's `trim()` produces an empty string. `Path.resolve("")` then returns the current working directory instead of falling back to `~/Pictures`. Consider trimming before the truthiness check.
Evidence trail:
apps/desktop/src/screenshotCapture.ts lines 290-305 (REVIEWED_COMMIT) - shows the regex, truthiness check on configuredDirectory before trim(), and the fallback to ~/Pictures. apps/desktop/src/screenshotCapture.ts lines 87-92 - shows expandUserPath() which just does string replacements and would return empty string for empty input. Node.js Path.resolve("") returns process.cwd() per standard Node.js behavior.
Comment on lines
+4607
to
+4614
| const MessageCopyButton = memo(function MessageCopyButton({ text }: { text: string }) { | ||
| const [copied, setCopied] = useState(false); | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| void navigator.clipboard.writeText(text); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }, [text]); |
Contributor
There was a problem hiding this comment.
🟢 Low components/ChatView.tsx:4607
navigator.clipboard.writeText throws TypeError when called in non-HTTPS contexts or unsupported browsers where the Clipboard API is unavailable. Consider guarding the call with a check for navigator.clipboard availability, similar to the pattern used in ChatMarkdown.tsx.
const handleCopy = useCallback(() => {
+ if (typeof navigator === "undefined" || navigator.clipboard == null) {
+ return;
+ }
void navigator.clipboard.writeText(text);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
}, [text]);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around lines 4607-4614:
`navigator.clipboard.writeText` throws `TypeError` when called in non-HTTPS contexts or unsupported browsers where the Clipboard API is unavailable. Consider guarding the call with a check for `navigator.clipboard` availability, similar to the pattern used in `ChatMarkdown.tsx`.
Evidence trail:
ChatView.tsx lines 4600-4620 at REVIEWED_COMMIT shows `handleCopy` calling `void navigator.clipboard.writeText(text)` without guards. ChatMarkdown.tsx lines 135-150 at REVIEWED_COMMIT shows the guarded pattern: `if (typeof navigator === "undefined" || navigator.clipboard == null) { return; }` before calling `navigator.clipboard.writeText(code)`.
| {questionIndex + 1}/{prompt.questions.length} {activeQuestion.header} |
Contributor
There was a problem hiding this comment.
🟢 Low components/ChatView.tsx:4562
Line 4562 displays questionIndex + 1 for the progress indicator, but activeQuestion is derived from progress.questionIndex (the normalized, clamped index). When questionIndex is out of bounds, the UI shows a mismatched number like "11/3" while displaying question 3. Use progress.questionIndex + 1 instead.
- {questionIndex + 1}/{prompt.questions.length} {activeQuestion.header}
+ {progress.questionIndex + 1}/{prompt.questions.length} {activeQuestion.header}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 4562:
Line 4562 displays `questionIndex + 1` for the progress indicator, but `activeQuestion` is derived from `progress.questionIndex` (the normalized, clamped index). When `questionIndex` is out of bounds, the UI shows a mismatched number like "11/3" while displaying question 3. Use `progress.questionIndex + 1` instead.
Evidence trail:
ChatView.tsx lines 4550-4562 (REVIEWED_COMMIT) - shows `questionIndex + 1` is used in the progress display. pendingUserInput.ts lines 92-121 (REVIEWED_COMMIT) - shows `derivePendingUserInputProgress` normalizes `questionIndex` via `Math.max(0, Math.min(questionIndex, questions.length - 1))` and returns it as `progress.questionIndex`. The `activeQuestion` is derived from `questions[normalizedQuestionIndex]`, not the raw `questionIndex`.
| return option.available && option.value !== "claudeCode"; |
Contributor
There was a problem hiding this comment.
🟢 Low components/ChatView.tsx:5549
isAvailableProviderOption narrows value to ProviderKind in its return type, but the runtime check only excludes "claudeCode" and not "cursor". If the cursor option ever has available: true, the function returns true while incorrectly asserting the value is ProviderKind, causing props.modelOptionsByProvider[option.value] to access an undefined property since that record is keyed by ProviderKind. Add option.value !== "cursor" to match the type assertion.
- return option.available && option.value !== "claudeCode";
+ return option.available && option.value !== "claudeCode" && option.value !== "cursor";🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 5549:
`isAvailableProviderOption` narrows `value` to `ProviderKind` in its return type, but the runtime check only excludes `"claudeCode"` and not `"cursor"`. If the `cursor` option ever has `available: true`, the function returns `true` while incorrectly asserting the value is `ProviderKind`, causing `props.modelOptionsByProvider[option.value]` to access an undefined property since that record is keyed by `ProviderKind`. Add `option.value !== "cursor"` to match the type assertion.
Evidence trail:
- apps/web/src/components/ChatView.tsx lines 5544-5549: `isAvailableProviderOption` type guard with return type asserting `value: ProviderKind` but runtime check only excludes `"claudeCode"`
- packages/contracts/src/orchestration.ts line 30: `ProviderKind = Schema.Literal("codex")` - only "codex" is a valid ProviderKind
- apps/web/src/session-logic.ts lines 18-27: `ProviderPickerKind = ProviderKind | "claudeCode" | "cursor"` and `PROVIDER_OPTIONS` array with all three values
- apps/web/src/components/ChatView.tsx line 5196: `modelOptionsByProvider: Record<ProviderKind, ...>` (keyed only by `"codex"`)
- apps/web/src/components/ChatView.tsx lines 5707, 5714, 873: usage of `props.modelOptionsByProvider[option.value]`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maininto this branch and resolve merge falloutgovernance/upstream_merge_policy.mdTesting
bun fmtbun lintbun typecheckNote
Add upstream repo routing for git operations and introduce a Git/GitHub panel with promote workflow
upstreambeforeorigin, so forks correctly target the upstream repo via--repo.git-panel/) with workspace management, branch sync, merge conflict handling, promote-to-target, GitHub auth, and issues browsing, wired via newNativeApiwebsocket methods.promotestacked action that merges a feature branch into a target, pushes, and deletes the source branch, or surfaces conflicted files on failure.~/.config/omarchy/theme files, exposes system theme via IPC, applies colors as CSS variables on:root, and syncs dark/light mode from the desktop theme.PrintScreenkey or camera button) with image validation (type, size, count) and error feedback in the composer.getServerHttpOriginnow always usesws://in browser (non-Electron) mode regardless of page protocol, which breaks secure WebSocket connections on HTTPS deployments.📊 Macroscope summarized 1a98935. 54 files reviewed, 10 issues evaluated, 1 issue filtered, 5 comments posted
🗂️ Filtered Issues
apps/desktop/src/screenshotCapture.ts — 1 comment posted, 3 evaluated, 1 filtered
catchblock at lines 330-335 returns an empty array for anyENOENTerror, butENOENTcan also be thrown byFS.stat()on line 319 if a file is deleted betweenreaddirandstat. This conflates "directory doesn't exist" (intended case) with "individual file disappeared during iteration" (unintended case). If any file in the directory is deleted during the listing operation, the function incorrectly returns an empty array instead of the remaining files, potentially causingchangedScreenshotFile()to miss detecting the actual screenshot file. [ Out of scope (triage) ]