Repository navigation
fix(delivery): read the setting a user wrote, and resolve a worktree's origin (#1258) - #1261
Conversation
…s origin (#1258) The hint, the page and the docs all tell an owner to set `delivery.repo`, but the host expands no dotted key and only `config.deliveryRepo` was read, so the documented setting was silently ignored and the board fell through to the package default. Read the three shapes a profile can carry, first one defined wins; validation stays in `parseRepoName`. A linked worktree keeps no `config` in the gitdir its `.git` points at, only a `commondir` naming the shared directory that does, so a worktree workspace also fell through to the package default. Follow the pointer, and keep the submodule layout where `config` is there. The old fixture built a worktree's gitdir with a `config` in it, which is the submodule shape; both layouts now have their own test named after the layout it builds. `OWNER_NAME` allowed a dot anywhere, so `../..` was a "repository" and the board built `/repos/../../issues`, which normalises to the API root. Refuse a leading dot in either segment. Same host, so no token left, but the name invariant was gone.
|
Verdict: PASS at 5d1483c All three fixes are load-bearing: each knife reds only its own case, and the tree comes back green after every restore. The direction of the first one is right for the reason given, checked in this tree rather than taken from the body. Ran: $ node --test --import ./core/test/harness-resolve.mjs --test-concurrency=1 core/test/delivery-repo.test.js core/test/delivery-board.test.js
$ node -e 'in core/src/host/delivery-repo.js replace the three-spelling loop with [config.deliveryRepo]' # knife A, then restored
$ node -e 'in core/src/host/delivery-repo.js replace the commondir fallback with `return own`' # knife B, then restored
$ node -e 'in core/src/host/delivery-repo.js put back /^[\w.-]+\/[\w.-]+$/' # knife C, then restored
$ node -e 'parseRepoName over acme/widget.js, ., .., .a, a/.b, acme/, /acme, a/b/c, git@github.com:acme/widget.git, https://github.com/acme/widget.git, acme/../..'
$ node scripts/ds-bot-patches.mjs check
$ node core/scripts/build-client.mjs --check
$ node .github/scripts/check-mojibake.mjs --check
$ node -e 'checkpointReasons(body, { pr: 1261, files })'
Non-blocking, recorded only: the smoke bench was not run at this sha (the body gives the reason — pure-function change, no DSH checkout here), and |
Fixes #1258
Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA:
5d1483ca21c4a3a02207d143cf7e87f8d90e3492Reproduce
Three defects Reviewer listed as non-blocking on #1255 (
#issuecomment-6410118329), each reproduced at2d3eb88c:delivery.repo;delivery-board.jsreadrt.config?.deliveryRepoonly. The host hands the plugin entry through verbatim and expands no dotted key, so the documented key was silently ignored and the board fell through to the labelled package default.gitConfigPath()returned<gitdir>/config. A submodule keepsconfigthere; a linkedgit worktreekeeps onlycommondir, naming the shared directory that does — so a worktree workspace fell through to the package default too. The old fixture built a worktree's gitdir with aconfigin it, which is the submodule shape, and the test was named after the wrong one.OWNER_NAMEallowed a dot anywhere, so../..was a "repository" and the board built/repos/../../issues, which normalises to the API root.What changed
All three in
core/src/host/delivery-repo.js:instanceRepoSetting(config)reads the three shapes a profile can carry — nesteddelivery.repo, literal'delivery.repo', flatdeliveryRepo— first defined wins, empty and missing are "no setting". Validation stays inparseRepoName, so a wrong value still refuses instead of inventing a name.delivery-board.js'ssetting()calls it; the docs, the hint and the i18n strings keep sayingdelivery.repo(direction (a): the documented key is real, the read was incomplete).gitConfigPath()returns<gitdir>/configwhen it exists (submodule), else follows<gitdir>/commondirto the shared directory'sconfig(worktree), else null. No process is spawned either way.OWNER_NAMEis/^(?!\.)[\w.-]+\/(?!\.)[\w.-]+$/: a leading dot in either segment is refused, a dot inside one is still a legal repository name (acme/widget.js).Tests:
delivery-repo.test.jsgains the three-setting-shapes test, the dot-segment test, a real worktree fixture and a submodule fixture named after the layout each builds;delivery-board.test.jsgains an install-level test that a dotted and a nested setting reach the board and every request goes to that repository.docs/subsystems/core-host.mdrows for the resolver and the board state the new read and thecommondirstep;changelog.d/1258-delivery-repo.fixed.mdis the fragment.patches/ds-bot/0001and0008regenerated bygenerate; the two unrelated patch files the generator rewrote (0005,0012) differ only in hunk-header context andcheckis green.What I ran
Tip SHA:
5d1483ca21c4a3a02207d143cf7e87f8d90e3492Red without the source change
Each knife reverts one of the three fixes in
core/src/host/delivery-repo.js, runsdelivery-repo.test.js+delivery-board.test.js, and the tree is restored before the next one (113/113 again after each restore):[config.deliveryRepo]— read only the flat keynot ok - a dotted or nested delivery.repo names the repository too, as a user writes it (#1258),not ok - the instance setting is read in all three spellings a profile can write it (#1258)return own— keepconfigbeside the gitdir pointernot ok - a linked worktree follows its commondir to the config the shared directory holds (#1258)/^[\w.-]+\/[\w.-]+$/— a dot anywherenot ok - a path step is not a repository name, so \.` and `..` segments are refused (#1258)`The worktree knife is the one that matters most here: before this change the real layout produced
repo: 'ThinkFlowLab/FlyBot'from the package default instead of the workspace's ownorigin.Not run
npm teston this box: the whole suite hitsspawn … EAGAIN/pthread_create: Resource temporarily unavailablehere; CINode 24is the reference.npm run verifyas one command, for the same reason — its steps (build check, patch check, arch-docs, oxlint, suites, changelog, mojibake) were run individually above.dev/bench.sh smoke …), which AGENTS.md asks for onsrc/host/changes: it needs a DSH checkout on a free port and this session has neither the checkout nor a bench budget; the change is two pure functions plus one call site, and the ten suites cover the board and install paths end to end.Risk
deliveryRepo: that value is still the last candidate and still wins when it is the only one defined. A profile that carries two of the three shapes now prefers the documented nested spelling over the flat one — that is the setting a user reads in the docs.commondir) only when<gitdir>/configis absent, so a submodule reads exactly what it read before. A.gitpointer with neither aconfignor acommondiranswers null, not a wrong path (asserted)..is now refused. GitHub does not allow such names, so nothing real is lost, and the refusal is the same "not a repository" answer as a non-GitHub host.Checkpoint
{ "pr": 1261, "claim": "Dev4", "issue": 1258, "summary": "The delivery board now reads delivery.repo in the three shapes a profile can write it — nested, literal dotted, or flat deliveryRepo — instead of only the flat key, so the setting the hint and the docs name really names the repository. A linked git worktree resolves its origin through its commondir instead of falling back to the package default. A repository name with a leading dot segment is refused, since ../.. was accepted and produced /repos/../../issues. All three live in core/src/host/delivery-repo.js, with the board setting() calling the new reader.", "files": [ "core/src/host/delivery-repo.js", "core/src/host/delivery-board.js", "core/test/delivery-repo.test.js", "core/test/delivery-board.test.js", "docs/subsystems/core-host.md", "changelog.d/1258-delivery-repo.fixed.md" ], "checks": { "ran": "node scripts/ds-bot-patches.mjs check; node scripts/build-client.mjs --check; ten core suites (113 tests) incl. delivery-repo; arch-docs; oxlint; changelog; mojibake; three knives", "result": "green", "at": "5d1483ca21c4a3a02207d143cf7e87f8d90e3492" }, "next": "none — ready for review" }