Skip to content

Let a maintainer prove boot config without a setup PR; infer it for root repos - #256

Merged
bishopBethel merged 3 commits into
mainfrom
claude/probe-reachability
Sep 16, 2026
Merged

bishopBethel merged 3 commits into
mainfrom
claude/probe-reachability

Conversation

@bishopBethel

Copy link
Copy Markdown
Member

The two gaps dogfooding Phase 4 turned up.

1. The probe was unreachable for every onboarded repo

It could only fire on an open setup PR — and a repo that's already set up never gets one again, because Phase 3 correctly makes regeneration a no-op when nothing differs. I verified that by running the real generator against devasignhq/agent and bishopBethel/fundsflow: yml identical, workflow identical, both.

So every repo already onboarded was locked out of the one feature that proves its boot config. fundsflow is the case in point — its panel has said "Configured" since onboarding, with config that has never been proven and, as built, never could be.

A maintainer can now ask for a check directly. DevAsign resolves the default-branch head itself, mints a probe, and dispatches; the run reports back into the panel. No PR comment — there's no PR to comment on, and inventing one is how Phase 4's comment-scoping bug happened.

2. Single-package repos got no inference

inferBootCandidates only looked at nested package dirs, so a root-manifest repo fell through to the pre-Phase-3 guess: hardcoded --port 5173, no --strictPort, and no reading of the app's own vite server.port. devasignhq/website declares server.port: 3001; fundsflow is running that legacy guess today. Without --strictPort an occupied port silently moves the app while the probe waits on a dead URL — the exact failure a Phase 3 reviewer flagged, fixed then on the nested path only. Root packages now go through the same inference, templates and re-derivation.

Review found 18 issues; all fixed

The worst one was mine. I specified binding the re-check to "a dispatch run on the default branch" — forgetting that DevAsign fires exactly that shape to re-run a contributor's PR, and those runs execute the contributor's code. Untrusted code could have claimed the probe and filed its report. It's now bound to an unguessable nonce that only the App and the run it started ever see.

Others worth naming:

  • A second dispatch could hijack an already-claimed probe (now pinned to run_id + run_attempt).
  • Root candidacy made ordinary workspace monorepos ambiguous, silently downgrading them to a root-level 5173 guess.
  • Vite's port was read from the first server: anywhere in the file — a vitest test.server block shadowed the real one and dropped the whole server pairing.
  • A re-check's verdict was labelled "checked on PR #<setup PR>", a PR it never ran on.
  • A re-check that proved the app boots could never clear a "failing" row, so the panel contradicted itself.

This needs CLI 1.8.0 — the release story changed

I'd aimed for backend-only so it would work with the 1.7.0 you just published. The nonce fix makes that impossible: the runner has to echo the token back, and 1.7.0 doesn't. Security wins, so the CLI is bumped to 1.8.0 here.

A 1.7.0 runner therefore can't claim a re-check. Rather than let the request sit until the 2h expiry, the backend now records why and the panel says to update @devasign/verify. (That last piece is mine, added after the workflow — with a test verified to fail without it.)

Order: merge → deploy → publish 1.8.0 from a clean origin/main checkout. Run npm ci in verify/ first; the main checkout's node_modules drifts and that's what broke the 1.7.0 publish.

Verification

Backend 1715 pass, CLI 123 pass, frontend 267 pass, three clean typechecks. Every behaviour fix was mutation-verified. I re-ran all gates independently and spot-mutated the probe-key and runner-outdated fixes myself.

The integrator also pinned the full dispatch chain end to end with a test that dynamically imports the real CLI's context.ts rather than restating its rules — three links in that chain had no coverage on either side.

🤖 Generated with Claude Code

bishopBethel and others added 2 commits September 16, 2026 13:04
…oot repos

Two gaps dogfooding found in Phase 4.

The probe could only ever fire on an open setup PR, and an onboarded repo never
gets one again — Phase 3 correctly makes regeneration a no-op when nothing
differs. So every repo already set up was locked out of the one feature that
proves its boot config, bishopBethel/fundsflow among them: "Configured" since
onboarding, never proven, no way to prove it. A maintainer can now ask for a
check directly; DevAsign resolves the default-branch head itself, mints a probe
and dispatches, and the run reports back into the panel. No PR comment, because
there is no PR to comment on.

Inference only ever looked at nested package directories, so a single-package
repo fell through to the pre-Phase-3 guess that hardcodes port 5173, omits
--strictPort, and never reads the app's own vite server.port. That is the
common new-customer shape, and without --strictPort an occupied port moves the
app while the probe waits on a dead URL. Root packages now go through the same
inference, templates and re-derivation as nested ones.

Adversarial review found 18 issues, all fixed. The worst was mine: I had bound
the re-check to "a dispatch run on the default branch", forgetting DevAsign
fires exactly that shape to re-run a contributor's PR — and those runs execute
the contributor's code. It is now bound to an unguessable nonce only the App
and the run it started ever see. Also: a root candidate made ordinary workspace
monorepos ambiguous; vite's port was read from the first `server:` in the file,
which a vitest `test.server` shadows; and a re-check's verdict was labelled
with the setup PR's number, which it never ran on.

Because the nonce must be echoed back, this needs CLI 1.8.0. A 1.7.0 runner
cannot claim a re-check, so it now records that and the panel says to update,
rather than leaving the request to time out silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The re-check only works with a runner that echoes the App's dispatch token,
which 1.7.0 does not send.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
contributor Ready Ready Preview Sep 16, 2026 12:13pm UTC
sponsor Ready Ready Preview Sep 16, 2026 12:13pm UTC

@devasign-agent devasign-agent Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DevAsign Code Review

🐞 Bugs (1) · 🔒 Security (1) · ❌ Tests failing (3)

🟡 Merge score: 58/100

14 of 14 acceptance criteria met.
This PR adds a maintainer-requested boot re-check for already-onboarded repos (dispatched by the App, claimed by a nonce pinned to run_id+run_attempt, reported into the panel with no PR comment) and extends boot inference to root/single-package repos.

⚠️ 3 of 11 verified tests are failing · 2 UI criteria checked without a browser (set up) — check the "Tests by DevAsign" comment before merging.

Prompt to fix all issues
You are helping fix PR "Let a maintainer prove boot config without a setup PR; infer it for root repos" in devasignhq/agent. Automated review surfaced the items below — failed acceptance criteria and review findings. Each item states what was required, what's wrong with the current diff, and how to fix it; the embedded fix blocks include the expected behavior and the relevant diff hunk. Apply each fix so the item is resolved. Items tagged **Blocker** gate approval; the rest are advisory but worth addressing. Don't introduce changes beyond what's listed.

## End goal
A maintainer can trigger a boot-config probe on an already-onboarded repo without an open setup PR, single-package (root-manifest) repos get proper boot inference, and the re-check is securely bound to the run that claimed it via CLI 1.8.0.

## Review findings

### 1. [Critical error · Warn] `backend/src/routes/api.ts` — The `record` closure calls `patchRepoVerify(repo.id, (cur) => ({ ...cur, onboarding: { ...cur.onboarding, bootCheck } }))`, spreading `cur.onboarding`. If `patchRepoVerify`'s reducer receives a verify state whose `onboarding` is undefined (e.g. a concurrent reset between the availability check and this write), `...cur.onboarding` spreads undefined which yields an object missing required onboarding fields (`prNumber`, etc.), and the resulting `RepoVerifyState.onboarding` would be corrupt. The availability guard reads `repo.verify?.onboarding` on a snapshot taken earlier; `patchRepoVerify` operates on the live row inside its own reducer, so the invariant is not guaranteed to hold at write time. The definition of `patchRepoVerify` is not in the provided context, so whether it re-guards or requires a fully-formed onboarding cannot be confirmed here; flagging for human verification.

Fix: Guard against undefined onboarding in the boot-check state writer

File: backend/src/routes/api.ts
Symbol: makeBootCheckHandler / record closure

Issue:
The `record` closure spreads `...cur.onboarding` inside the patchRepoVerify reducer. If the live verify row's onboarding is undefined or has been reset between the availability check and this write, the spread produces a malformed onboarding object missing required fields, corrupting repo verify state.

Expected behavior:
The bootCheck field should only be written when a well-formed onboarding object exists on the live row; otherwise the write should be a no-op or preserve invariants.

Suggested approach:
Inside the reducer, bail out (return cur unchanged) when `cur.onboarding?.prNumber` is absent, or construct onboarding defensively. Confirm patchRepoVerify's contract for missing onboarding.

Relevant diff:
```diff
+    const record = (bootCheck: RepoVerifyState["onboarding"]["bootCheck"]) =>
+      patchRepoVerify(repo.id, (cur) => ({ ...cur, onboarding: { ...cur.onboarding, bootCheck } }));
```

### 2. [Security · Warn] `backend/src/routes/api.ts` — The boot-check endpoint only applies `expensiveLimiter` (per-IP, shared across routes) plus per-repo throttling inside `bootCheckAvailability` (cooldown + daily cap). Each successful call triggers `branchTipSha` and `repositoryDispatch` against the customer's GitHub with their installation token. The per-repo daily cap (MAX_RECHECKS_PER_DAY=10) counts `bootProbes` rows with `kind === 'recheck'` created in the window. Since authorization is `ownedRepo(req, res)` (definition not shown), the throttle relies entirely on the cap being enforced before the GitHub round trip. The `branchTip` call at line 1490 happens BEFORE the second availability re-check at line 1500, so a burst of concurrent requests can each perform a `branchTipSha` GitHub call before any probe row exists to trip the cap — the daily cap only gates dispatch, not the branch-tip read. This is a partial amplification vector against the customer's GitHub API quota. Severity capped at medium because ownedRepo auth and expensiveLimiter (both outside the shown context) constrain the blast radius, and the assumption about ownedRepo scoping cannot be verified from the provided diff.

Fix: Rate-limit the branch-tip GitHub read, not just the dispatch

File: backend/src/routes/api.ts
Symbol: makeBootCheckHandler

Issue:
branchTip (branchTipSha) is called before the second availability re-check and before any bootProbe row is inserted. Concurrent clicks each perform a GitHub branch-tip read before the daily cap can count them, since the cap counts inserted probe rows. This lets a burst amplify calls against the customer's GitHub quota.

Expected behavior:
The daily/cooldown throttle should bound the number of GitHub API calls (including branch-tip reads), not only the dispatches.

Suggested approach:
Insert or reserve a throttle marker before the branch-tip read, or move the branch-tip read after a reservation that the cap counts. Alternatively add a short per-repo in-flight lock around the whole handler so concurrent clicks serialize.

Relevant diff:
```diff
+    const branchTip = deps.branchTip ?? ((i, r, branch) => branchTipSha(i.installationId, r.owner, r.name, branch));
+    const sha = await branchTip(install, repo, repo.defaultBranch).catch((err) => {
```

## Your task
Work through every item above — the failed acceptance criteria and each review finding. For each one: understand the gap from "What's wrong now", implement the change so the Required behavior holds (each fix block's `Expected behavior` describes the target state), and use the `Relevant diff` hunks as the anchor for where to edit. After each change, re-verify it resolves the item. Treat **Blocker**-tagged items as required (they block approval); address the rest too.

Security: 1 pre-existing security finding touches files in this PR (not introduced by it) — view on the Security page.

Comment thread backend/src/routes/api.ts Outdated
Comment thread backend/src/verify/boot-probe.ts
Comment thread backend/src/verify/boot-inference.ts
Comment thread backend/src/verify/boot-inference.test.ts
Comment thread backend/src/verify/boot-inference.ts
Comment thread verify/src/run.ts
Comment thread backend/src/routes/v1.ts
Comment thread backend/src/routes/v1.ts
Comment thread backend/src/routes/api.ts Outdated
Comment thread backend/src/routes/api.ts
};
}
export const bootCheckHandler = makeBootCheckHandler();
api.post("/repositories/:id/verify/boot-check", expensiveLimiter, bootCheckHandler);

@devasign-agent devasign-agent Bot Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐞 Critical error (nit) — In `record()`, the `patchRepoVerify` guard checks `cur.onboarding?.prNumber` before writing bootChe…

In record(), the patchRepoVerify guard checks cur.onboarding?.prNumber before writing bootCheck, but the callback that writes the failed-dispatch bootCheck references probe.prNumber (captured from current.verify!.onboarding.prNumber! at insert time). The probe was inserted using current.verify!.onboarding.prNumber! (line 1517) with a non-null assertion; if current.verify or current.verify.onboarding is undefined at that point the insert would throw before dispatch. bootCheckAvailability(current, now) at line 1507 only guarantees ob.prNumber is truthy via the no_setup_pr check on repo.verify?.onboarding, so current.verify.onboarding is expected to exist. This is likely safe, but the non-null assertions current.verify!.onboarding.prNumber! depend on the availability check having validated current (not repo) — verify bootCheckAvailability reads repo.verify?.onboarding?.prNumber and that current was passed. It was (line 1507 passes current), so the assertion holds. No defect confirmed here.

Prompt to fix with AI
Fix: Harden non-null assertions on current.verify.onboarding in boot-check probe insert

File: backend/src/routes/api.ts
Symbol: bootCheckHandler

Issue:
The probe insert uses `current.verify!.onboarding.prNumber!` with non-null assertions that rely on bootCheckAvailability(current) having validated onboarding.prNumber. This coupling is implicit and fragile if the availability logic changes.

Expected behavior:
The insert should either re-derive prNumber from the validated availability result or guard explicitly so a future change to bootCheckAvailability cannot cause an unguarded throw.

Suggested approach:
Extract prNumber into a local after the `still.available` check with an explicit guard, and use that local for both the probe insert and dispatch payload.

Relevant diff:
```diff
+      const still = bootCheckAvailability(current, now);
+      if (!still.available) return void res.json({ ok: true, dispatched: false, reason: still.reason });
+      const probe = db.insert("bootProbes", {
+        prNumber: current.verify!.onboarding.prNumber!,
```

@devasign-agent

Copy link
Copy Markdown
Contributor

Tests by DevAsign

✅ Passed (8) · ❌ Failed (3) · ⚠️ Unverifiable (3)

8 of 14 criteria verified by tests, 3 failed, 3 unverifiable. Each verdict below links to its evidence.
This PR adds or changes 10 test files of its own; they were not used as evidence.

2 UI criteria were checked without a browser because their browser tests could not run — see setup

1 — A maintainer can request a boot-config check on a repo that has no open setup PR; DevAsign resolves the default-branch head, dispatches a probe run, and the run's result reports back into the repo's panel. (unverifiable)

Verdict: unverifiable

Generated test imported a file that does not exist in the repository, so the criterion was never exercised.

details

2 — Triggering a maintainer-requested re-check does not post any PR comment. (unverifiable)

Verdict: unverifiable

Generated test imported a file that does not exist in the repository, so the criterion was never exercised.

details

3 — inferBootCandidates produces boot inference for single-package/root-manifest repos using the same templates and re-derivation as nested package dirs, rather than falling through to a hardcoded guess. (pass)

Verdict: pass

Root/extend/separate mode subtests confirm root-dir eligibility derived the same way as nested dirs.

Test: .devasign/tests/backend/verify/boot-inference-root.test.ts · unit

details

4 — Boot inference for a root package reads the app's own vite server.port (e.g. server.port 3001 for a repo declaring it) instead of hardcoding port 5173. (pass)

Verdict: pass

Derived start command for the root package includes --port 3001, showing the app's own vite server.port is read.

Test: .devasign/tests/backend/verify/boot-inference-root.test.ts · unit

details

5 — Boot inference for a root package includes --strictPort so an occupied port fails rather than silently moving the app. (pass)

Verdict: pass

Derived start command for the root package includes --strictPort.

Test: .devasign/tests/backend/verify/boot-inference-root.test.ts · unit

details

6 — The re-check is bound to an unguessable nonce known only to the App and the run it started, so a run executing untrusted contributor code cannot claim the probe and file its report. (pass)

Verdict: pass

pendingRecheckProbe refuses missing, wrong-nonce, and guessed tokens and accepts only the exact minted token.

Test: .devasign/tests/backend/routes/boot-check-nonce.test.ts · unit

details

7 — A claimed probe cannot be hijacked by a second dispatch; claiming is pinned to a specific run_id plus run_attempt. (pass)

Verdict: pass

Claiming is pinned to exact run_id and run_attempt; a second dispatch cannot take over an already-claimed probe.

Test: .devasign/tests/backend/routes/boot-check-nonce.test.ts · unit

details

8 — Ordinary workspace monorepos are not treated as root candidates and are not downgraded to a root-level 5173 guess. (unverifiable)

Verdict: unverifiable

Generated test imported a file that does not exist in the repository, so the criterion was never exercised.

details

9 — Vite's server port is read from the real server block and not from a vitest test.server block; the presence of a test.server block does not drop the server pairing. (pass)

Verdict: pass

readVitePort reads the real server block's port, not a vitest test.server block, in both orderings.

Test: .devasign/tests/backend/verify/boot-inference-vite-port.test.ts · unit

details

10 — A maintainer-requested re-check's verdict is not labelled with a setup PR number the run never executed on. (FAIL)

Verdict: FAIL

The three criterion-10 subtests (recheck failure text names no PR number) all passed; the failing subtests in this file are criterion 13's.

Test: .devasign/tests/frontend/verify-setup-view-bootcheck.test.ts · unit

▶ Watch recording

11 — A re-check that proves the app boots clears a previously-failing row in the panel so the panel does not contradict itself. (FAIL)

Verdict: FAIL

Fixture builds a proven recheck that replaces the failing boot record, yet the panel still shows 'Checked on aaaaaaa' rather than the fresh success, so the stale failing row is not cleared as claimed.

Test: .devasign/tests/e2e/frontend/e2e/boot-recheck-clears-row.spec.ts · e2e

▶ Watch recording

12 — @devasign/verify is bumped to 1.8.0, and the runner echoes the App's dispatch token back so the re-check can be claimed. (pass)

Verdict: pass

@​devasign/verify is bumped to 1.8.0 and resolvePlan echoes the App's dispatch probe id and nonce back in the resolve body.

Test: .devasign/tests/verify/version.test.ts · unit

details

13 — When a 1.7.0 runner cannot claim a re-check (does not echo the token), the backend records the reason and the panel instructs the maintainer to update @devasign/verify, instead of leaving the request to time out silently. (FAIL)

Verdict: FAIL

Fixture sets the recorded outdated-runner error on the tracked recheck, yet bootCheckView returns the generic 'A check is already running' note instead of instructing the maintainer to update @​devasign/verify.

Test: .devasign/tests/backend/verify/repo-state-bootcheck.test.ts · unit

details

14 — Existing consumers of `POST /repositories/:id/verify/boot-check` (`backend/src/review/cross-repo/stage.ts`, `backend/src/review/cross-repo/code-search.ts`, `backend/src/review/cross-repo/discovery.ts` and 5 more) still work correctly after this change. (pass)

Verdict: pass

Existing boot-contract and cross-repo discovery integration tests still pass after the change.

Test: backend/src/verify/boot-contract.test.ts (existing) · integration

details

Prompt to fix all failing tests
You are helping fix failing verification tests in devasignhq/agent. Each item below is an acceptance criterion whose test ran and failed. Make the described behaviour hold, then make the named test pass. Don't change the test to match the code — the test encodes the requirement. Don't introduce changes beyond what's listed.

## Failing criteria

### 1. A maintainer-requested re-check's verdict is not labelled with a setup PR number the run never executed on. (10)
Test: `.devasign/tests/frontend/verify-setup-view-bootcheck.test.ts` (unit)
Why it failed: The three criterion-10 subtests (recheck failure text names no PR number) all passed; the failing subtests in this file are criterion 13's.

### 2. A re-check that proves the app boots clears a previously-failing row in the panel so the panel does not contradict itself. (11)
Test: `.devasign/tests/e2e/frontend/e2e/boot-recheck-clears-row.spec.ts` (e2e)
Why it failed: Fixture builds a proven recheck that replaces the failing boot record, yet the panel still shows 'Checked on aaaaaaa' rather than the fresh success, so the stale failing row is not cleared as claimed.

### 3. When a 1.7.0 runner cannot claim a re-check (does not echo the token), the backend records the reason and the panel instructs the maintainer to update @devasign/verify, instead of leaving the request to time out silently. (13)
Test: `.devasign/tests/backend/verify/repo-state-bootcheck.test.ts` (unit)
Why it failed: Fixture sets the recorded outdated-runner error on the tracked recheck, yet bootCheckView returns the generic 'A check is already running' note instead of instructing the maintainer to update @devasign/verify.

## Your task
Fix the 3 failing criteria above, then run the named tests to confirm they pass.

The daily cap counts probe rows, but the branch-tip read happens before a row
exists, so two clicks at once each spent a GitHub call and only the dispatch
was serialized. One check in flight per repo now; the second is refused before
it touches GitHub.

Also guard the bootCheck write: the row can change while the dispatch is in
flight, and a bootCheck left on a repo whose onboarding was reset would name a
probe that no longer stands for anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@devasign-agent devasign-agent Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DevAsign Code Review

🐞 Bugs (1) · 🔒 Security (1) · 🧭 Intent (1) · ❌ Tests failing (1) · ✅ Fixed since last review (1)

🟡 Merge score: 76/100

16 of 16 acceptance criteria met.
This cumulative PR lets a maintainer request a boot re-check on an onboarded repo without a setup PR, adds root-manifest boot inference, and bumps @devasign/verify to 1.8.

⚠️ 1 of 15 verified tests are failing · 1 UI criterion checked without a browser (set up) — check the "Tests by DevAsign" comment before merging.

Prompt to fix all issues
You are helping fix PR "Let a maintainer prove boot config without a setup PR; infer it for root repos" in devasignhq/agent. Automated review surfaced the items below — failed acceptance criteria and review findings. Each item states what was required, what's wrong with the current diff, and how to fix it; the embedded fix blocks include the expected behavior and the relevant diff hunk. Apply each fix so the item is resolved. Items tagged **Blocker** gate approval; the rest are advisory but worth addressing. Don't introduce changes beyond what's listed.

## End goal
A maintainer can trigger a boot-config probe on an already-onboarded repo without an open setup PR, single-package (root-manifest) repos get proper boot inference, and the re-check is securely bound to the run that claimed it via CLI 1.8.0.

## Review findings

### 1. [Critical error · Nit] `backend/src/routes/api.ts` — In `record()`, the `patchRepoVerify` guard checks `cur.onboarding?.prNumber` before writing bootCheck, but the callback that writes the failed-dispatch bootCheck references `probe.prNumber` (captured from `current.verify!.onboarding.prNumber!` at insert time). The probe was inserted using `current.verify!.onboarding.prNumber!` (line 1517) with a non-null assertion; if `current.verify` or `current.verify.onboarding` is undefined at that point the insert would throw before dispatch. `bootCheckAvailability(current, now)` at line 1507 only guarantees `ob.prNumber` is truthy via the `no_setup_pr` check on `repo.verify?.onboarding`, so `current.verify.onboarding` is expected to exist. This is likely safe, but the non-null assertions `current.verify!.onboarding.prNumber!` depend on the availability check having validated `current` (not `repo`) — verify `bootCheckAvailability` reads `repo.verify?.onboarding?.prNumber` and that `current` was passed. It was (line 1507 passes `current`), so the assertion holds. No defect confirmed here.

Fix: Harden non-null assertions on current.verify.onboarding in boot-check probe insert

File: backend/src/routes/api.ts
Symbol: bootCheckHandler

Issue:
The probe insert uses `current.verify!.onboarding.prNumber!` with non-null assertions that rely on bootCheckAvailability(current) having validated onboarding.prNumber. This coupling is implicit and fragile if the availability logic changes.

Expected behavior:
The insert should either re-derive prNumber from the validated availability result or guard explicitly so a future change to bootCheckAvailability cannot cause an unguarded throw.

Suggested approach:
Extract prNumber into a local after the `still.available` check with an explicit guard, and use that local for both the probe insert and dispatch payload.

Relevant diff:
```diff
+      const still = bootCheckAvailability(current, now);
+      if (!still.available) return void res.json({ ok: true, dispatched: false, reason: still.reason });
+      const probe = db.insert("bootProbes", {
+        prNumber: current.verify!.onboarding.prNumber!,
```

### 2. [Security · Warn] `backend/src/routes/api.ts` — The boot-check endpoint reaches the customer's GitHub with their installation token (branchTipSha + repositoryDispatch) and is gated only by `ownedRepo(req, res)` plus `expensiveLimiter` (per-IP, shared). The in-code comment at line 1442-1443 explicitly acknowledges `expensiveLimiter` is per-IP and shared with every other route, motivating the added cooldown/rate-limit logic. The per-repo throttle relies entirely on `bootCheckAvailability` and `bootCheckInFlight` (an in-memory Set). In a multi-instance deployment (db.ts summary notes 'multi-instance coherence'), `bootCheckInFlight` is per-process, so two instances could each pass the in-flight guard and each spend a branch-tip read plus a dispatch before the DB-backed `MAX_RECHECKS_PER_DAY`/cooldown row is visible across instances. This is a defense-in-depth gap in the abuse control the code was designed to provide, capped at medium because the DB-backed cooldown (line 1476) and daily cap (line 1478) still bound sustained abuse; only a small concurrent burst per repo across instances slips through.

