Conversation
Greptile SummaryThis PR redesigns the agent Tool Access panel, replacing flat tool rows with collapsible The implementation is clean overall, with good responsive breakpoints, reduced-motion support, and thorough test coverage. One noteworthy behaviour: the enable/disable toggle for each tool is placed after Confidence Score: 4/5Safe to merge — no functional bugs or security issues found; all changes are UI/style with good test coverage. The PR is a substantial but well-executed UI redesign. No P0 or P1 issues were found. The only noteworthy behaviour (toggle only accessible when card is expanded) is an intentional design decision. Tests cover the new navigation and collapse cascading logic correctly. No files require special attention. Reviews (1): Last reviewed commit: "fix(ui): tighten tool access scanning" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d59f17757
ℹ️ 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".
| <a | ||
| class="agent-tools-runtime-chip" | ||
| href="#${anchorId}" | ||
| @click=${(event: Event) => handleRuntimeToolJump(event, anchorId)} |
There was a problem hiding this comment.
Render non-catalog live tools without jump links
tools.catalog groups only include core|plugin tools, while tools.effective can include channel tools, so some live entries can never have a matching row in the section list. This block renders every live tool as a clickable anchor, but handleRuntimeToolJump immediately returns when no matching id exists, leaving channel chips as dead links that only mutate the URL hash instead of opening anything. Consider rendering a non-link chip (or a disabled link) when there is no matching catalog row.
Useful? React with 👍 / 👎.
| <label | ||
| class="cfg-toggle agent-tool-toggle" | ||
| @click=${(event: Event) => event.stopPropagation()} | ||
| > |
There was a problem hiding this comment.
Keep toggle control inside the details summary
This toggle is rendered as a sibling of <summary> inside <details>, and closed <details> only expose the summary while other children are hidden by default browser behavior. Because each tool row is collapsed by default, the checkbox is not visible/clickable until the row is expanded, which regresses quick enable/disable control on the main list view. Move the toggle into the summary (with propagation guarded) or explicitly account for closed-details rendering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Codex review/fixup: looks good after the follow-up patch.
I fixed two small regressions before landing:
- kept each per-tool access toggle inside the collapsed tool summary so the primary control stays reachable without expanding the card
- preserved section-level plugin provenance in the expanded Source detail when a catalog tool inherits source metadata from its group
Local proof:
pnpm test ui/src/ui/views/agents-panels-tools-skills.browser.test.ts ui/src/ui/app-render.helpers.browser.test.tspnpm format:check CHANGELOG.md ui/src/ui/views/agents-panels-tools-skills.ts ui/src/ui/views/agents-panels-tools-skills.browser.test.tspnpm check:changedpassed through typecheck/lint/guards; the broadened test batch was SIGTERM'd locally without an assertion failure after the branch rebased in unrelated main changes, so I split and reran the affected test targets successfully.
|
Landed via squash: 982230f Thanks @BunsDev. I kept the Tool Access panel polish, added the visible-toggle/provenance fixups, synced with current main, and verified the final head. Proof:
I also searched open issues/PRs for related |
- Remove duplicate #66884 alexlomt entry from top Unreleased > Fixes; the canonical entry already lives under 2026.4.24 (Unreleased) per Mason Huang's earlier 'move #66884 entry to 2026.4.24' commit. - Reflow the wrapped 3-line Tool Access bullet (#71405) onto a single line so it matches every other bullet in the section.
- Remove duplicate openclaw#66884 alexlomt entry from top Unreleased > Fixes; the canonical entry already lives under 2026.4.24 (Unreleased) per Mason Huang's earlier 'move openclaw#66884 entry to 2026.4.24' commit. - Reflow the wrapped 3-line Tool Access bullet (openclaw#71405) onto a single line so it matches every other bullet in the section.
* feat(ui): refine tool access controls * fix(ui): tighten tool access scanning * fix(ui): keep tool access toggles visible (openclaw#71405) * test(daemon): cover launchd restart fallback plist reads (openclaw#71405) * test(daemon): drop duplicate launchd read mock (openclaw#71405) --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
- Remove duplicate openclaw#66884 alexlomt entry from top Unreleased > Fixes; the canonical entry already lives under 2026.4.24 (Unreleased) per Mason Huang's earlier 'move openclaw#66884 entry to 2026.4.24' commit. - Reflow the wrapped 3-line Tool Access bullet (openclaw#71405) onto a single line so it matches every other bullet in the section.
- Remove duplicate openclaw#66884 alexlomt entry from top Unreleased > Fixes; the canonical entry already lives under 2026.4.24 (Unreleased) per Mason Huang's earlier 'move openclaw#66884 entry to 2026.4.24' commit. - Reflow the wrapped 3-line Tool Access bullet (openclaw#71405) onto a single line so it matches every other bullet in the section.
Summary
Validation