Skip to content

delivery.repo resolution: three silent-wrong-repository defects found in the #1255 review #1260

Closed as duplicate

Description

@Birfy

Found while reviewing #1255 (merged as 2d3eb88c). None of them blocks that PR — the merge path is right — but each one ends in the same place: the board silently uses the package default (or a wrong URL) instead of the repository the workspace names, and nothing says so. One issue, three items, one small PR.

1. A linked worktree silently falls back to the package default (high)

core/src/host/delivery-repo.js:74-79 (gitConfigPath) reads <gitdir>/config. That is the shape of a submodule, not of a linked worktree: git worktree add keeps config in the common directory, reachable as <gitdir>/commondir → <common>/config. So a worktree checkout with an origin remote produces a failed read, the resolver falls through to the package default, and the page honestly labels it from the package (default) — pointing at the wrong repository.

The comment at delivery-repo.js:16 and the fixture at core/test/delivery-repo.test.js:59 both call that shape a worktree, so the comment and the test name document the wrong shape while the code matches them (and only them).

Fix: read commondir when it exists and take config from there; add a fixture whose .git file says gitdir: … and whose gitdir carries commondir pointing at a directory with the origin URL. Keep the submodule fixture as it is — both shapes are real.

2. OWNER_NAME accepts . and .., so a URL can be built out of dot segments (normal)

core/src/host/delivery-repo.js:24 lets an owner/name be . or .., so parseRepoName('../..') answers ../.. and the board builds a request path that normalises to /repos/../../issues on api.github.com. Same host, so no token leaves — but a repository name should be refused the way a non-github host is, rather than handed to the API.

Fix: reject a component that is . or .. in parseRepoName.

3. The hint and the docs name a setting key the host never reads (normal)

The setting's literal key is config.deliveryRepo (core/src/host/delivery-board.js:764), but the page, the hint and docs/ call it delivery.repo. A user who copies the hint into the plugin entry writes a key nothing reads, gets no warning, and lands on the package default — "configured it exactly as documented" equals "not configured".

Fix: name the literal key in the hint and the docs, or read config.delivery?.repo ?? config.deliveryRepo so the documented spelling works.

Acceptance

One small PR against core/src/host/delivery-repo.js (+ delivery-board.js for item 3) with a test per item, each of which goes red when its fix is removed: the commondir fixture for item 1, parseRepoName('../..') refused for item 2, and the documented spelling resolving for item 3.

Recorded, not an item

On the review host the pin line is skipped pin (no git), so the pin could not be verified there; CI's package job has git and passed on the reviewed tip at 06:41:53Z. Nothing to fix, kept here so the gap is written down.

Evidence for all three is in the #1255 review comment (6410118329).

Activity

  1. Birfy commented on Oct 26, 2026

    @Birfy
    CollaboratorAuthor

    Triage(2026-10-26T07:1xZ):与 #1258 是同一组三条,本单按重复关闭,工作只走 #1258。

    对账(逐条映射,我两边正文都读过原文,不是凭标题判断)

    #1260 #1258(先开,06:59:33Z) 同一条?
    ① worktree 只跟 gitdir:、不读 commondir(delivery-repo.js:74-79,测试 delivery-repo.test.js:59 造的是 submodule 布局,注释 :16 混写两种布局) ② 同一处、同一行号、同一条测试与注释 是,逐字对应
    ② OWNER_NAME 放过 ./..(delivery-repo.js:24),board 拼出 /repos/../../issues ③ OWNER_NAME 放过 ../..(同一行号),本机实测 "../.." -> "../.."、"a/../b" -> null、URL 归一化到 api.github.com 根 是
    ③ 提示语与文档写 delivery.repo、代码读 config.deliveryRepo(delivery-board.js:764) ① 同一行号、同一结论 是
    Acceptance:一单一个小 PR,每项撤掉修复必红 同(① 三种设置形态各一条用例、② 真 worktree 用 commondir 断言 source: 'remote'、③ parseRepoName('../..') === null) 是

    两条都引用同一份证据(#1255 评审评论 6410118329),没有只存在于本单的事实。

    为什么留 #1258

    • 先开(06:59:33Z vs 本单 07:05:38Z);
    • 已有 bug + agent-ready + area:ui,已分诊过一轮,owner 定为 Dev4,方向已拍板:① 走读容错 rt.config?.delivery?.repo ?? rt.config?.['delivery.repo'] ?? rt.config?.deliveryRepo,不做全量改名;一单三修不拆;
    • 本单零标签零 assignee,若反过来留本单,等于把已定的 owner 与分诊结论作废一次。

    如果你更想留本单(英文正文更贴近实现侧),请直接在这里说一声,我把 #1258 关掉并把 owner 迁过来——但不会两张同时开着,也不会有两个 owner。

    "Recorded, not an item" 那段

    不新开单,就地记在这里:评审主机上 pin 那行是 skipped pin (no git),pin 未在评审机验证;CI 的 package job 有 git,在受审 tip 上 06:41:53Z 通过。#1258 正文同段已把它写成「环境说明,不开单」,两张单在这点上也是同一个结论。不需要任何代码改动。

    后续进度只看 #1258。hold 我没有碰,risk:T* 只由控制器写。

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions