Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review mode: initial
Findings
-
[Major] This PR commits the Codex agent workspace to the public repo — all 31 changed files live under
.Codex/workspace/**and are maintainer-local agent artifacts: PR-body drafts, a verbatim copy of the prior bot review, planning notes, and a ~87 KB validation bundle with PNG evidence. None of it is product source or public documentation, and the repo convention keeps agent/private working state out of the public tree (seeAGENTS.md, which treatsdocs/as internal/gitignored; the same intent applies to.Codex/). Landing this adds ~2,900 lines of noise, leaks workflow internals, and gives future contributors the impression that agent scratch state is version-controlled. Evidence:.Codex/workspace/bot433-reply.md:1,.Codex/workspace/pr433-bot-review.md:1,.Codex/workspace/validation/source-edit-browser/attempt-2026-09-21T16-11-19-211Z/verification.json:1.
Suggested fix: drop.Codex/workspace/**from this PR and add.Codex/(or at least.Codex/workspace/) to.gitignoreso agent scratch state stays local:git rm -r --cached .Codex printf '\n# Local agent workspace\n.Codex/\n' >> .gitignore -
[Minor] The committed validation evidence embeds a workstation-specific absolute path —
.Codex/workspace/validation/source-edit-browser/attempt-2026-09-21T16-11-19-211Z/persists-an-exact-text-patch-through-the-full-FilesTab-UI.txt:2showsC:\Users\hjh\AppDa…r-qLNxfB\workspace, repeated across the sibling.txtcaptures. Local user paths should not be published; this is resolved automatically by removing.Codex/workspace/**as above. -
[Nit] The PR body is the unmodified pull-request template — no summary, no type-of-change box selected, and the PR is only labeled
docs. If anything here is genuinely meant to be published, the description must state the why; otherwise use this PR to remove the files instead.
Questions
- Is the intent to publish these notes, or was
.Codex/workspace/committed by accident while archiving PR #433 material? If the latter, closing the PR and gitignoring.Codex/is simplest. (I could not verify from the public checkout whether.gitignorealready lists.Codex/.)
Summary
- Review mode: initial. No product code, tests, dependencies, config, release, or distribution files are touched, so there is no runtime, security, license, or packaging path to validate.
- The single substantive issue is scope: the entire diff is the Codex agent workspace under
.Codex/workspace/**. Recommend closing this PR, or rewriting it to remove those files and add.Codex/to.gitignore. No changeset is needed since nothing user-visible changes. - Residual risk: if
.gitignorealready ignores.Codex/, these files were force-added and should still be removed rather than kept.
Testing
- Not run (automation). No tests are warranted for agent workspace notes; the correct outcome is removal, not coverage.
Open-CoDesign Bot
github-actions
Bot
added
the
area:desktop
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
-
[Major] This PR still commits the Codex agent workspace to the public repo, and the follow-up grew it rather than reducing it — the diff is
.Codex/workspace/**scratch notes (PR-body drafts, plans, a verbatim copy of the prior bot review) plus a ~87 KB browser-validation bundle (verification.jsonand PNG/TXT captures). These are maintainer-local agent artifacts, not product source or public documentation; the repo treats this class of internal working state as non-public (AGENTS.md). Landing it publishes ~2,900 lines of noise and exposes workflow internals. Evidence:.Codex/workspace/credential-boot-fix.md:1,.Codex/workspace/pr433-main-sync.md:1,.Codex/workspace/validation/source-edit-browser/attempt-2026-09-21T16-11-19-211Z/verification.json:1. The prior review's [Major] on this remains unaddressed — no.gitignorechange is included in the diff.
Suggested fix: remove.Codex/workspace/**from the PR and ignore the directory so agent scratch state stays local:git rm -r --cached .Codex printf '\n# Local agent workspace\n.Codex/\n' >> .gitignore -
[Minor] The committed validation captures embed a workstation-specific absolute path —
C:\Users\hjh\AppDa…r-qLNxfB\workspace— repeated across the sibling.txtfiles. Evidence:.Codex/workspace/validation/source-edit-browser/attempt-2026-09-21T16-11-19-211Z/persists-an-exact-text-patch-through-the-full-FilesTab-UI.txt:2. Local user paths should not be published; this is resolved automatically by removing.Codex/workspace/**. -
[Minor] The PR also modifies
apps/desktop/src/renderer/src/components/chat/PromptInput.browser.test.ts(dev-server →build+preview, plus added console/request diagnostics) and addsapps/desktop/src/renderer/src/components/chat/__fixtures__/active-message-browser.html. These are unrelated to "archive workspace notes and validation evidence" and are not described in the PR body. If intentional, split them into their own PR with a description; if they are incidental branch content, drop them from this one. -
[Nit] The PR body is still the unmodified pull-request template — no summary/why, no type-of-change box selected. The previous review noted this as well; if anything here is genuinely meant to be published, the description must state the purpose.
Questions
- Is the intent to publish
.Codex/workspace/content, or was this committed by accident while archiving PR #433 material? If accidental, closing this PR and adding.Codex/to.gitignoreis the simplest resolution. (I could not verify from the public checkout whether.gitignorealready lists.Codex/.)
Summary
- Review mode: follow-up after new commits. The prior [Major] scope finding (private agent artifacts under
.Codex/workspace/**) is unresolved and now larger, and the prior [Minor] workstation-path leak persists. Aside from the twoapps/desktoptest/fixture files, the diff adds no product code, dependencies, config, release, or distribution files, so there is no runtime, security, license, or packaging path to validate here. Recommend removing.Codex/workspace/**and gitignoring.Codex/before this is considered for merge. - Residual risk: if
.Codex/is already ignored, these files were force-added and should still be removed rather than kept. Not found in the public repo I can see whether.gitignorealready covers.Codex/.
Testing
- Not run (automation). No tests are warranted for agent workspace notes; the correct outcome is removal, not coverage. If the
PromptInput.browser.test.tsrefactor is kept, it belongs in a separate PR so CI exercises it on its own.
Open-CoDesign Bot
CI failure follow-up — 236ef0cThe failing run timed out in the first PromptInput browser test's beforeEach while waiting for textarea, before any locale/layout assertion. The other cases passed. The original fixture served raw HTML from a Vite development middleware, so the readiness fetch did not prepare its module graph. Cold dependency optimization can invalidate in-flight imports; the existing TweakPanel browser suite already avoids this class of startup race with build + preview. The old logs did not capture failed HTTP responses, so a specific failed chunk/status cannot be proven retrospectively. This update builds the existing composer fixture into a unique temporary directory before launching preview/Chrome, retains production CSS and all 18 existing behavioral assertions, adds HTTP/request/console diagnostics for mounting failures, and cleans both build output and browser profile. No retries, longer timeouts, product-code changes, or dependencies were added. The branch also merges current main without rewriting history. Validation:
Compatibility: green — no runtime behavior changes. This follow-up addresses the test failure only; the original archive contents are unchanged. |
This branch has not been deployed
1 participant
Summary
Type of change
Linked issue
Checklist
pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleDependency additions (if any)
Screenshots / recordings (UI changes)