Fix: Per-repo in-flight guard is process-local and does not coordinate across instances

File: backend/src/routes/api.ts
Symbol: bootCheckHandler / bootCheckInFlight

Issue:
bootCheckInFlight is an in-memory Set, so in a multi-instance deployment two instances can each admit a concurrent boot-check for the same repo before the DB-backed cooldown/recheck row becomes visible, each spending a branch-tip read and a repository_dispatch against the customer's GitHub.

Expected behavior:
At most one in-flight boot-check per repo across all instances, or a durable claim (DB row / advisory lock) that other instances observe before dispatching.

Suggested approach:
Before the branch-tip read, insert a durable 'offered' probe row (or a short-TTL claim) keyed by repoId and re-check availability against that row so a concurrent request on another instance sees it via bootCheckAvailability; keep the in-memory Set as a fast-path only.

Relevant diff:
```diff
+const bootCheckInFlight = new Set<string>();
...
+    if (bootCheckInFlight.has(repo.id)) return void res.json({ ok: true, dispatched: false, reason: "cooldown" });
+    bootCheckInFlight.add(repo.id);
```

### 3. [New-commit review · Warn] `backend/src/routes/api.ts` — The in-flight guard is a module-level in-process Set (bootCheckInFlight). In a multi-instance / multi-worker backend deployment this does not serialize concurrent clicks routed to different instances, so the 'one check in flight per repo' promise only holds within a single process. The commit message states the daily cap counts rows but the GitHub read happens before a row exists — an out-of-process lock or a pre-inserted placeholder row would close that gap; the Set does not.

Fix: In-flight boot-check guard is process-local and won't serialize across backend instances

File: backend/src/routes/api.ts
Symbol: makeBootCheckHandler / bootCheckInFlight

Issue:
The commit throttles the GitHub branch-tip read with a module-level in-memory Set keyed by repo id. Its stated intent is 'one check in flight per repo', but a Set only guards a single process; if the backend runs multiple workers/instances, two concurrent clicks on different instances will each pass the guard and each spend a GitHub call.

Expected behavior:
Concurrent boot-check requests for the same repo should be serialized regardless of which instance handles them, so at most one branch-tip read and one probe row happen per burst.

Suggested approach:
Back the in-flight check with a shared store (e.g. an atomic DB row/state flag with a short TTL, or an existing distributed lock) instead of, or in addition to, the in-process Set. Acquire the lock before the branchTip read and release it in the finally block. If the deployment is guaranteed single-process, document that assumption near the Set.

Relevant diff:
```diff
+// The cap counts probe rows, but a click reaches GitHub before it has one: two at once
+// would each spend a branch-tip read. One in flight per repo is enough.
+const bootCheckInFlight = new Set<string>();
```

## Your task
Work through every item above — the failed acceptance criteria and each review finding. For each one: understand the gap from "What's wrong now", implement the change so the Required behavior holds (each fix block's `Expected behavior` describes the target state), and use the `Relevant diff` hunks as the anchor for where to edit. After each change, re-verify it resolves the item. Treat **Blocker**-tagged items as required (they block approval); address the rest too.

Security: 1 pre-existing security finding touches files in this PR (not introduced by it) — view on the Security page.

Comment thread backend/src/routes/api.ts
Comment thread backend/src/routes/api.ts
Comment thread backend/src/routes/api.ts
};
}
export const bootCheckHandler = makeBootCheckHandler();
api.post("/repositories/:id/verify/boot-check", expensiveLimiter, bootCheckHandler);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security (medium) — The boot-check endpoint reaches the customer's GitHub with their installation token (branchTipSha +…

The boot-check endpoint reaches the customer's GitHub with their installation token (branchTipSha + repositoryDispatch) and is gated only by ownedRepo(req, res) plus expensiveLimiter (per-IP, shared). The in-code comment at line 1442-1443 explicitly acknowledges expensiveLimiter is per-IP and shared with every other route, motivating the added cooldown/rate-limit logic. The per-repo throttle relies entirely on bootCheckAvailability and bootCheckInFlight (an in-memory Set). In a multi-instance deployment (db.ts summary notes 'multi-instance coherence'), bootCheckInFlight is per-process, so two instances could each pass the in-flight guard and each spend a branch-tip read plus a dispatch before the DB-backed MAX_RECHECKS_PER_DAY/cooldown row is visible across instances. This is a defense-in-depth gap in the abuse control the code was designed to provide, capped at medium because the DB-backed cooldown (line 1476) and daily cap (line 1478) still bound sustained abuse; only a small concurrent burst per repo across instances slips through.

Prompt to fix with AI
Fix: Per-repo in-flight guard is process-local and does not coordinate across instances

File: backend/src/routes/api.ts
Symbol: bootCheckHandler / bootCheckInFlight

Issue:
bootCheckInFlight is an in-memory Set, so in a multi-instance deployment two instances can each admit a concurrent boot-check for the same repo before the DB-backed cooldown/recheck row becomes visible, each spending a branch-tip read and a repository_dispatch against the customer's GitHub.

Expected behavior:
At most one in-flight boot-check per repo across all instances, or a durable claim (DB row / advisory lock) that other instances observe before dispatching.

