Skip to content

fix(config): anchor agent/command name matching at the relative path - #1136

Merged
Astro-Han merged 1 commit into
devfrom
claude/upstream-entry-name-anchor
Jun 3, 2026
Merged

fix(config): anchor agent/command name matching at the relative path#1136
Astro-Han merged 1 commit into
devfrom
claude/upstream-entry-name-anchor

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

Summary

configEntryNameFromPath matched a known prefix (agent/, agents/, command/, commands/, mode/, modes/) anywhere in the absolute file path. A user or parent directory whose name coincidentally contained one of those segments won the substring match before the real entry directory, leaking the intervening path into the key — an agent at /Users/agent/proj/agent/build.md keyed as proj/agent/build instead of build.

This anchors the match at the start of the path and has the three call sites pass the path relative to the directory they scanned (path.relative(dir, item)), so the relative path is always rooted at the prefix.

Why

Glob.scan already scans {agent,agents}/**, {command,commands}/**, and {mode,modes}/* with cwd=dir, so path.relative(dir, item) is always agent/…, command/…, etc. With the relative path, anchoring at the start fixes the mis-keying and the .opencode/ prefix variants become unnecessary (.opencode lives in dir, not in the relative path), so the prefix list simplifies to the bare directory names. PawWork's case-insensitive matching and nested-subdirectory keys (agents/team/build.mdteam/build) are preserved.

Related Issue

No PawWork issue. Reimplemented for PawWork from upstream anomalyco/opencode e94d46af86 (PR #28359, original regression #25713, thanks Kit Langton). The fork has no common ancestor with upstream, so this is an adapted port that keeps PawWork's case-insensitive normalization (upstream's variant is case-sensitive), not a cherry-pick.

Human Review Status

Pending

Review Focus

Medium risk: configEntryNameFromPath feeds agent/command/mode keys across three call sites. Confirm (1) the contract change from absolute path → path.relative(dir, item) is correct for all three (the glob guarantees items are under dir), (2) case-insensitive matching is preserved, and (3) nested-subdirectory keys are preserved. The existing config.test.ts "tolerate unnormalized roots" assertion still passes unchanged.

Risk Notes

Medium. The helper's contract changed (now expects a relative path); all three in-repo call sites were updated in the same change and there are no other callers. Behavior for the normal case is unchanged; only the mis-keyed parent-segment case changes (now correct). No platform/packaging/UI surface touched.

How To Verify

New unit test: bun test test/config/entry-name.test.ts -> 9 pass
Red proof (startsWith reverted to indexOf): the misleading-parent-segment case fails with Received "proj/agent/build" (1 fail, 8 pass)
Existing assertion: bun test test/config/config.test.ts -t "tolerate unnormalized roots" -> 1 pass
Full config suite regression: bun test test/config/ -> 222 pass, 0 fail
Typecheck: packages/opencode bun run typecheck -> clean

Screenshots or Recordings

N/A — no visible UI change.

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

configEntryNameFromPath matched a prefix (agent/, agents/, command/,
commands/, mode/, modes/) anywhere in the absolute file path. A user or
parent directory whose name coincidentally contained one of those segments
(e.g. a home dir /Users/agent/) won the substring match before the real
entry directory, leaking the intervening path into the key — an agent at
/Users/agent/proj/agent/build.md keyed as "proj/agent/build" instead of
"build".

Anchor the match at the start of the path and have the call sites pass the
path relative to the directory they scanned (path.relative(dir, item)), so
the relative path is always rooted at the prefix. Glob already scans
{agent,agents}/**, {command,commands}/**, and {mode,modes}/* with cwd=dir,
so the .opencode/ prefix variants are subsumed and the prefix list
simplifies to the bare directory names. Case-insensitive matching and
nested-subdirectory keys are preserved.

Add test/config/entry-name.test.ts covering prefix stripping, nested keys,
backslash normalization, case-insensitive matching, basename fallback, and
the #28359 regression (a misleading parent /agent/ segment is no longer
stripped). The existing "tolerate unnormalized roots" assertion still holds.

Reimplemented for PawWork from upstream anomalyco/opencode e94d46af86
(PR #28359, regression #25713, thanks Kit Langton); the fork has no common
ancestor so this is an adapted port that keeps PawWork's case-insensitive
normalization, not a cherry-pick.
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 52 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d7da774-c05a-4854-8d4c-af557c3edc61

📥 Commits

Reviewing files that changed from the base of the PR and between 2081b37 and eadc3e2.

📒 Files selected for processing (4)
  • packages/opencode/src/config/agent.ts
  • packages/opencode/src/config/command.ts
  • packages/opencode/src/config/entry-name.ts
  • packages/opencode/test/config/entry-name.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/upstream-entry-name-anchor

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Astro-Han Astro-Han added the bug Something isn't working label Jun 3, 2026
@github-actions github-actions Bot added the harness Model harness, prompts, tool descriptions, and session mechanics label Jun 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the config entry name resolution to prevent incorrect key mapping when parent directory segments coincidentally match config prefixes (such as '/agent/'). It introduces path.relative at the call sites in agent.ts and command.ts to anchor prefix matching at the start of the relative path, and updates configEntryNameFromPath to use startsWith instead of indexOf. Comprehensive unit tests have also been added to verify this behavior and prevent regressions. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@github-actions github-actions Bot added the P2 Medium priority label Jun 3, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han
Astro-Han merged commit 5c99f9f into dev Jun 3, 2026
35 of 36 checks passed
@Astro-Han
Astro-Han deleted the claude/upstream-entry-name-anchor branch June 3, 2026 11:40
Astro-Han added a commit that referenced this pull request Jun 3, 2026
Restructure the right-side Status panel from 2 sections (Progress + Sources) into 4 (Progress -> Workspace/Git -> Changed files/Artifact -> Sources), fold the standalone Files tab into the panel as a compact changed-files list, and move the titlebar worktree badge into the Git section.

Change boundary (packages/app, packages/ui):
- Status panel: new Git section (diff stats -> Review, branch row, worktree indicator with tooltip + open-directory) and Artifact section (changed files with hover/focus-visible open + reveal); Git section hidden outside a git repo. Data from sync.data.vcs (branch), sessionInfo().executionContext.activeWorktree (worktree), aggregateFiles(turn_change_aggregate) (diff stats).
- Files tab removed from RightPanelStaticTab / RIGHT_PANEL_TAB_META / command palette / keybind; legacy persisted "files" coerced to "status" via coerceLegacySidePanelTab + migrateLegacyRightPanelTab.
- Titlebar PawworkWorktreeBadge removed (worktree info now lives in the panel Git section).
- New `changes` icon registered (packages/ui).

Verification:
- typecheck clean; full app unit suite 1753 pass / 0 fail.
- Visual: status-summary-panel snap (4 sections + rest->hover open/reveal) plus a dev:desktop walk in the real Electron host (dark theme, zh locale, live sidecar) - Git section populated from real VCS (+N/-N, branch main), Artifact section listing a real session file (sample-test.md, +76).
- Codex adversarial review P1 findings fixed; 5 review threads resolved.

Brought current with dev: one conflict in context/layout.tsx - dev had extracted the inline layout helpers into context/layout-state.ts / layout-projects.ts (#1056 slice work). Took dev's refactored layout.tsx and ported this branch's defaultSidePanelTab `| "files"` widening into layout-state.ts. Final state 0 behind dev; also merged #1135/#1136/#1137/#1139 cleanly (no file overlap).

Deferred: dev:desktop worktree-indicator tooltip + non-git-hide are state-conditional and were not exercised live (hover open/reveal covered by the snap). session-side-panel.test.tsx isolation failure is the pre-existing mock.module warmup flake (its @/context/command mock omits matchKeybind/parseKeybind that terminal.tsx imports transitively); green in the full suite, tracked under the #1084 mock.module cleanup.

Relates #1056.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant