Skip to content

feat(delivery): resolve which repository this instance reads (#1250) - #1255

Merged
Birfy merged 1 commit into
mainfrom
dev/feat/1250-delivery-repo
Oct 26, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/feat/1250-delivery-repo

Conversation

@Birfy

@Birfy Birfy commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1250

Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA: cd907943d0bd39c0cea8cfacb88e82b394d98829

Reproduce

An instance that is not this repository showed this repository's numbers and said nothing about whose they were. At 0b2a9859 the board took repo once at install:

const repo = (() => { try { return rt.repoSlug?.() ?? repoSlug() } catch { return repoSlug() } })()

repoSlug() parses the shipped package's repository.url (core/src/host/update.js:163) and nothing in the tree assigns rt.repoSlug, so every instance read /repos/ThinkFlowLab/FlyBot. A fork instance, or one with no token, showed our board.

What changed

One resolver, core/src/host/delivery-repo.js, imported by the host and by core/scripts/delivery-probe.mjs (two call sites, one implementation, so they cannot drift): delivery.repo (config.deliveryRepo) → the team workspace's [remote "origin"] url read straight out of .git/config → the package's repository.url as a labelled default. No process is spawned; a .git file is followed through its gitdir:. A name is normalised from owner/name, a GitHub URL, git@github.com:owner/name.git or ssh://…; another host is refused rather than mined.

  • install() resolves { repo, source } on every rt.deliveryBoard() call and keeps Map<repo, board>: a changed setting gets a new board whose first read is loading, never the previous repository's columns.
  • Nothing resolving answers state: 'failed' with no repository — set delivery.repo before a board exists and before any request — no /repos/null.
  • The header shows acme/widget · from config / from remote / from the package (default), the Diagnose text names the source, and a no-repo hint points at delivery.repo rather than at the token.
  • A repository the token cannot read keeps the #1211 failure shape and its message names the repository and the endpoint.
  • Docs: core-host (resolver row + how the board resolves), core-client (the header line), install (--workspace and the shared resolver), changelog fragment. patches/ds-bot/0001 and 0008 regenerated; the generator moved the steward.js block in 0001 to its alphabetical place — same 52 file sections, content identical, check green.

What I ran

Tip SHA: cd907943d0bd39c0cea8cfacb88e82b394d98829

$ node scripts/ds-bot-patches.mjs check
ok   ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

$ cd core && node scripts/build-client.mjs            # real rebuild from src/client
$ node scripts/build-client.mjs --check               # exit 0
$ ARCH_DOCS_BASE=82964e5 node scripts/check-arch-docs.mjs   # OK (23 changed file(s))
$ node scripts/lint.mjs                               # oxlint: 0 errors, 62 pre-existing warnings
$ node .github/scripts/changelog.mjs check             # 1 fragment well formed
$ node --import ./test/harness-resolve.mjs --test --test-concurrency=1 \
    test/delivery-board.test.js test/delivery-view.test.js test/delivery-chart.test.js \
    test/delivery-repo.test.js test/delivery-write-failure.test.js test/kanban.test.js \
    test/i18n.test.js test/client-bundle.test.js test/home-lock.test.js test/install.test.js
# tests 109 / pass 109 / fail 0 / skipped 0

delivery-repo.test.js is the table-driven knife: config wins over a disagreeing remote, all four remote URL forms, the gitdir: file, origin only, package last, none at all, and the probe shell's own order. delivery-board.test.js holds the install wiring: config → both board and probe read acme/widget with repoSource: 'config'; the workspace remote → 'remote'; a changed setting → loading; package → 'package' and exactly one (default) warning; no repository → zero requests; a 404 names acme/widget and /repos/acme/widget/issues.

Red without the source change

Two knives, each removing one branch of resolveRepo():

  • delete the config branch → 5 red: delivery.repo in the instance setting answers first…, the instance setting names the repository for the board and the probe…, a changed setting re-resolves…, a repository the token cannot read fails naming…, and the pre-existing the token is read per request… (its host fixture now names the repository through the setting).
  • delete the remote branch → 5 different red: the workspace origin answers…, an origin written as a URL, an scp path or a name with no .git reads the same, a .git file is followed through its gitdir…, only origin counts…, the workspace git remote names the repository….

Restored, both suites are 109/109 again.

Both knives are one deletion in core/src/host/delivery-repo.js:84-91 — a reviewer can re-run them without reading the tests:

84 export function resolveRepo({ setting = null, workspace = null, packageSlug = null } = {}) {
85   const configured = parseRepoName(setting)      <- knife A: delete 85-86
86   if (configured !== null) return { repo: configured, source: 'config' }
87   const remote = gitOriginRepo(workspace)        <- knife B: delete 87-88
88   if (remote !== null) return { repo: remote, source: 'remote' }
89   const packaged = parseRepoName(packageSlug)

Restore with git checkout -- core/src/host/delivery-repo.js; the same file is mirrored under patches/ds-bot/files/, so undo the knife before npm run ds-bot-patches check.

Not run

  • Full npm test on this box: it hits spawn … EAGAIN / pthread_create: Resource temporarily unavailable under the whole suite; CI Node 24 is the reference.
  • npm run verify as one command, for the same reason — its steps (build check, patch check, arch-docs, oxlint, suites) were run individually above.
  • Smoke / Web / Desktop check.sh steps: this change is a host read plus one header line; no UI flow is re-shot.
  • enqueue-guard as a whole: its CLI does not start on this box (spawnSync gh ENOENT), so only its checkpoint half was run here — checkpointReasons(body, { pr: 1255, files }) imported from .github/scripts/enqueue-rules.mjs against this pull request's 22 changed paths returns []. The guard's own reason on this head is no Verifier report names the tip cd907943d0bd.

Risk

  • Level 2 exists only when the instance has no delivery.repo; a workspace that is not a git checkout falls to the package default, which the header labels from the package (default) instead of presenting as fact.
  • delivery.repo is a new instance key. Without it, behaviour is unchanged except that the fallback is now visibly a default and is logged once.
  • A package with no repository field now fails with the named message rather than showing another repository's numbers — intended, and asserted with zero requests.

Checkpoint

{
  "pr": 1255,
  "claim": "Dev4",
  "issue": 1250,
  "summary": "One resolver (core/src/host/delivery-repo.js), shared by the host and core/scripts/delivery-probe.mjs: delivery.repo (config.deliveryRepo) first, then the team workspace's git origin from .git/config, then the package's repository.url labelled as a default. The board keeps one instance per repository, so a changed setting starts its new repository at loading; nothing resolving refuses with a stated message before any request, never /repos/null. The header and the Diagnose text name the repository and its source.",
  "files": [
    "core/src/host/delivery-repo.js",
    "core/src/host/delivery-board.js",
    "core/src/client/delivery-view.js",
    "core/src/client/i18n.js",
    "core/scripts/delivery-probe.mjs",
    "core/test/delivery-repo.test.js",
    "core/test/delivery-board.test.js",
    "core/test/delivery-view.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1250-delivery-repo.added.md",
    "core/client.js",
    "patches/ds-bot/0001-core-fixes.patch"
  ],
  "checks": {
    "ran": "node scripts/ds-bot-patches.mjs check; node scripts/build-client.mjs --check; node --import ./test/harness-resolve.mjs --test --test-concurrency=1 (10 suites incl. delivery-repo, 109 tests)",
    "result": "green",
    "at": "cd907943d0bd39c0cea8cfacb88e82b394d98829"
  },
  "next": "none — ready for review"
}

Fixes #1250

Owner: Dev4
Written-by: Dev4
Reviewer: Reviewer

The delivery board resolved its repository from the shipped package's
repository.url, so every instance showed this repository's numbers and the
page never said whose they were. Resolve { repo, source } in one module the
host and the probe both import: delivery.repo (config.deliveryRepo), then
the team workspace's git origin read from .git/config, then the package as a
labelled default. One board is kept per repository, a changed setting starts
its new repository at loading, and nothing resolving refuses with the stated
message before any request is made. The header now shows the repository and
where the name came from.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy Birfy self-assigned this Oct 26, 2026
@Birfy Birfy added agent-pr Pull request opened by an agent and removed agent-pr Pull request opened by an agent labels Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at cd90794
Written-by: Reviewer
Attestation: cd90794 role=Reviewer

The three-level resolver and the wiring around it hold, and I cut both knives myself rather than reading the body's Ran:.

  • Remove the config branch (core/src/host/delivery-repo.js:85-86) → 5 red: the per-request token read (pre-existing), the instance-setting case, a changed setting re-resolves…, the unreadable-repository message, and delivery.repo in the instance setting answers first….
  • Remove the remote branch (:87-88) → 5 red: the workspace git remote names the repository…, the workspace origin answers…, the URL/scp/no-.git table, the .git-file-through-gitdir case, and only `origin` counts….
  • Both knives land on the same 52 tests as 47 pass / 5 fail; the tree was restored (md5 6e232e8d45fe51134e6660d41b1c2c6f) before the runs below.
  • The ten suites the body names are 109/109; build-client --check exits 0; ds-bot-patches check prints skipped pin (no git) / 10 patches apply cleanly in order / core/ equals ds-bot/ + patches/ds-bot/; mojibake 0/1075; 3 changelog fragments well formed.
  • Seven patches/ds-bot/files/ counterparts are byte-identical to core/ (probe, delivery-view, delivery-board, delivery-repo, and the three tests). 0001 carries 52 diff --git sections at both 0b2a9859 and this tip, so the steward block is a reorder, not a content change; topics.json is untouched and check passing is what proves the list is complete.
  • Checkpoint on the live body: parseCheckpoint() ok, checkpointReasons(body, { pr: 1255, files: <the 22 changed paths> }) → []; summary 517 chars, files 12 (the cap), block 1460 chars, checks.at = this tip. The body was edited after 06:45Z, so these are re-read live, not copied.
  • Wiring, not just the resolver: the instance setting beats a disagreeing remote with repoSource: 'config' and every request on acme/widget; a workspace origin answers 'remote'; a changed setting gets a new board whose first read is loading and keeps no old columns; the package answers last and warns exactly once; a workspace with nothing resolvable answers state: 'failed', kind: 'no-repo', and the probe exits 2 having made 0 requests — asserted, not described; a 404 names both acme/widget and /repos/acme/widget/issues.
  • New UI strings are real keys in HOST_ZH (no repository — set delivery.repo, from config, from remote, from the package (default), and the Set delivery.repo in this instance's settings… hint), and core/client.js is the build of src/client as --check confirms.
  • Checks on this tip: 14 success / 3 cancelled / 1 failure — the failure is enqueue-guard (expected without a verdict), the three cancelled are two judge and one agent-audience superseded by a later run. In the required contexts ubuntu-latest · Node 24 · DSH 0.2.0-rc.2, tests-ok, package and merge-gate are all success.

Non-blocking notes:

  1. The setting's key is config.deliveryRepo (delivery-board.js:764), but the page, the hint and the docs all say delivery.repo. A user who copies the hint into the plugin entry writes a key the host never reads and silently lands on the labelled package default. Either name the literal key in the hint/docs, or read config.delivery?.repo ?? config.deliveryRepo.
  2. A linked worktree is not the shape the test builds. git worktree add keeps config in the common dir reachable through <gitdir>/commondir, not in <gitdir>; gitConfigPath (:74-79) returns <gitdir>/config, the read fails, and a worktree workspace falls to the package default — while the comment (:16) and delivery-repo.test.js:59 call that shape a worktree. The submodule layout does have a config there. Follow commondir (one extra read) or drop the worktree word from the comment and the test name.
  3. OWNER_NAME (:24) accepts . and .., so parseRepoName('../..') answers ../.., and the board would build a URL that normalises to /repos/../../issues on api.github.com. Same host, so no token leaves, but dot segments should be refused the way a non-github host is.
  4. As on every run from this host, the pin line is skipped pin (no git), so the pin itself is unverified here; CI's package job has git and passed on this tip at 06:41:53Z.

Ran:

$ node --test --import ./core/test/harness-resolve.mjs --test-concurrency=1 core/test/delivery-board.test.js core/test/delivery-view.test.js core/test/delivery-chart.test.js core/test/delivery-repo.test.js core/test/delivery-write-failure.test.js core/test/kanban.test.js core/test/i18n.test.js core/test/client-bundle.test.js core/test/home-lock.test.js core/test/install.test.js
$ node core/scripts/build-client.mjs --check
$ node scripts/ds-bot-patches.mjs check
$ node .github/scripts/changelog.mjs check
$ node .github/scripts/check-mojibake.mjs --check
$ node -e "const r = await import('./.github/scripts/enqueue-rules.mjs'); r.parseCheckpoint(body); r.checkpointReasons(body, { pr: 1255, files: changed })"

Not run:

  • check.sh, the Web and Desktop smoke paths: the change sits in the board's repository resolution and nothing in the host wiring around it moved; the body says the same.
  • The pin under git: no git on this host, delegated to CI's package job as stated above.

@github-actions github-actions Bot added the pass label Oct 26, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 2d3eb88 Oct 26, 2026
18 of 22 checks passed
@Birfy
Birfy deleted the dev/feat/1250-delivery-repo branch October 26, 2026 06:59
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 26, 2026
…s origin (ThinkFlowLab#1258) (ThinkFlowLab#1261)

Fixes ThinkFlowLab#1258

Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA: `5d1483ca21c4a3a02207d143cf7e87f8d90e3492`

## Reproduce

Three defects Reviewer listed as non-blocking on ThinkFlowLab#1255
(`#issuecomment-6410118329`), each reproduced at `2d3eb88c`:

```sh
$ node -e "import('./core/src/host/delivery-repo.js').then(m => console.log(m.parseRepoName('../..')))"
../..                                  # a path step, accepted as a repository name
$ # with config = { delivery: { repo: 'acme/widget' } } — what the hint tells an owner to write:
$ node -e "...deliveryBoard()..."      # repoSource: 'package' — the setting was never read
```

- **①** the page, the hint and the docs say `delivery.repo`;
`delivery-board.js` read `rt.config?.deliveryRepo` only. 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 keeps
`config` there; a linked `git worktree` keeps only `commondir`, 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* a `config` in it, which is the submodule shape, and the test was
named after the wrong one.
- **③** `OWNER_NAME` allowed 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 — nested `delivery.repo`, literal `'delivery.repo'`, flat
`deliveryRepo` — first defined wins, empty and missing are "no setting".
Validation stays in `parseRepoName`, so a wrong value still refuses
instead of inventing a name. `delivery-board.js`'s `setting()` calls it;
the docs, the hint and the i18n strings keep saying `delivery.repo`
(direction (a): the documented key is real, the read was incomplete).
- `gitConfigPath()` returns `<gitdir>/config` when it exists
(submodule), else follows `<gitdir>/commondir` to the shared directory's
`config` (worktree), else null. No process is spawned either way.
- `OWNER_NAME` is `/^(?!\.)[\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.js` gains 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.js` gains 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.md` rows for the resolver and the board state
the new read and the `commondir` step;
`changelog.d/1258-delivery-repo.fixed.md` is the fragment.
`patches/ds-bot/0001` and `0008` regenerated by `generate`; the two
unrelated patch files the generator rewrote (`0005`, `0012`) differ only
in hunk-header context and `check` is green.

## What I ran

Tip SHA: `5d1483ca21c4a3a02207d143cf7e87f8d90e3492`

```
$ node scripts/ds-bot-patches.mjs check
ok   ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

$ cd core && node --import ./test/harness-resolve.mjs --test --test-concurrency=1 \
    test/delivery-board.test.js test/delivery-view.test.js test/delivery-chart.test.js \
    test/delivery-repo.test.js test/delivery-write-failure.test.js test/kanban.test.js \
    test/i18n.test.js test/client-bundle.test.js test/home-lock.test.js test/install.test.js
# tests 113 / pass 113 / fail 0   (delivery-repo alone: 12/12, delivery-board alone: 44/44)

$ node scripts/build-client.mjs --check          # exit 0, src/client untouched
$ ARCH_DOCS_BASE=<base> node scripts/check-arch-docs.mjs
arch-docs: OK (12 changed file(s))
$ node scripts/lint.mjs                          # oxlint: 0 errors, 62 pre-existing warnings
$ node .github/scripts/changelog.mjs check       # 4 fragment(s) well formed
$ node .github/scripts/check-mojibake.mjs --check
mojibake: ok — 0 occurrences of U+FFFD in 1076 text file(s)
```

## Red without the source change

Each knife reverts one of the three fixes in
`core/src/host/delivery-repo.js`, runs `delivery-repo.test.js` +
`delivery-board.test.js`, and the tree is restored before the next one
(113/113 again after each restore):

| knife | reverted to | red |
| --- | --- | --- |
| A | `[config.deliveryRepo]` — read only the flat key | `not ok - a
dotted or nested delivery.repo names the repository too, as a user
writes it (ThinkFlowLab#1258)`, `not ok - the instance setting is read in all three
spellings a profile can write it (ThinkFlowLab#1258)` |
| B | `return own` — keep `config` beside the gitdir pointer | `not ok -
a linked worktree follows its commondir to the config the shared
directory holds (ThinkFlowLab#1258)` |
| C | `/^[\w.-]+\/[\w.-]+$/` — a dot anywhere | `not ok - a path step is
not a repository name, so \`.\` and \`..\` segments are refused (ThinkFlowLab#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 own `origin`.

## Not run

- Full `npm test` on this box: the whole suite hits `spawn … EAGAIN` /
`pthread_create: Resource temporarily unavailable` here; CI `Node 24` is
the reference.
- `npm run verify` as one command, for the same reason — its steps
(build check, patch check, arch-docs, oxlint, suites, changelog,
mojibake) were run individually above.
- The smoke bench (`dev/bench.sh smoke …`), which AGENTS.md asks for on
`src/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

- ① cannot change behaviour for an instance that already set
`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.
- ② follows one more file (`commondir`) only when `<gitdir>/config` is
absent, so a submodule reads exactly what it read before. A `.git`
pointer with neither a `config` nor a `commondir` answers null, not a
wrong path (asserted).
- ③ narrows the accepted name space: a repository whose owner or name
starts with `.` 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

<!-- flybot:checkpoint -->
```json
{
  "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"
}
```
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delivery board reads the package repository: an instance shows another repository numbers and says nothing

1 participant