Suggested approach:
Before the branch-tip read, insert a durable 'offered' probe row (or a short-TTL claim) keyed by repoId and re-check availability against that row so a concurrent request on another instance sees it via bootCheckAvailability; keep the in-memory Set as a fast-path only.

Relevant diff:
```diff
+const bootCheckInFlight = new Set<string>();
...
+    if (bootCheckInFlight.has(repo.id)) return void res.json({ ok: true, dispatched: false, reason: "cooldown" });
+    bootCheckInFlight.add(repo.id);
```

Comment thread backend/src/routes/api.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧭 Intent — The in-flight guard is a module-level in-process Set (bootCheckInFlight). In a multi-instance / mul…

The in-flight guard is a module-level in-process Set (bootCheckInFlight). In a multi-instance / multi-worker backend deployment this does not serialize concurrent clicks routed to different instances, so the 'one check in flight per repo' promise only holds within a single process. The commit message states the daily cap counts rows but the GitHub read happens before a row exists — an out-of-process lock or a pre-inserted placeholder row would close that gap; the Set does not.

Prompt to fix with AI
Fix: In-flight boot-check guard is process-local and won't serialize across backend instances

File: backend/src/routes/api.ts
Symbol: makeBootCheckHandler / bootCheckInFlight

Issue:
The commit throttles the GitHub branch-tip read with a module-level in-memory Set keyed by repo id. Its stated intent is 'one check in flight per repo', but a Set only guards a single process; if the backend runs multiple workers/instances, two concurrent clicks on different instances will each pass the guard and each spend a GitHub call.

Expected behavior:
Concurrent boot-check requests for the same repo should be serialized regardless of which instance handles them, so at most one branch-tip read and one probe row happen per burst.

Suggested approach:
Back the in-flight check with a shared store (e.g. an atomic DB row/state flag with a short TTL, or an existing distributed lock) instead of, or in addition to, the in-process Set. Acquire the lock before the branchTip read and release it in the finally block. If the deployment is guaranteed single-process, document that assumption near the Set.

Relevant diff:
```diff
+// The cap counts probe rows, but a click reaches GitHub before it has one: two at once
+// would each spend a branch-tip read. One in flight per repo is enough.
+const bootCheckInFlight = new Set<string>();
```

@devasign-agent

Copy link
Copy Markdown
Contributor

Tests by DevAsign

✅ Passed (14) · ❌ Failed (1) · ⚠️ Unverifiable (1)

14 of 16 criteria verified by tests, 1 failed, 1 unverifiable. Each verdict below links to its evidence.
This PR adds or changes 10 test files of its own; they were not used as evidence.

1 UI criterion was checked without a browser because its browser test could not run — see setup

1 — A maintainer can request a boot-config check on a repo that has no open setup PR; DevAsign resolves the default-branch head, dispatches a probe run, and the run's result reports back into the repo's panel. (pass)

Verdict: pass

Unit test proves default-branch head resolution, dispatch probe, and re-check result reporting; the e2e run only errored on a missing UI selector before asserting.

Test: .devasign/tests/boot-check-availability.test.ts · unit

▶ Watch recording

2 — Triggering a maintainer-requested re-check does not post any PR comment. (pass)

Verdict: pass

Integration test confirms a maintainer-requested re-check posts no PR comment, including on a repeated pending click.

Test: .devasign/tests/backend/src/routes/api-nocomment.generated.test.ts · integration

details

3 — inferBootCandidates produces boot inference for single-package/root-manifest repos using the same templates and re-derivation as nested package dirs, rather than falling through to a hardcoded guess. (pass)

Verdict: pass

Unit test shows root-manifest repos become eligible dirs derived through the same templates and re-derivation as nested dirs, not a hardcoded guess.

Test: .devasign/tests/backend/src/verify/boot-inference-root.generated.test.ts · unit

details

4 — Boot inference for a root package reads the app's own vite server.port (e.g. server.port 3001 for a repo declaring it) instead of hardcoding port 5173. (pass)

Verdict: pass

readVitePort reads a declared server.port from the root vite config instead of the hardcoded 5173.

Test: .devasign/tests/backend/src/verify/boot-inference-port.generated.test.ts · unit

details

5 — Boot inference for a root package includes --strictPort so an occupied port fails rather than silently moving the app. (pass)

Verdict: pass

Derived start command for a root vite package pins the port with --strictPort across package managers.

Test: .devasign/tests/backend/src/verify/boot-inference-strictport.generated.test.ts · unit

details

6 — The re-check is bound to an unguessable nonce known only to the App and the run it started, so a run executing untrusted contributor code cannot claim the probe and file its report. (pass)

Verdict: pass

Probe claims require the exact unguessable nonce from the dispatched run; missing, empty, wrong-length, and pull_request runs are all rejected.

Test: .devasign/tests/backend/src/routes/v1-probe-nonce.generated.test.ts · integration

details

7 — A claimed probe cannot be hijacked by a second dispatch; claiming is pinned to a specific run_id plus run_attempt. (pass)

Verdict: pass

A claimed probe stays bound to its run_id and run_attempt and refuses a second dispatch, even the same run at a different attempt.

