Sync upstream main into codex and verify rebaseability#16
Merged
DavidUmKongs merged 70 commits intocodexfrom Mar 25, 2026
Merged
Sync upstream main into codex and verify rebaseability#16DavidUmKongs merged 70 commits intocodexfrom
DavidUmKongs merged 70 commits intocodexfrom
Conversation
* feat: add bypass permissions mode for agents Right-click the "+ Agent" button to choose between Normal and "⚡ Bypass Permissions" launch modes. Bypass passes --dangerously-skip-permissions to the claude CLI, skipping all tool-call approval prompts. * fix: bypass menu multi-folder support, CSS variable, hover states * fix: add bypass docs --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Florin Timbuc <florin@sowild.design>
* feat: add support for external asset packs Persist and load furniture assets from user-defined directories outside the extension, enabling third-party asset packs to be used alongside built-in furniture. - Add configPersistence.ts to read/write ~/.pixel-agents/config.json - Load external asset dirs on boot and merge with bundled assets - Add/remove directories via Settings modal with live palette refresh - Add docs/external-assets.md covering the manifest format and usage Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: Windows path display, asset ID dedup, path traversal defense in external assets --------- Co-authored-by: Marc teBoekhorst <marctebo@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Florin Timbuc <florin@sowild.design>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…e JSONL interference - getProjectDirPath now validates cwd/workspace exists before returning (returns null if not) - Extract isRelevantToWorkspace() helper to check session_meta.cwd in JSONL files - Apply workspace filtering during ensureProjectScan initial seed (previously seeded ALL files) - Deduplicate workspace check in scanForNewJsonlFiles using shared helper Fixes PR #2 review comment about cross-workspace agent-to-file mapping corruption when multiple VS Code windows share the same ~/.codex/sessions/ directory. Amp-Thread-ID: https://ampcode.com/threads/T-019cfe31-cba5-76c0-8ed6-1c0caf42519b Co-authored-by: Amp <amp@ampcode.com>
…nused constant - Add CODEX_DIR_NAME, CODEX_SESSIONS_DIR_NAME, CODEX_SESSION_COMMAND to constants.ts - Replace inline '.codex', 'sessions', 'codex --session-id' in agentManager.ts - Remove unused TERMINAL_NAME_PREFIX_CLAUDE constant Addresses PR #2 review comments from Devin, Gemini, and CodeRabbit about inline magic strings and unused constants. Amp-Thread-ID: https://ampcode.com/threads/T-019cfe31-cba5-76c0-8ed6-1c0caf42519b Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Phase A — Click/Selection flow normalization: - Changed handleClick to dispatch agentSelected as local window event instead of posting to vscode extension (selection is webview-internal) - Terminal focus sync from onDidChangeActiveTerminal still works Phase B — Tool history preservation: - Added TOOL_HISTORY_MAX_SIZE (20) constant - Added agentToolHistory and subagentToolHistory state - On agentToolDone/subagentToolDone: completed tools appended to history - On agentToolsClear: only active tools cleared, history preserved - On agentClosed: both active tools and history cleaned up - ToolOverlay merges past history with active tools (shows latest 5) - DebugView merges history for full timeline view Amp-Thread-ID: https://ampcode.com/threads/T-019d045f-1fa1-74ee-8425-ef53073db9ac Co-authored-by: Amp <amp@ampcode.com>
Phase C — Propagate heuristic metadata to webview: - agentToolPermission handler now reads source/inferred/confidence from extension message and sets them on affected ToolActivity items - subagentToolPermission handler does the same for sub-agent tools - Permission tools marked with source='heuristic', inferred=true, confidence='low' when originating from timer-based detection Phase D (partial) — Inspector heuristic display: - Status line shows '(heuristic)' suffix when status is inferred - Permission line shows '(estimated)' when tool permission is inferred - Heuristic-sourced tools show 'Auto (estimated)' label Amp-Thread-ID: https://ampcode.com/threads/T-019d045f-1fa1-74ee-8425-ef53073db9ac Co-authored-by: Amp <amp@ampcode.com>
Pass emitActiveStatus=true in clearAgentActivity so the webview receives the agentStatus:active message during agent reassignment, preventing the UI from staying stuck on 'waiting' after /clear. Co-Authored-By: Oz <oz-agent@warp.dev>
… constants.ts Co-Authored-By: 엄준호 <joonho.um@konglabs.net>
A newly launched Codex session could create its JSONL file before the scan loop was armed, leaving the agent attached to a pending path with no transcript events while the office character still started in a working pose. This change arms scanning before launching Codex, keeps fresh agents visually inactive until a real status arrives, and shows the current lifecycle in Recent actions when work has no tool history yet. Constraint: Codex session files can appear almost immediately after the terminal command starts Rejected: Increase scan frequency or rely on retry timing alone | does not remove the launch-time race Confidence: high Scope-risk: moderate Directive: Keep terminal launch ordering and initial character activation aligned with the first real transcript/status event Tested: npm run lint; npm run check-types; cd webview-ui && npm test; cd webview-ui && npm run lint; cd webview-ui && npm run build Not-tested: Manual VS Code launch flow against a live Codex session in this environment
TypeScript was walking up into parent node_modules and picking up unrelated ambient types, which made this repo's typecheck fail in a nested workspace. Restricting type roots keeps the build scoped to the repo's own dependencies. The repo also advertised Node 22.12.0 even though the locked ESLint stack now requires the 22.13.0 line. Aligning .nvmrc, engine fields, and README guidance makes fresh installs less misleading. Constraint: package-lock engine ranges now require ^22.13.0 for the lint toolchain Rejected: Keep .nvmrc and engine docs at 22.12.0 | leaves npm engine warnings and setup confusion in place Confidence: high Scope-risk: narrow Directive: Keep version pins and README setup steps aligned with lockfile engine requirements Tested: npm ci --prefer-offline (root); npm ci --prefer-offline (webview-ui); npm run build; npm run check-types; npm run lint; npm run lint:webview (warnings only); cd webview-ui && npm test Not-tested: Exact build run under Node 22.13.0
The webview build was pinned to Vite 8 and plugin-react 6, which require newer Node releases than the workstation environment used here. Downgrade that toolchain to a Node 20.11-compatible pair, refresh the lockfile, and align the documented minimum engine version with the versions that now build successfully while keeping Node 22.13.0 as the recommended nvm-pinned target. Constraint: npm run build needed to pass under the current Node 20.11.1 environment Rejected: Keep Vite 8 and require a Node upgrade | does not fix the reported local build failure Confidence: high Scope-risk: narrow Reversibility: clean Directive: If upgrading Vite again, re-verify the minimum supported Node version before changing engines/docs Tested: npm run build; cd webview-ui && npm test Not-tested: webview-ui lint warning cleanup (67 existing warnings remain)
The webview had accumulated repeated inline color literals and a stale hook dependency warning, which made lint output noisy and hid real regressions. Centralize the repeated UI/matrix colors into shared constants, reuse them across the affected components, and switch the message hook to ref-backed callbacks so the layout listener stays stable without suppressing lint feedback. Constraint: Cleanup had to preserve existing editor/rendering behavior while removing warnings Rejected: Disable the lint rules | would hide valid future regressions instead of cleaning the code Confidence: high Scope-risk: narrow Reversibility: clean Directive: Prefer shared UI/constants for future webview styling changes instead of new inline color literals Tested: cd webview-ui && npm run lint; npm run build; cd webview-ui && npm test Not-tested: Manual visual regression pass in VS Code webview
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough외부 자산 디렉토리 지원 및 사용자 설정 지속성(JSON config) 추가, 웹뷰↔확장 메시지/핸들러·UI(설정 모달·에이전트 선택) 확장, 어댑터·에이전트 복원·트랜스크립트 처리 조정, GitHub 템플릿·워크플로우·문서·메타데이터 업데이트가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebView as WebView UI
participant Extension as PixelAgentsViewProvider
participant Config as Config 저장소
participant FS as FileSystem
participant AssetLoader as assetLoader
User->>WebView: 설정 모달 열고 "자산 디렉토리 추가" 클릭
WebView->>Extension: addExternalAssetDirectory 메시지
Extension->>FS: 폴더 선택기 열기
FS-->>Extension: 선택된 폴더 경로 반환
Extension->>Config: readConfig()
Config-->>Extension: 현재 설정 반환
Extension->>Config: writeConfig(업데이트된 dirs)
Config->>FS: ~/.pixel-agents/config.json에 기록
FS-->>Config: 파일 쓰기 성공
Extension->>AssetLoader: loadAllFurnitureAssets()
AssetLoader->>FS: 번들/외부 디렉토리에서 자산 로드
AssetLoader->>AssetLoader: mergeLoadedAssets()
AssetLoader-->>Extension: 병합된 자산 전달
Extension->>WebView: externalAssetDirectoriesUpdated + furniture data
WebView->>User: UI 업데이트(목록 및 자산 반영)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
시 🐰
🚥 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 unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Pixel Agents extension by adding support for multiple AI coding agent backends (Codex and Claude), improving agent visibility through UI enhancements, and allowing users to customize their experience with external asset directories. It also includes various bug fixes and code improvements to ensure stability and maintainability. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/assetLoader.ts (1)
136-154:⚠️ Potential issue | 🟠 Major경로 검증이 symlink 우회를 막지 못합니다.
Line 137-147의
path.resolve기반 검사만으로는 디렉터리 내부 symlink가 외부 파일을 가리키는 경우를 차단하지 못합니다.realpath기준으로 경계 검증하고, 실제 읽기도 정규화된 경로를 사용해야 합니다.🔒 제안 패치
for (const asset of assets) { try { const assetPath = path.join(itemDir, asset.file); - const resolvedAsset = path.resolve(assetPath); - const resolvedDir = path.resolve(itemDir); - if ( - !resolvedAsset.startsWith(resolvedDir + path.sep) && - resolvedAsset !== resolvedDir - ) { + if (!fs.existsSync(assetPath)) { + console.warn(` ⚠️ Asset file not found: ${asset.file} in ${dir.name}`); + continue; + } + + const resolvedDir = fs.realpathSync(itemDir); + const resolvedAsset = fs.realpathSync(assetPath); + const relative = path.relative(resolvedDir, resolvedAsset); + if (relative.startsWith('..') || path.isAbsolute(relative)) { console.warn( ` [AssetLoader] Skipping asset with path outside directory: ${asset.file}`, ); continue; } - if (!fs.existsSync(assetPath)) { - console.warn(` ⚠️ Asset file not found: ${asset.file} in ${dir.name}`); - continue; - } - const pngBuffer = fs.readFileSync(assetPath); + const pngBuffer = fs.readFileSync(resolvedAsset); const spriteData = pngToSpriteData(pngBuffer, asset.width, asset.height); sprites.set(asset.id, spriteData); } catch (err) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assetLoader.ts` around lines 136 - 154, The path validation can be bypassed via symlinks because path.resolve is insufficient; change the check to use real filesystem paths: call fs.realpathSync (or async realpath) on both itemDir (or resolvedDir) and assetPath to get realDir and realAsset, verify realAsset startsWith realDir + path.sep or equals realDir, and only then proceed; also use the realAsset path for exists/read operations (fs.existsSync and fs.readFileSync) and pass the resulting buffer to pngToSpriteData so all file access and boundary checks are performed against canonicalized paths (referencing assetPath, resolvedDir/realDir, resolvedAsset/realAsset, pngBuffer, and pngToSpriteData).webview-ui/src/office/components/OfficeCanvas.tsx (1)
670-734:⚠️ Potential issue | 🟠 Major선택 해제 분기에서 상위 콜백 누락으로 상태 동기화가 깨질 수 있습니다
Line 690에서는 콜백을 호출하지만, 같은 핸들러 내에서 좌석 재배치/빈 공간 클릭으로
selectedAgentId를null로 바꾸는 분기(Line 705-716, Line 730-731)에서는onClick(null)이 호출되지 않습니다. 상위 컴포넌트가 선택 상태를 별도로 들고 있으면 UI가 stale 상태가 될 수 있습니다.💡 제안 패치
@@ if (selectedCh.seatId === seatId) { // Clicked own seat — send agent back to it officeState.sendToSeat(officeState.selectedAgentId); officeState.selectedAgentId = null; officeState.cameraFollowId = null; + onClick(null); return; } else if (!seat.assigned) { // Clicked available seat — reassign officeState.reassignSeat(officeState.selectedAgentId, seatId); officeState.selectedAgentId = null; officeState.cameraFollowId = null; @@ vscode.postMessage({ type: 'saveAgentSeats', seats }); + onClick(null); return; } @@ // Clicked empty space — deselect officeState.selectedAgentId = null; officeState.cameraFollowId = null; + onClick(null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/src/office/components/OfficeCanvas.tsx` around lines 670 - 734, The click handler handleClick clears officeState.selectedAgentId in several branches (after sendToSeat, after reassignSeat + postMessage, and the final deselect-for-empty-space) but doesn't notify the parent via onClick, causing external state to go stale; update those branches in handleClick to call onClick(null) immediately after you set officeState.selectedAgentId = null (and cameraFollowId = null) so the parent stays in sync (refer to officeState.sendToSeat, officeState.reassignSeat, the postMessage block, and the final deselect branch).src/agentManager.ts (1)
32-49:⚠️ Potential issue | 🟠 Major
bypassPermissions가 실제 실행 명령으로 전달되지 않습니다.Line 48에서 옵션을 받지만 Line 123에서는
adapter.getTerminalCommand(sessionId)만 호출해서 값이 완전히 버려집니다. 현재 상태로는 bypass mode를 켜도 새 터미널 실행 방식이 바뀌지 않습니다.🔧 제안 수정
- terminal.sendText(adapter.getTerminalCommand(sessionId)); + terminal.sendText(adapter.getTerminalCommand(sessionId, { bypassPermissions }));
AgentAdapter인터페이스와 각 adapter 구현도 같은 시그니처로 확장해 두는 편이 좋겠습니다.Also applies to: 123-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentManager.ts` around lines 32 - 49, The launchNewTerminal function currently accepts bypassPermissions but never forwards it to adapter.getTerminalCommand, so enabling bypassPermissions has no effect; update launchNewTerminal to pass bypassPermissions into adapter.getTerminalCommand(sessionId, bypassPermissions) (or similar second arg) and update the AgentAdapter interface signature and all adapter implementations to accept the new bypassPermissions parameter so the command generation can honor bypass mode (ensure function names: launchNewTerminal, adapter.getTerminalCommand, and the AgentAdapter interface are updated consistently).
🧹 Nitpick comments (8)
webview-ui/test/tool-utils.test.ts (1)
1-43:defaultZoom()회귀 테스트도 같이 고정해 두면 좋겠습니다.이번 PR에서 브라우저 전역 의존성이
window에서globalThis로 바뀌었는데, 현재 테스트는 문자열→tool/status 포맷팅만 커버합니다.devicePixelRatio가 없을 때와 비정상 값일 때 fallback이 유지되는지도 같이 잡아두면 Node 기반 테스트 경로 회귀를 막기 쉽습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/test/tool-utils.test.ts` around lines 1 - 43, Add a regression test for defaultZoom to ensure it falls back correctly when devicePixelRatio is missing or invalid: write node:test cases that import defaultZoom and temporarily set/clear globalThis.devicePixelRatio (undefined, 0, NaN, Infinity, negative) and assert defaultZoom() returns the fallback (e.g., 1) for those values; make sure to restore/delete the globalThis.devicePixelRatio after each case to avoid cross-test pollution and use assert.equal for comparisons so the Node-based test path is covered.src/PixelAgentsViewProvider.ts (1)
306-335: 새 IPC 타입도 상수로 올려주세요.
addExternalAssetDirectory,removeExternalAssetDirectory,externalAssetDirectoriesUpdated가 다시 문자열 리터럴로 추가됐습니다. 프로토콜 이름이 흩어지기 시작하면 webview/extension 양쪽을 함께 바꿔야 할 때 회귀가 나기 쉽습니다. As per coding guidelines "All backend magic numbers, strings, timing intervals, truncation limits, PNG/asset parsing values, and VS Code IDs must be centralized insrc/constants.ts".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/PixelAgentsViewProvider.ts` around lines 306 - 335, Add new IPC string constants for 'addExternalAssetDirectory', 'removeExternalAssetDirectory', and 'externalAssetDirectoriesUpdated' in src/constants.ts (exported) and replace the string literals in PixelAgentsViewProvider.ts (the message.type checks and the webview.postMessage calls) with those constants; import the constants into PixelAgentsViewProvider and use them wherever those literals appear, and update the corresponding webview/frontend code to use the same exported constants so both extension and webview share the centralized names.package.json (1)
55-72: esbuild problem matcher를 manifest에 같이 넣어주세요.
package.json을 건드린 상태에서도 inline problem matcher가 없어서node esbuild.js계열 빌드 오류가 VS Code task 진단으로 연결되지 않습니다. 이 PR처럼 manifest를 만질 때 함께 넣어두는 편이 이후 빌드 디버깅에 훨씬 낫습니다. As per coding guidelines "Inline esbuild problem matcher inpackage.jsonfor build error reporting (no separate extension needed)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 55 - 72, The package.json change adds a configuration block (see "configuration" and "pixel-agents.agentType") but omits the inline esbuild problem matcher required for VS Code to map "node esbuild.js" build errors into the Problems panel; add a "contributes.problemMatchers" entry in package.json that registers an esbuild problem matcher (matching esbuild/node output) so running your esbuild-based tasks will surface diagnostics without a separate extension—update package.json to include the problem matcher manifest alongside the existing "configuration" section.src/transcriptParser.ts (1)
194-317: Codex transcript literal들도src/constants.ts로 모아두는 편이 좋겠습니다.이 함수에
response_item,function_call,event_msg,task_started같은 backend literal이 다시 흩어져 있습니다. 포맷이 한 번 바뀌면 수정 포인트가 늘어나서 adapter/parser 쪽 누락이 생기기 쉬운 구조입니다.As per coding guidelines, "All backend magic numbers, strings, timing intervals, truncation limits, PNG/asset parsing values, and VS Code IDs must be centralized in
src/constants.ts."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/transcriptParser.ts` around lines 194 - 317, The code in processCodexTranscriptLine contains hard-coded backend literal strings (e.g., 'response_item', 'function_call', 'function_call_output', 'event_msg', 'task_started', 'task_complete', 'agent_message') and the responseItemStatusMap mapping; move these into src/constants.ts as named exports (e.g., RESPONSE_ITEM, FUNCTION_CALL, FUNCTION_CALL_OUTPUT, EVENT_MSG, TASK_STARTED, TASK_COMPLETE, AGENT_MESSAGE, RESPONSE_ITEM_STATUS_MAP) and import them into transcriptParser.ts; then replace all string occurrences and the inline responseItemStatusMap in processCodexTranscriptLine with the new constant names so the parser references centralized constants rather than literal strings.webview-ui/src/office/components/ToolOverlay.tsx (2)
267-268: 동일한 패턴이 반복됩니다.findLast()사용을 고려하세요.Line 196과 동일하게
[...mergedTools].reverse().find()가 사용되고 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/src/office/components/ToolOverlay.tsx` around lines 267 - 268, The code repeats the pattern [...mergedTools].reverse().find(...) to get the last non-done tool; replace that with Array.prototype.findLast (use mergedTools.findLast(tool => !tool.done)) to avoid creating a reversed copy and improve clarity, updating the activeTool assignment (and any identical occurrences) to use findLast while keeping the history computation (mergedTools.slice(-5).reverse()) unchanged.
314-328: 인스펙터 패널의 레이아웃 관련 매직 넘버들을 상수화하세요.
top: 12,right: 12,width: 'min(440px, ...)',maxHeight,padding: 18,zIndex: 85등의 값들이 하드코딩되어 있습니다. 코딩 가이드라인에 따르면 이러한 값들은constants.ts에 중앙 집중화되어야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/src/office/components/ToolOverlay.tsx` around lines 314 - 328, The inline style object in the ToolOverlay component contains hardcoded layout "magic numbers" (e.g., top: 12, right: 12, width: 'min(440px, calc(100% - 24px))', maxHeight: 'calc(100% - 24px)', padding: 18, zIndex: 85); extract these values into centralized constants in constants.ts (e.g., OVERLAY_TOP, OVERLAY_RIGHT, OVERLAY_MAX_WIDTH, OVERLAY_MAX_HEIGHT_PADDING, OVERLAY_PADDING, OVERLAY_Z_INDEX) and import them into ToolOverlay.tsx, then replace the literal values in the overlay style object (the object passed to the main container in the ToolOverlay component) with the new constants so layout values are centralized and maintainable.webview-ui/src/components/DebugView.tsx (2)
62-62: TimelineRail 내 매직 넘버들을 상수화하세요.
height: 44,top: 2,fontSize: 10,minWidth: 6,height: 12, 위치 오프셋24/16, 최소 너비 비율1.5등의 값들이 하드코딩되어 있습니다. 코딩 가이드라인에 따르면 이러한 렌더링/레이아웃 관련 매직 넘버는constants.ts에 중앙 집중화되어야 합니다.Also applies to: 91-92, 124-125, 135-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/src/components/DebugView.tsx` at line 62, The TimelineRail component contains multiple hardcoded layout numbers (e.g., height: 44, top: 2, fontSize: 10, minWidth: 6, height: 12, offsets 24/16, min-width ratio 1.5)—extract these magic numbers into named constants in a shared constants.ts (e.g., TIMELINE_RAIL_HEIGHT, TIMELINE_RAIL_PADDING_TOP, TIMELINE_LABEL_FONT_SIZE, TIMELINE_TICK_MIN_WIDTH, TIMELINE_TICK_HEIGHT, TIMELINE_OFFSET_LARGE, TIMELINE_OFFSET_SMALL, TIMELINE_MIN_WIDTH_RATIO) and replace the literals in TimelineRail (and the related spots around lines referenced like 91-92, 124-125, 135-138) to use those constants; update imports in DebugView.tsx to import the new constants from constants.ts and ensure names are clear and grouped for future layout changes.
45-45:DEBUG_Z상수를constants.ts로 이동하세요.
DEBUG_Z = 40이 로컬에 정의되어 있지만, 코딩 가이드라인에 따르면 모든 webview 매직 넘버는webview-ui/src/constants.ts에 중앙 집중화되어야 합니다.♻️ 제안된 수정
webview-ui/src/constants.ts에 추가:export const DEBUG_Z = 40;그리고 이 파일에서:
-const DEBUG_Z = 40; +import { DEBUG_Z } from '../constants.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview-ui/src/components/DebugView.tsx` at line 45, DEBUG_Z is defined locally in DebugView.tsx but must be centralized; add "export const DEBUG_Z = 40" to webview-ui/src/constants.ts, remove the local "const DEBUG_Z = 40" from DebugView.tsx, and import { DEBUG_Z } from 'webview-ui/src/constants' at the top of DebugView.tsx so all references in the DebugView component use the shared constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comments.json`:
- Line 1: Remove the committed auto-generated PR comment archive (the JSON blob
added to the repo root), delete it from the branch (git rm and commit), add a
.gitignore entry to prevent future commits of that artifact, and update
CI/workflow to store the review/comment dump as a build artifact or external
storage instead of committing it to source; ensure any restore or build steps
that assumed the file exist are adjusted to read from the CI artifact location.
In `@CONTRIBUTING.md`:
- Line 122: The contribution guide currently hardcodes the feature-request link
to pablodelucca/pixel-agents; update the "Open a feature request" link so it
points to this repository's issue template by using a relative URL
(/issues/new?template=feature_request.yml) instead of the upstream owner/repo
path; search CONTRIBUTING.md for other occurrences of
"pablodelucca/pixel-agents" or the same feature_request.yml link and replace
them similarly to ensure contributors open the template in this repo.
In `@docs/agent-visibility-followup-plan.md`:
- Around line 88-110: The doc uses a mismatched event name `agentToolsClear`
while the IPC spec uses `agentToolStart`, `agentToolDone`, and `agentToolClear`;
update the document and any referenced handler names to use the exact IPC
message strings (`agentToolStart`, `agentToolDone`, `agentToolClear`) so
grep/handler lookups match runtime names, and audit other occurrences (e.g.,
mentions of “ToolsClear”/plural forms) to the singular `agentToolClear` and
ensure any referenced handler identifiers (agentToolStart, agentToolDone,
agentToolClear) and the new state keys (activeAgentTools, agentToolHistory,
activeSubagentTools, subagentToolHistory) are described consistently.
In `@docs/agent-visibility-plan.md`:
- Line 6: Update the docs to match the implementation: change the "7초
permission" timer mentioned in the sentence about src/transcriptParser.ts and
agentStatus/agentToolPermission heuristics to the actual 5s permission timeout
used by the code; also clarify that activeSubagentToolNames (parentToolId →
subToolId → toolName) is cleared when data resumes or the Task completes and
that the 5s timeout triggers permission bubbles on both parent and sub-agent.
In `@README.ko.md`:
- Around line 161-162: The community links in README.ko.md currently point to
the upstream repository (the "GitHub Discussions" and "Issues" links in the
sentence on lines referencing community links); update those URLs so they point
to the fork repository instead, making them consistent with the document's
fork-first policy—replace the existing discussion and issue links with the
fork's corresponding Discussions and Issues URLs in that sentence.
In `@README.md`:
- Line 172: 현재 README의 "Issues"와 "Discussions" 링크가 업스트림을 가리키고 있어 포크된 저장소의
리포트/토론이 업스트림으로 흩어집니다; README의 해당 문장(문구 "Use **[Issues](...)** to report bugs or
request features. Join **[Discussions](...)** for questions and
conversations.")을 찾아 Issues/Discussions 링크를 이 포크의 이슈·토론 페이지로 변경하거나
상대경로(./issues, ./discussions 또는 https://github.com/<this-repo>/issues 등)로 업데이트하여
모든 버그 리포트와 토론이 현재 저장소로 들어오도록 수정하세요.
In `@src/adapters/shared.ts`:
- Around line 26-27: normalizeWorkspacePath currently uses path.normalize which
can leave trailing slashes and always applying case normalization is unsafe
across platforms; change it to use path.resolve(value) to canonicalize and
remove trailing slashes (except root) and only apply .toLowerCase() when running
on Windows (e.g., process.platform === 'win32') so that normalizeWorkspacePath
reliably canonicalizes paths while preserving case-sensitivity on POSIX systems.
In `@src/agentManager.ts`:
- Around line 239-243: The code currently defaults p.adapterName to 'claude',
which breaks restoring legacy Codex sessions; instead, when p.adapterName is
missing, infer adapterName from persisted session storage: inspect the persisted
path/property on the persisted agent object (e.g., check p.persistPath, p.path
or p.storagePath) for ".codex" vs ".claude" patterns or open the session meta
(session_meta.cwd or stored metadata) to detect Codex vs Claude JSONL layout,
and set adapterName accordingly before constructing the AgentState (refer to
adapterName, p.adapterName, AgentState and terminalRef); if no storage hint
exists, fall back to the current runtime default adapter configuration rather
than unconditionally 'claude'.
In `@src/configPersistence.ts`:
- Around line 36-49: The writeConfig function currently swallows filesystem
errors; change it to propagate failures instead so callers can react (e.g.,
prevent reload and externalAssetDirectoriesUpdated). Modify writeConfig to
either return a boolean indicating success or (preferred) remove the catch or
rethrow the caught error so the exception bubbles up; keep the existing file
write steps (getConfigFilePath, tmpPath write and rename) but do not call
console.error then suppress the error. Update callers that rely on writeConfig
(those that trigger reloads or send externalAssetDirectoriesUpdated) to handle
the thrown error or check the boolean result and abort the reload/notification
when writing fails.
In `@src/constants.ts`:
- Line 39: The function buildCodexSessionCommand currently declares an unused
parameter _sessionId which trips the noUnusedParameters rule; either remove the
parameter and change the signature to a parameterless function returning
"codex", or implement session-specific logic using the sessionId parameter
(replace _sessionId with sessionId) so the value is consumed; update the
function declaration (buildCodexSessionCommand) and any callers accordingly.
In `@src/PixelAgentsViewProvider.ts`:
- Around line 214-215: The fallback branch that reads bundled assets fails to
set this.assetsRoot, causing reloadAndSendFurniture to return early and ignore
external directory changes; update the code path that loads bundled assets (the
same area where this.assetsRoot = assetsRoot is set for the normal path) to
assign this.assetsRoot to the resolved bundled-assets directory after loading,
so reloadAndSendFurniture sees a valid assetsRoot and will proceed; apply the
same initialization fix in the other similar block referenced (the code around
the second bundled-assets handling in PixelAgentsViewProvider.ts).
In `@src/transcriptParser.ts`:
- Around line 169-171: finalizeToolActivity currently computes duration using
Date.now() at call time which inflates durations when you delay sending
TOOL_DONE events; change finalizeToolActivity(signature ToolActivityPayload,
endTime?: number) or add an endTime parameter and use that to compute duration
(durationMs = (endTime ?? Date.now()) - activity.startTime). For every delayed
send (the agentToolDone/similar delayed timeouts referenced by
finalizeToolActivity calls at the three other spots), capture const endTime =
Date.now() immediately before scheduling the setTimeout and pass that endTime
into finalizeToolActivity (or pass it into the closure so the duration uses the
captured endTime when building the payload), ensuring timeline/history durations
reflect the real finish time rather than Date.now() inside the delayed callback.
In `@tsconfig.json`:
- Around line 5-8: The root tsconfig.json currently includes "DOM" in the "lib"
array which injects browser globals project-wide and causes DOM vs Node type
conflicts; remove "DOM" from the root "lib" array in tsconfig.json and ensure
any webview-specific tsconfig (e.g., webview-ui/tsconfig.*.json) retains "DOM"
so only webview compilation gets browser types; after change, verify separation
by printing the root tsconfig.json and the tsconfig files under webview-ui as
suggested.
In `@webview-ui/src/components/AgentLabels.tsx`:
- Around line 102-107: The confidenceHint calculation in AgentLabels.tsx can
mismatch the displayed summary because summary for non-sub agents derives from
agentTools via getActiveToolSummary(agentTools[id]) while confidenceHint only
inspects agentStatuses (status); update the confidenceHint assignment so it
mirrors the same branching as summary: when isSub use
subagentStateForId?.inferred, otherwise derive inference/source from the active
tool data (from agentTools[id] / the same getActiveToolSummary or the active
tool object) and/or fallback to status (e.g., activeTool?.source === 'heuristic'
|| activeTool?.inferred || status?.source === 'heuristic' || status?.inferred);
change the confidenceHint expression accordingly to reference the same symbols
(isSub, subagentStateForId, agentTools, getActiveToolSummary, status) so the
visual “?” matches the actual tool-derived summary.
In `@webview-ui/src/components/SettingsModal.tsx`:
- Around line 159-217: The Add Asset Directory and per-directory remove buttons
in the SettingsModal component use hard-coded RGBA colors (e.g., the hovered
background and remove-button background/border/text) which breaks the theme
token system; update the inline style objects in the SettingsModal JSX (the
button with onClick that posts addExternalAssetDirectory and the mapped remove
button for each externalAssetDirectories item) to replace those RGBA literals
with the project CSS custom properties (UI_* / --pixel-*) defined in :root
(index.css) — use the appropriate background, border and color tokens (e.g.,
--pixel-bg-hover, --pixel-accent-error-bg, --pixel-border, --pixel-fg-muted) so
the styles follow the existing theme system and keep the hovered-state logic
(hovered / setHovered) intact.
In `@webview-ui/src/hooks/useExtensionMessages.ts`:
- Around line 563-595: The side effects for 'waiting' (os.showWaitingBubble and
playDoneSound) run even when the incoming agent status is identical; fix by
first reading the previous entry from agentStatusesRef.current[id], compare its
status/source/inferred/confidence to the incoming values, and if they are all
equal then skip running os.setAgentActive, os.showWaitingBubble and
playDoneSound (and avoid updating agentStatusesRef). Otherwise proceed to update
state/ref and call os.setAgentActive, and if the new status === 'waiting' call
os.showWaitingBubble and playDoneSound; use the existing setAgentStatuses,
agentStatusesRef, os.setAgentActive, os.showWaitingBubble and playDoneSound
identifiers to locate the code to change.
In `@webview-ui/src/office/components/ToolOverlay.tsx`:
- Around line 341-354: In ToolOverlay, the JSX block that renders the status
indicator has inconsistent indentation for the wrapper <div style={{ display:
'flex', alignItems: 'center', gap: 6 }}> and its children (<span> elements using
statusColor and currentStatusLabel); fix by reformatting so the <div> and its
inner <span> elements share consistent indentation level and alignment with
surrounding JSX in the ToolOverlay component to match the file's existing style
conventions.
---
Outside diff comments:
In `@src/agentManager.ts`:
- Around line 32-49: The launchNewTerminal function currently accepts
bypassPermissions but never forwards it to adapter.getTerminalCommand, so
enabling bypassPermissions has no effect; update launchNewTerminal to pass
bypassPermissions into adapter.getTerminalCommand(sessionId, bypassPermissions)
(or similar second arg) and update the AgentAdapter interface signature and all
adapter implementations to accept the new bypassPermissions parameter so the
command generation can honor bypass mode (ensure function names:
launchNewTerminal, adapter.getTerminalCommand, and the AgentAdapter interface
are updated consistently).
In `@src/assetLoader.ts`:
- Around line 136-154: The path validation can be bypassed via symlinks because
path.resolve is insufficient; change the check to use real filesystem paths:
call fs.realpathSync (or async realpath) on both itemDir (or resolvedDir) and
assetPath to get realDir and realAsset, verify realAsset startsWith realDir +
path.sep or equals realDir, and only then proceed; also use the realAsset path
for exists/read operations (fs.existsSync and fs.readFileSync) and pass the
resulting buffer to pngToSpriteData so all file access and boundary checks are
performed against canonicalized paths (referencing assetPath,
resolvedDir/realDir, resolvedAsset/realAsset, pngBuffer, and pngToSpriteData).
In `@webview-ui/src/office/components/OfficeCanvas.tsx`:
- Around line 670-734: The click handler handleClick clears
officeState.selectedAgentId in several branches (after sendToSeat, after
reassignSeat + postMessage, and the final deselect-for-empty-space) but doesn't
notify the parent via onClick, causing external state to go stale; update those
branches in handleClick to call onClick(null) immediately after you set
officeState.selectedAgentId = null (and cameraFollowId = null) so the parent
stays in sync (refer to officeState.sendToSeat, officeState.reassignSeat, the
postMessage block, and the final deselect branch).
---
Nitpick comments:
In `@package.json`:
- Around line 55-72: The package.json change adds a configuration block (see
"configuration" and "pixel-agents.agentType") but omits the inline esbuild
problem matcher required for VS Code to map "node esbuild.js" build errors into
the Problems panel; add a "contributes.problemMatchers" entry in package.json
that registers an esbuild problem matcher (matching esbuild/node output) so
running your esbuild-based tasks will surface diagnostics without a separate
extension—update package.json to include the problem matcher manifest alongside
the existing "configuration" section.
In `@src/PixelAgentsViewProvider.ts`:
- Around line 306-335: Add new IPC string constants for
'addExternalAssetDirectory', 'removeExternalAssetDirectory', and
'externalAssetDirectoriesUpdated' in src/constants.ts (exported) and replace the
string literals in PixelAgentsViewProvider.ts (the message.type checks and the
webview.postMessage calls) with those constants; import the constants into
PixelAgentsViewProvider and use them wherever those literals appear, and update
the corresponding webview/frontend code to use the same exported constants so
both extension and webview share the centralized names.
In `@src/transcriptParser.ts`:
- Around line 194-317: The code in processCodexTranscriptLine contains
hard-coded backend literal strings (e.g., 'response_item', 'function_call',
'function_call_output', 'event_msg', 'task_started', 'task_complete',
'agent_message') and the responseItemStatusMap mapping; move these into
src/constants.ts as named exports (e.g., RESPONSE_ITEM, FUNCTION_CALL,
FUNCTION_CALL_OUTPUT, EVENT_MSG, TASK_STARTED, TASK_COMPLETE, AGENT_MESSAGE,
RESPONSE_ITEM_STATUS_MAP) and import them into transcriptParser.ts; then replace
all string occurrences and the inline responseItemStatusMap in
processCodexTranscriptLine with the new constant names so the parser references
centralized constants rather than literal strings.
In `@webview-ui/src/components/DebugView.tsx`:
- Line 62: The TimelineRail component contains multiple hardcoded layout numbers
(e.g., height: 44, top: 2, fontSize: 10, minWidth: 6, height: 12, offsets 24/16,
min-width ratio 1.5)—extract these magic numbers into named constants in a
shared constants.ts (e.g., TIMELINE_RAIL_HEIGHT, TIMELINE_RAIL_PADDING_TOP,
TIMELINE_LABEL_FONT_SIZE, TIMELINE_TICK_MIN_WIDTH, TIMELINE_TICK_HEIGHT,
TIMELINE_OFFSET_LARGE, TIMELINE_OFFSET_SMALL, TIMELINE_MIN_WIDTH_RATIO) and
replace the literals in TimelineRail (and the related spots around lines
referenced like 91-92, 124-125, 135-138) to use those constants; update imports
in DebugView.tsx to import the new constants from constants.ts and ensure names
are clear and grouped for future layout changes.
- Line 45: DEBUG_Z is defined locally in DebugView.tsx but must be centralized;
add "export const DEBUG_Z = 40" to webview-ui/src/constants.ts, remove the local
"const DEBUG_Z = 40" from DebugView.tsx, and import { DEBUG_Z } from
'webview-ui/src/constants' at the top of DebugView.tsx so all references in the
DebugView component use the shared constant.
In `@webview-ui/src/office/components/ToolOverlay.tsx`:
- Around line 267-268: The code repeats the pattern
[...mergedTools].reverse().find(...) to get the last non-done tool; replace that
with Array.prototype.findLast (use mergedTools.findLast(tool => !tool.done)) to
avoid creating a reversed copy and improve clarity, updating the activeTool
assignment (and any identical occurrences) to use findLast while keeping the
history computation (mergedTools.slice(-5).reverse()) unchanged.
- Around line 314-328: The inline style object in the ToolOverlay component
contains hardcoded layout "magic numbers" (e.g., top: 12, right: 12, width:
'min(440px, calc(100% - 24px))', maxHeight: 'calc(100% - 24px)', padding: 18,
zIndex: 85); extract these values into centralized constants in constants.ts
(e.g., OVERLAY_TOP, OVERLAY_RIGHT, OVERLAY_MAX_WIDTH,
OVERLAY_MAX_HEIGHT_PADDING, OVERLAY_PADDING, OVERLAY_Z_INDEX) and import them
into ToolOverlay.tsx, then replace the literal values in the overlay style
object (the object passed to the main container in the ToolOverlay component)
with the new constants so layout values are centralized and maintainable.
In `@webview-ui/test/tool-utils.test.ts`:
- Around line 1-43: Add a regression test for defaultZoom to ensure it falls
back correctly when devicePixelRatio is missing or invalid: write node:test
cases that import defaultZoom and temporarily set/clear
globalThis.devicePixelRatio (undefined, 0, NaN, Infinity, negative) and assert
defaultZoom() returns the fallback (e.g., 1) for those values; make sure to
restore/delete the globalThis.devicePixelRatio after each case to avoid
cross-test pollution and use assert.equal for comparisons so the Node-based test
path is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4fab951a-bc76-40ef-a475-d03bc4966539
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonwebview-ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (56)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/config.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/workflows/pr-title.yml.github/workflows/update-badges.yml.gitignore.nvmrc.vscode/launch.jsonCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.ko.mdREADME.mdcomments.jsondocs/agent-visibility-followup-plan.mddocs/agent-visibility-plan.mddocs/external-assets.mdpackage.jsonpr_comments.jsonpr_reviews.jsonreviews.jsonsrc/PixelAgentsViewProvider.tssrc/adapters/claudeAdapter.tssrc/adapters/codexAdapter.tssrc/adapters/shared.tssrc/agentAdapter.tssrc/agentManager.tssrc/assetLoader.tssrc/configPersistence.tssrc/constants.tssrc/fileWatcher.tssrc/timerManager.tssrc/transcriptParser.tssrc/types.tstsconfig.jsonwebview-ui/README.mdwebview-ui/package.jsonwebview-ui/src/App.tsxwebview-ui/src/agentVisibilityUtils.tswebview-ui/src/components/AgentLabels.tsxwebview-ui/src/components/BottomToolbar.tsxwebview-ui/src/components/DebugView.tsxwebview-ui/src/components/SettingsModal.tsxwebview-ui/src/constants.tswebview-ui/src/hooks/useEditorActions.tswebview-ui/src/hooks/useExtensionMessages.tswebview-ui/src/index.csswebview-ui/src/office/components/OfficeCanvas.tsxwebview-ui/src/office/components/ToolOverlay.tsxwebview-ui/src/office/editor/EditorToolbar.tsxwebview-ui/src/office/engine/matrixEffect.tswebview-ui/src/office/engine/renderer.tswebview-ui/src/office/floorTiles.tswebview-ui/src/office/toolUtils.tswebview-ui/src/office/types.tswebview-ui/test/tool-utils.test.ts
- Fix toClaudeProjectHash regex to match original behavior (replace all non-alphanumeric) - Replace hardcoded rgba colors with UI constants in SettingsModal - Don't permanently mark unadopted JSONL files as known in scanner - Infer adapter from projectDir path for legacy persisted agents - Return success/failure from writeConfig and guard callers - Remove unused _sessionId parameter from buildCodexSessionCommand - Capture endTime before setTimeout delay in finalizeToolActivity - Remove DOM lib from root tsconfig (extension host is Node-only) - Prevent duplicate waiting sound on repeated agentStatus messages - Fix indentation inconsistency in ToolOverlay status section - Update community/issue links from upstream to fork repository - Fallback to bundled assets in reloadAndSendFurniture when assetsRoot unset Amp-Thread-ID: https://ampcode.com/threads/T-019d2049-4461-761e-a6ba-6797cbab4eb8 Co-authored-by: Amp <amp@ampcode.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…odex Resolve 12 merge conflicts, keeping PR #16 review fixes: - toClaudeProjectHash regex, buildCodexSessionCommand param removal - finalizeToolActivity endTime capture, adapter inference for legacy agents - fileWatcher unadopted JSONL retry, duplicate waiting sound prevention - SettingsModal UI constants, ToolOverlay indentation, community links - CLAUDE.md external asset directory messages, .gitignore /notes/ - webview-ui/package-lock.json: use HEAD version Amp-Thread-ID: https://ampcode.com/threads/T-019d2049-4461-761e-a6ba-6797cbab4eb8 Co-authored-by: Amp <amp@ampcode.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add aria-label and title to asset directory remove button (a11y) - Fix trailing slash producing empty display name with filter(Boolean) - Centralize Claude project hash regex to constants.ts - Sync React selection state on empty-space click in OfficeCanvas Amp-Thread-ID: https://ampcode.com/threads/T-019d2049-4461-761e-a6ba-6797cbab4eb8 Co-authored-by: Amp <amp@ampcode.com>
Rename agentToolsClear → agentToolClear to match the actual IPC message name (agentToolStart/Done/Clear) so grep and handler lookups stay consistent. Addresses: #16 (comment) Amp-Thread-ID: https://ampcode.com/threads/T-019d2203-8131-744f-a339-2a4e74c156f6 Co-authored-by: Amp <amp@ampcode.com>
Delete comments.json, pr_comments.json, pr_reviews.json, reviews.json from tracking and add .gitignore entries to prevent future commits of these artifacts. Addresses: #16 (comment) Amp-Thread-ID: https://ampcode.com/threads/T-019d2203-8131-744f-a339-2a4e74c156f6 Co-authored-by: Amp <amp@ampcode.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This wraps up the remaining CodeRabbit follow-up by moving the SettingsModal danger hover color onto a shared UI constant, tightening heuristic confidence hints in AgentLabels, and correcting docs that drifted from the fork and current timer behavior. While verifying the branch, I also removed an accidentally duplicated helper block in useExtensionMessages so the webview typecheck/build path works again.\n\nConstraint: Needed to preserve the existing review-response branch and avoid broad refactors while finishing the pending PR work\nRejected: Commit only the color-token tweak | left the branch in a broken verification state because the duplicated hook helpers still failed build\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep status-confidence and theme-token logic centralized so future PR review fixes do not reintroduce inline literals or duplicate helper blocks\nTested: npm run build; npm run lint:webview; cd webview-ui && npm test\nNot-tested: Manual VS Code extension interaction for SettingsModal hover and AgentLabels confidence rendering
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
CodeRabbit flagged that replaying the same agentStatus payload could still retrigger waiting side effects after state dedupe. This change centralizes status equality checks, exits early when the incoming status metadata matches the ref-backed value, and only runs active/waiting side effects when the payload actually changes.\n\nConstraint: Needed to preserve the existing hook flow and avoid broader message-handler refactors while fixing the repeated alert behavior\nRejected: Guard only on previousStatus !== 'waiting' | still reruns other side effects and misses duplicate source/inferred/confidence payloads\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep agentStatus dedupe logic shared between state updates and side effects so replayed events do not drift from UI behavior\nTested: npm run build; npm run lint:webview; cd webview-ui && npm test\nNot-tested: Manual webviewReady replay in VS Code with live sound output
Job 68449150096 failed only because the merge commit tripped the blocking format check. This commit applies the repository's existing Prettier style to the three files called out in the CI logs so the final gate sees FORMAT_CHECK=success while leaving the advisory audit output unchanged.\n\nConstraint: Needed to match the existing workflow contract instead of changing CI semantics or dependency policy during a PR fix\nRejected: Change the workflow to ignore format drift | would hide a real blocking check instead of fixing the code checked by CI\nRejected: Run npm audit fix in this PR | advisory audit failures are non-blocking in ci.yml and unrelated to the failing gate\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: When CI names specific files in Format Check, prefer a pure Prettier commit over workflow changes unless the style rule itself is wrong\nTested: npm run format:check; npm run lint && npm run lint:webview; npm run build; cd webview-ui && npm test\nNot-tested: Fresh GitHub Actions rerun after push
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The centralized Claude hash sanitizer widened the replacement set enough to change existing ~/.claude/projects directory names. This restores the historical replacement behavior so previously-created Claude session folders remain discoverable instead of silently moving to a new hash scheme.\n\nConstraint: Existing Claude session directories already rely on the legacy path hash and must remain readable across upgrades\nRejected: Keep the broader sanitizer and add a second lookup path later | leaves the current PR with a known regression in session discovery\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Treat Claude project hash generation as a compatibility boundary — do not widen the character replacement set without a tested migration or fallback lookup strategy\nTested: npm run check-types && npm run lint && npm run lint:webview && npm run format:check; npm run build && cd webview-ui && npm test\nNot-tested: Manual discovery of pre-existing ~/.claude/projects directories in a live VS Code session
The settings modal previously rendered every external asset directory row inline, which could push the toggle controls off-screen once the list grew. This wraps the directory rows in a capped scroll container so removal controls remain usable while the rest of the modal stays visible.\n\nConstraint: Needed to preserve the existing modal layout and hover/remove interactions without redesigning the settings UI\nRejected: Add page-level scrolling to the whole modal only | would still let the directory list compete with the settings controls for visible space\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep variable-length settings collections in bounded containers so core modal actions remain reachable on smaller viewports\nTested: npm run lint:webview && npm run format:check; npm run build && cd webview-ui && npm test\nNot-tested: Manual interaction with a very long external asset directory list in VS Code
| webview: vscode.Webview | undefined, | ||
| persistAgents: () => void, | ||
| folderPath?: string, | ||
| bypassPermissions?: boolean, |
There was a problem hiding this comment.
🟡 Unused bypassPermissions parameter added to launchNewTerminal is dead code
The bypassPermissions parameter is added to launchNewTerminal at src/agentManager.ts:48 and is passed from src/PixelAgentsViewProvider.ts:105, but the parameter is never referenced anywhere in the function body. Additionally, the webview never sends a bypassPermissions field in any openAgent message (webview-ui/src/hooks/useEditorActions.ts:107 only sends { type: 'openAgent', folderPath }), so the value is always undefined. This violates the CONTRIBUTING.md rule: "No unused locals or parameters (noUnusedLocals and noUnusedParameters are enabled)." The extension backend tsconfig.json does not actually enforce noUnusedParameters, so this won't be caught by CI.
Prompt for agents
Remove the unused `bypassPermissions` parameter from `launchNewTerminal` in src/agentManager.ts:48, and remove the corresponding argument passed from src/PixelAgentsViewProvider.ts:105 (message.bypassPermissions as boolean | undefined). Also consider adding `noUnusedParameters: true` to the root tsconfig.json compilerOptions so the TypeScript compiler can catch such issues in the extension backend.
Was this helpful? React with 👍 or 👎 to provide feedback.
Relevant Codex JSONL files that could not yet be adopted were being re-read on every scan cycle because their workspace relevance had to be recomputed from disk each time. This introduces a dedicated relevant-but-unadopted cache so confirmed files skip repeated relevance I/O while still being retried for adoption until a suitable terminal becomes available.\n\nConstraint: Needed to preserve deferred adoption behavior for external sessions while avoiding extra 1-second file reads from Codex transcript headers\nRejected: Put unadopted files back into knownJsonlFiles | avoids I/O but reintroduces the session-missed regression that the previous review fixed\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep relevance caching separate from adoption state so transcript discovery can be retried without redoing expensive header reads\nTested: npm run check-types && npm run lint && npm run lint:webview && npm run format:check; npm run build && cd webview-ui && npm test\nNot-tested: Manual VS Code scenario with multiple external Codex sessions and delayed terminal focus
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
4 participants
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
pablodelucca/pixel-agentsonto the currentcodexline by rebasingcodexontoupstream/mainUpstream commits included
1eb308cchore: add feature request template, update community docs (chore: add feature request template, update community docs pablodelucca/pixel-agents#164)04edcbefeat: add bypass permissions mode for agents (feat: add bypass permissions mode for agents pablodelucca/pixel-agents#170)d7f2b7ffeat: add support for external asset packs 🎨 (feat: add support for external asset packs 🎨 pablodelucca/pixel-agents#169)Rebase result
codexcan be rebased onto the latest upstreammain, but it is not conflict-freesrc/agentManager.ts,CLAUDE.md,README.md,webview-ui/src/components/BottomToolbar.tsx,.gitignore, andwebview-ui/package-lock.json.omx/files were temporarily moved out of the working tree during the rebase and restored afterwardValidation
npm run build❌tsc --noEmitwith environment/type-definition errors (for examplelib.dom.d.tsvs Node typings, missingesnext.disposable/Disposable, and related ambient typing errors)Notes
codex, notmain, so the rebased codex history can be reviewed directlySummary by CodeRabbit
릴리즈 노트
새로운 기능
문서
버그 수정
기타