Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 273fbae0ac
ℹ️ 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 (outcome.ok) { | ||
| this.params.onReported?.({ reportId: outcome.reportId, issueUrl: outcome.issueUrl }); |
There was a problem hiding this comment.
Link the originating issue in a nearby explanatory comment and in the it(...) descriptions covering both outcomes of this new success-only callback branch. The commit references issue 672, but neither the branch nor its success/failure tests preserve that URL as required for newly introduced behavioral paths.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in a6a4b00: an issue-linked comment on the outcome.ok branch, and the issue URL in both the success and failure it(...) descriptions.
| const outcome = await uploadReport(this.params.uploader, report); | ||
| this.uploading = false; | ||
| if (outcome.ok) { | ||
| this.params.onReported?.({ reportId: outcome.reportId, issueUrl: outcome.issueUrl }); |
There was a problem hiding this comment.
Guard this notification callback so a synchronous exception cannot reject upload() after the server has already accepted the report. In that scenario ReviewPage.runUpload() has no rejection handler and remains in the uploading state instead of opening the issue; if the dialog was closed mid-upload, the exception also skips the fallback notice containing the otherwise unrecoverable report ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed. The only caller (#3566) passes AgentSession.markFeedbackReported, a synchronous state update with nothing that can throw, so a try/catch would guard a case that cannot happen.
| label: "OpenCode backend log", | ||
| description: "may include unrelated sessions", | ||
| defaultChecked: false, | ||
| defaultChecked: this.params.preselectAllSources === true, |
There was a problem hiding this comment.
When debugFullFrames is false, the activity-log source remains visible but defaultChecked is false, and this new option only overrides the OpenCode source here. Consequently a feedback-card caller requesting preselectAllSources: true still receives a form where the activity log is unticked, contrary to the option's all-sources contract; apply the option consistently when constructing every source rather than special-casing one backend log.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed on purpose. When the activity log is turned off it records nothing: activityLogRequest returns only unavailableReason. Ticking it would attach an 'unavailable' note and no data. The option ticks every source that has something to attach.
| dismissSettings: () => { | ||
| (app as unknown as { setting: { close: () => void } }).setting.close(); |
There was a problem hiding this comment.
Make Settings dismissal caller-specific instead of hard-wiring it into the reusable opener. When a feedback card in an Agent pane popout opens this modal while Settings remains open in the main window, the report overlay and capture target share the popout, so screenshot preparation invokes this callback and unexpectedly closes the unrelated Settings window even though the report did not originate there.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a6a4b00. openReportIssueModal takes an origin element. When one is passed, the dialog captures the Agent leaf that contains it and leaves Settings alone. Only the Settings button, which passes no origin, still closes Settings. Covered by "captures the chat a report was opened from and leaves Settings open".
273fbae to
a6a4b00
Compare
a6a4b00 to
7cee4d6
Compare
openReportIssueModal builds the dialog from the app alone, so callers outside settings can open it. Callers can seed the note, pre-check every attachment and hear when an upload lands. Settings now goes through it. Relates to Brevilabs/obsidian-copilot-private#672 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop two assertions that checked a private params shape or repeated the default-unchecked test, and link issue #672 on the preselectAllSources branch and its test. The rebase onto master also drops the unused ReportIssueModal re-export from the agentMode barrel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The report's screenshot is now the window of the pane the user was last working in, with every open dialog (Settings included) hidden for the capture. Errors outside the chat are now visible, and no caller has to say which Agent pane to photograph, so the origin, leaf-reveal, capture-target and settle-polling plumbing is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b56c954 to
22f7b7f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22f7b7f5df
ℹ️ 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".
| const all = remote.BrowserWindow?.getAllWindows?.() ?? []; | ||
| if (all.length === 0) return current; | ||
| if (all.length === 1) return all[0]; |
There was a problem hiding this comment.
Add a captureWindowBehindDialogs() test using a non-global Window and multiple mocked BrowserWindows. The current tests exercise only the domWindow === window shortcut, while the modal test mocks this function entirely, so the new geometry-based popout selection—including its zero-, one-, and multiple-window branches—has no observable unit coverage even though it determines which potentially sensitive window is captured.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
Each Obsidian window exposes window.electron, whose remote.getCurrentWindow()
is that window's BrowserWindow. Reading it from the target window drops the
require("electron") call and the bounds matching that guessed which
BrowserWindow a popout belonged to.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
openReportIssueModal takes the plugin and reads manifest.version like the rest of the codebase, instead of reaching through app.plugins.getPlugin with an untyped cast. Settings gets the plugin from usePlugin(), which also replaces its own getPlugin lookup of the active session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
1 participant
Relates to Brevilabs/obsidian-copilot-private#672
Why
The feedback card in the next step sends the user to the existing Report an issue dialog with the agent's draft already written. Today that dialog can only be opened from Settings → Advanced → Report an issue: the settings tab builds it inline, and it always starts with an empty description, the OpenCode log unticked, and no way for the caller to learn that the upload landed.
Its screenshot is of the first Agent Chat pane only. An error shown anywhere else (a note, another dialog, a sidebar) never reaches the report, a report about chat B can carry a picture of chat A, and with no Agent Chat open there is no screenshot at all. Choosing the right pane would mean every caller telling the dialog where it was opened from.
What
Any Copilot surface can now open Report an issue, optionally with a drafted description, every attachment ticked, and a callback when the upload succeeds. The settings button opens it as it does today.
The screenshot now shows the whole window, so no caller has to name a pane:
Non goal
Screenshot
The drafted state, from the component gallery story
Agent Mode/Report Issue Flow/DraftedNote:The
screenshot.pngbundled from Settings → Report an issue, opened over a note with Settings inside the main window. Settings and the dialog are hidden for the capture:Risk
Medium
ReportIssueModal,ReportIssueFlowandreportScreenshottests cover the drafted note, the pre-ticked OpenCode log, the upload callback, which window is captured, and hiding and restoring dialogs; whether the hidden dialogs have been painted out before Electron reads the frame can only be checked in a real Obsidian windowscreenshot.pngReview: read
captureWindowBehindDialogsinsrc/agentMode/ui/reportScreenshot.tsandprepareinsrc/agentMode/ui/ReportIssueModal.tsx, then run Verification steps 2-4.Verification
screenshot.png. It shows the whole main window with your note, without Settings or the dialog, and both are back on screen afterwards. Press Cancel.