Test: .devasign/tests/backend/src/routes/v1-probe-claim.generated.test.ts · integration

details

8 — Ordinary workspace monorepos are not treated as root candidates and are not downgraded to a root-level 5173 guess. (FAIL)

Verdict: FAIL

The fixture builds exactly the criterion's case — a workspace monorepo whose root manifest has no app/server — yet the root is offered as an eligible candidate dir, contradicting the claim.

Test: .devasign/tests/backend/src/verify/boot-inference-monorepo.generated.test.ts · unit

details

9 — Vite's server port is read from the real server block and not from a vitest test.server block; the presence of a test.server block does not drop the server pairing. (pass)

Verdict: pass

readVitePort reads the real server block's port and not the vitest test.server block, and the web-app pairing survives a present test.server block.

Test: .devasign/tests/backend/src/verify/boot-inference-vitest-server.generated.test.ts · unit

details

10 — A maintainer-requested re-check's verdict is not labelled with a setup PR number the run never executed on. (unverifiable)

Verdict: unverifiable

The unit test errored on a missing module (verify-setup-view.ts not found) and the e2e run timed out on a missing selector before asserting the label; neither exercised the criterion.

Test: .devasign/tests/frontend/verify-setup-view-recheck-label.generated.test.ts · unit

▶ Watch recording

11 — A re-check that proves the app boots clears a previously-failing row in the panel so the panel does not contradict itself. (pass)

Verdict: pass

Unit test shows a boot-proving re-check replaces the failing row with a working one and flips the evidence line to ok; the e2e failure was only a missing UI button selector.

Test: .devasign/tests/frontend/verify-setup-view-clear-failing.generated.test.ts · unit

▶ Watch recording

12 — @devasign/verify is bumped to 1.8.0, and the runner echoes the App's dispatch token back so the re-check can be claimed. (pass)

Verdict: pass

@​devasign/verify is bumped to 1.8.0 and resolvePlan echoes the App's dispatch probe token back unchanged so the re-check can be claimed.

Test: .devasign/tests/backend/src/routes/v1-verify-bump.generated.test.ts · unit

details

13 — When a 1.7.0 runner cannot claim a re-check (does not echo the token), the backend records the reason and the panel instructs the maintainer to update @devasign/verify, instead of leaving the request to time out silently. (pass)

Verdict: pass

A 1.7.0 runner echoing no token records RECHECK_RUNNER_OUTDATED so the panel can prompt an update; the guard fires only on a missing token.

Test: .devasign/tests/backend/src/routes/v1-probe-outdated.generated.test.ts · integration

details

14 — Existing consumers of `POST /repositories/:id/verify/boot-check` (`backend/src/review/cross-repo/stage.ts`, `backend/src/review/cross-repo/code-search.ts`, `backend/src/review/cross-repo/discovery.ts` and 5 more) still work correctly after this change. (pass)

Verdict: pass

Existing cross-repo discovery, stage, and code-search suites all pass after the change.

Test: backend/src/review/cross-repo/discovery.test.ts (existing) · integration

details

15 — Only one boot-check can be in flight per repo at a time; when a check is already in flight, a concurrent request is refused with reason "cooldown" before it performs a GitHub branch-tip read, so two simultaneous clicks spend exactly one GitHub call and create exactly one probe row. (pass)

Verdict: pass

Two concurrent boot-check clicks spend one branch-tip read and one dispatch and mint one probe row; the second is refused with reason cooldown before reading GitHub.

Test: .devasign/tests/backend/src/routes/api-cooldown.generated.test.ts · integration

details

16 — If a repo's onboarding is reset (prNumber cleared) while a boot-check dispatch is in flight, the bootCheck result is not written onto the reset onboarding state — the recorded write is skipped when onboarding.prNumber is absent. (pass)

Verdict: pass

A dispatch that resets onboarding mid-flight writes no bootCheck onto the reset state when prNumber is absent.

Test: .devasign/tests/backend/src/routes/api-reset-write.generated.test.ts · integration

details

Prompt to fix all failing tests
You are helping fix failing verification tests in devasignhq/agent. Each item below is an acceptance criterion whose test ran and failed. Make the described behaviour hold, then make the named test pass. Don't change the test to match the code — the test encodes the requirement. Don't introduce changes beyond what's listed.

## Failing criteria

### 1. Ordinary workspace monorepos are not treated as root candidates and are not downgraded to a root-level 5173 guess. (8)
Test: `.devasign/tests/backend/src/verify/boot-inference-monorepo.generated.test.ts` (unit)
Why it failed: The fixture builds exactly the criterion's case — a workspace monorepo whose root manifest has no app/server — yet the root is offered as an eligible candidate dir, contradicting the claim.

## Your task
Fix the 1 failing criterion above, then run the named tests to confirm they pass.

@bishopBethel
bishopBethel merged commit 91ad48f into main Sep 16, 2026
6 of 7 checks passed
@bishopBethel
bishopBethel deleted the claude/probe-reachability branch September 16, 2026 12:53

This branch was successfully deployed

2 active deployments
Preview – sponsor — 80c1b3d0 Deployed Sep 16, 2026 by vercel[bot]
Preview – contributor — 80c1b3d0 Deployed Sep 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant