feat: feat(loop): per-feature project resolution + per-project preflight isolation (#90 slice 2/3)

[mabry1985]

Summary

Slice 2/3 of the multi-project design (#90): threads the projects: map through the loop so every repo/gate/coder/preflight decision resolves from the FEATURE's project rather than the instance default.

• BoardLoop.__init__ now resolves self._projects = resolve_projects(cfg) and self._default_project, and hands the same map to the store via _store_kw.
• Added _project_cfg(feature) plus per-feature resolvers (_repo_for, _base_branch_for, _local_gate_cmd_for, _format_cmd_for, _gate_files_for, _repo_conventions_for, _coders_for, _coder_solve_settings, _all_repos). A feature carrying a project:<name> label resolves strictly to that project's settings; an unlabeled feature falls back to the default project, then the instance defaults — so a single-repo board and every pre-Design: multi-project boards — a projects: map so a team can own a repo plus its toolchain without repoint-restart detours #90 feature behave exactly as before.
• Preflight is now per-project. _preflight_state/_preflight_held/_last_preflight are dicts keyed by project name. _maybe_preflight smokes each project that has ready work (plus any still-failed project, so holds can recover), and _spawn_ready holds only the failed projects' features while continuing to dispatch healthy ones — a broken gate in one repo no longer HOLDs the whole board.
• The health sweep reaps orphaned worktrees across every project's checkout, and the dispatch prompt, coder ladder, and solve() search all read the feature's project config.

Test note: tests/test_env_sanitization.py::test_preflight_spawns_with_sanitized_env needed a one-line fake-store addition because preflight now queries ready work; its assertion is unchanged. Two suite failures are pre-existing and environmental (a tomllib import on Python 3.9 in test_packaging.py, and a br-version JSON-shape assertion in test_integration.py) — both fail identically on the unmodified tree.

Fixes #90

[protoreview]

QA panel review — WARN

code-review-structural · head efe16cfdbf1a · formal

[review-synthesizer completed: workflow code-review-structural:report]

All seven findings came back confirmed against the actual branch-tip code; there are no refutations and no uncertain items to mark, so the whole panel survives to the canonical list — re-ranked (major first, then minors, then the nit) with the verifier's verdict/note/evidence carried byte-for-byte. Per the protocol, no dispositions block is emitted since this panel has no prior requests.

Overall risk is moderate: no correctness regression in the single-repo path, but the PR's own new multi-repo/multi-project surface has a confirmed wrong-repo build (finding 1, major — the merge-blocking item, fix first: thread projects/default_project into the tool's store_kw / give board_create_feature a project param) plus removed fail-closed gate behavior and a per-feature escalation ladder that can't climb. The panel agreed on substance — all seven confirmed; the one nuance is finding 5, a deliberate per-project preflight tradeoff flagged as design-behavior, not accidental regression. Verification changed two things: it sharpened finding 1's binding mechanism (store repo-stamp + no-label create, not _project_cfg), and it confirmed grounding despite the pinned SHA efe16cf… 404'ing (force-push) — reads used branch/default-tip files matching the PR diff byte-for-byte, so treat as grounded but know the SHA pin is stale. Gaps: no tool-sourced structural pass surfaced this round, and the under-changed tool side (store.py/init.py, the board_create_feature call path) is exactly the load-bearing surface for the blocker — add a tool-mediated create test asserting the card builds in its project repo.

Findings

	Severity	Location	Finding	Verified
🟠	major	__init__.py	On a multi-repo board, features created through the board_create_feature tool build in the instance repo instead of a project repo: the tool's store_kw was not…	confirmed
🟡	minor	loop.py:2205	On a multi-project board whose config declares no default_project, unlabeled (pre-#90 back-compat) features lose the fail-closed gate preflight entirely: `_r…	confirmed
🟡	minor	loop.py:2200	The gate preflight smoke is no longer run at boot on an idle board — a deliberate per-project tradeoff in this PR, but still a removed-behavior change: OLD `_m…	confirmed
🟡	minor	loop.py:1565	Per-feature coder escalation is dead on boards whose flat config doesn't already enable it: _drive resolves coders = self._coders_for(feature) but switches on …	confirmed
🟡	minor	loop.py:850	_all_repos() does not actually enumerate 'every distinct repo root the board builds in': when any project declares a repo it omits the store default, while `…	confirmed
🟡	minor	loop.py	Per-feature project resolution was threaded into _reconcile_orphan, _salvage_verified_candidate, _verify_merged_state, _maybe_rebase, _reconcile_prs,…	confirmed
⚪	nit	loop.py:737	The seven per-feature resolvers each hand-roll the same 'labeled project lookup, else feature-stamped value, else _project_cfg, else store default' fallback …	confirmed

findings JSON (machine-readable)

[
  {
    "file": "__init__.py",
    "line": 0,
    "severity": "major",
    "category": "cross-file",
    "claim": "On a multi-repo board, features created through the board_create_feature tool build in the instance repo instead of a project repo: the tool's store_kw was not threaded with the projects/default_project map this PR hands the loop's store, and the tool has no `project` param, so created features carry no `project:` label and the store stamps them with the tool's flat instance repo — the loop's `_repo_for` then builds the card in that instance repo rather than any project repo.",
    "evidence": "Loop side (this PR): \"projects=self._projects,\\n            default_project=self._default_project,\" threaded into _store_kw → get_store(**self._store_kw). Tool side (unchanged, current tree): \"store_kw = dict(\\n        db=cfg.get(\"db_path\") or None,\\n        repo=cfg.get(\"repo\", \".\"),\\n        base_branch=cfg.get(\"base_branch\", \"main\"),\\n        max_files_by_difficulty=cfg.get(\"max_files_by_difficulty\"),\\n    )\" and \"f = store.create_feature(\\n                title,\\n                spec=spec,\\n ... \" with no project argument. In the loop, a feature whose project label is absent/unknown falls to \"self._store_kw[\"repo\"]\".",
    "verdict": "confirmed",
    "note": "Verified: tool _board_tools() builds flat-only store_kw (no projects/default_project) and board_create_feature passes no project → create_feature's normalize_project('' or default='')='' → no project:<name> label; store._project() always returns feature['repo']=self.repo (tool's flat repo), so loop._repo_for() hits str(feature.get('repo'))/store_kw['repo'] regardless of the loop's default_project → the card builds in the flat instance repo, not a project repo."
  },
  {
    "file": "loop.py",
    "line": 2205,
    "severity": "minor",
    "category": "removed-behavior",
    "claim": "On a multi-project board whose config declares no `default_project`, unlabeled (pre-#90 back-compat) features lose the fail-closed gate preflight entirely: `_ready_projects` drops the empty project name, so the instance gate is never smoked for them and the claim-loop's per-project hold skip never fires — work is dispatched with zero gate protection where the old code held ALL work on gate failure.",
    "evidence": "def _ready_projects(self, store):\n        names: list[str] = []\n        seen: set[str] = set()\n        for f in store.list_features(state=\"ready\"):\n            name = self._project_name(f)\n            if name and name not in seen:\n                seen.add(name)\n                names.append(name)\n        return names\n\n# ... and in _hold_ready_for_preflight:\n            name = self._project_name(f)\n            reason = self._preflight_state.get(name)\n            if not isinstance(reason, str):\n                continue  # this feature's project can run its gate (or hasn't been checked)\n\n# while OLD _spawn_ready did, before the claim loop:\n        if isinstance(self._preflight_state, str):\n            self._hold_ready_for_preflight()\n            return False",
    "verdict": "confirmed",
    "note": "Verified in loop.py: _project_name returns '' when default_project='' (multi-project, none named); `if name and ...` drops '' → _ready_projects=[] and _maybe_preflight's replay only adds keys already str in _preflight_state (none initially), so the '' gate is never smoked; _spawn_ready's `isinstance(self._preflight_state.get(pname), str)` and _hold_ready_for_preflight both no-op for ''. Diff minus side shows old `if isinstance(self._preflight_state, str): self._hold_ready_for_preflight(); return False` did hold-all. Claim accurate."
  },
  {
    "file": "loop.py",
    "line": 2200,
    "severity": "minor",
    "category": "removed-behavior",
    "claim": "The gate preflight smoke is no longer run at boot on an idle board — a deliberate per-project tradeoff in this PR, but still a removed-behavior change: OLD `_maybe_preflight` smoked on the first tick unconditionally (state starts `None`), so a broken `local_gate_cmd` was detected and logged loudly immediately; NEW only smokes projects surfaced by `_ready_projects` (ready work) plus already-failed keys, so a broken gate on an idle single-repo board is silent until the first card goes ready.",
    "evidence": "OLD:\n        if not self.preflight or not self.local_gate_cmd:\n            self._preflight_state = True\n            return\n        if self._preflight_state is True:\n            return\n        now = time.monotonic()\n        if self._preflight_state is not None and (now - self._last_preflight) < max(self.interval, 60.0):\n            return\n        self._last_preflight = now\n        await self._preflight()\n\nNEW:\n        if not self.preflight:\n            return\n        store = self._store()\n        names = list(self._ready_projects(store))\n        seen = set(names)\n        for name, st in self._preflight_state.items():\n            if isinstance(st, str) and name not in seen:\n                seen.add(name)\n                names.append(name)",
    "verdict": "confirmed",
    "note": "Verified against the diff minus/plus: OLD ran smoke whenever `self._preflight_state is not True` (fires immediately from None on tick 1, logging on failure); NEW runs only for names from _ready_projects (needs ready work) plus pre-existing str keys — on an idle single-repo board names=[] and _preflight_state={}, so nothing is smoked and a broken gate stays silent until a card goes ready. Note: this is a deliberate per-project tradeoff, but the removed-behavior claim as stated is accurate."
  },
  {
    "file": "loop.py",
    "line": 1565,
    "severity": "minor",
    "category": "cross-file",
    "claim": "Per-feature coder escalation is dead on boards whose flat config doesn't already enable it: _drive resolves coders = self._coders_for(feature) but switches on the instance-level self.escalation_on (computed once from the flat `coders` map), so a project entry declaring fast/smart never escalates through its own ladder when the instance coders map has fewer than two distinct delegates.",
    "evidence": "Drive hunk: \"+        coders = self._coders_for(feature)\" and \"coder_name = coders.get(tier, self.coder_name) if self.escalation_on else self.coder_name\" / \"tier = store.current_tier(fid) if self.escalation_on else \"\". Test pinning escalation_on to the flat map: \"assert loop.escalation_on is False  # no coders map → single-coder mode\". The PR's own _MULTI_CFG has flat \"coders\": {\"fast\": \"instance-coder\"} (one entry) with project \"board-plugin\" declaring {\"fast\", \"smart\"}.",
    "verdict": "confirmed",
    "note": "Verified: store.escalation_enabled(cfg) = len({v for v in cfg.get('coders',{}).values()})>1 — reads ONLY flat cfg['coders'], never the per-project map; self.escalation_on is set once from it. _drive computes coders=_coders_for(feature) but gates `tier = ... if self.escalation_on else ''` and coder_name selection on the instance flag, so a project ladder (fast+smart) with single-entry flat coders is never climbed. (The test-pin quote was not in the diff I read, but the code mechanism is directly verified.)"
  },
  {
    "file": "loop.py",
    "line": 850,
    "severity": "minor",
    "category": "cross-file",
    "claim": "`_all_repos()` does not actually enumerate 'every distinct repo root the board builds in': when any project declares a repo it omits the store default, while `_repo_for()` sends unlabeled (pre-#90) features to that same store-stamped default repo first — so the orphaned-worktree reap skips exactly the checkout those features build in on a mixed multi-project board.",
    "evidence": "`_all_repos` docstring says: \"Every distinct repo root the board builds in — one per project (#90)...\"; its body only falls back to the store default when the project set is empty: `if not seen: seen.setdefault(self._store_kw[\"repo\"], None)`. Meanwhile `_repo_for`'s fallback reads the feature-stamped repo first: \"str(feature.get(\"repo\") or \"\").strip() or str(self._project_cfg(feature).get(\"repo\") or \"\").strip() or self._store_kw[\"repo\"]\" — and the PR's own fixture stamps an unlabeled feature with the instance repo (e.g. `\"repo\": \"/instance/repo\"`). On a board with a `projects:` map whose default repo differs from the flat `repo:`, unlabeled features build in `/instance/repo`, which `_all_repos()` never yields, so `_sweep` never reaps their `feat-<id>` worktrees.",
    "verdict": "confirmed",
    "note": "Verified: _all_repos() collects only project-entry repos and adds self._store_kw['repo'] only `if not seen` — with any project map, the flat repo is omitted; store._project() returns feature['repo']=self.repo (loop store's flat repo) so _repo_for(unlabeled) returns that flat repo first; _sweep->_sweep_worktrees only iterates _all_repos(), so the unlabeled-feature checkout is never reaped. Claim accurate. (The 'fixture stamps an unlabeled feature with the instance repo' evidence is the code path store._project, not a specific new test; the mechanism holds regardless.)"
  },
  {
    "file": "loop.py",
    "line": 0,
    "severity": "minor",
    "category": "tests",
    "claim": "Per-feature project resolution was threaded into `_reconcile_orphan`, `_salvage_verified_candidate`, `_verify_merged_state`, `_maybe_rebase`, `_reconcile_prs`, and `_sweep`/`_all_repos`, but the new tests cover only the resolvers, the dispatch prompt, `_drive`, and preflight — none of these changed flows has a multi-project test proving repo/base/gate resolve from the feature's project.",
    "evidence": "The salvage path changes to \"repo = self._repo_for(f) / base = self._base_branch_for(f)\" (`@@ -771,8 +954,8 @@`), `_verify_merged_state` to \"if not self._local_gate_cmd_for(feature)\" and \"base = self._base_branch_for(feature)\", `_maybe_rebase` to \"base = self._base_branch_for(feature)\", and `_reconcile_orphan` to \"cwd=self._repo_for(feature)\" — while the added tests (`test_feature_resolves_repo_gate_and_coders_from_its_project`, `test_drive_builds_in_the_features_project_repo`, `test_preflight_isolation_a_fails_b_passes`, …) never exercise those call sites' project resolution.",
    "verdict": "confirmed",
    "note": "Verified: all six call sites are changed in the PR diff exactly as quoted; the added tests in the diff cover the resolvers (_repo_for/_base_branch_for/_coders_for/_gate_files_for), _build_prompt, _drive, _coder_solve_settings, and preflight — none call _reconcile_orphan/_salvage_verified_candidate/_verify_merged_state/_maybe_rebase/_reconcile_prs/_sweep. Test-coverage-gap claim accurate (negative claim verified against the added-test set in the diff)."
  },
  {
    "file": "loop.py",
    "line": 737,
    "severity": "nit",
    "category": "conventions",
    "claim": "The seven per-feature resolvers each hand-roll the same 'labeled project lookup, else feature-stamped value, else `_project_cfg`, else store default' fallback chain — `_repo_for` and `_base_branch_for` duplicate the labeled-branch that `_project_cfg(feature)` already centralizes for the other five, giving near-identical bodies that can drift independently.",
    "evidence": "`_base_branch_for`'s body is a line-for-line mirror of `_repo_for`'s: \"str(feature.get(\"base_branch\") or \"\").strip() or str(self._project_cfg(feature).get(\"base_branch\") or \"\").strip() or self._store_kw.get(\"base_branch\") or \"main\"\"; `_format_cmd_for`/`_gate_files_for`/`_repo_conventions_for`/`_coders_for` are all variants of `pc = self._project_cfg(feature); if \"<key>\" in pc: return ... ; return self.<instance_attr>`.",
    "verdict": "confirmed",
    "note": "Verified in loop.py: _repo_for and _base_branch_for each begin with the explicit-name branch (`name = str(feature.get('project')...); if name: ... from self._projects.get(name)`) before the feature-stamped→_project_cfg→store-default fallback, whereas _format_cmd_for/_gate_files_for/_repo_conventions_for/_coders_for only call _project_cfg(feature). The duplicated labeled-branch and near-identical bodies exist as described (a real style nit, not behavior-affecting)."
  }
]

---

1 finding(s) excluded from the verdict by in-diff confinement (file not among this PR's changed paths):

• __init__.py (major) — On a multi-repo board, features created through the board_create_feature tool build in the instance repo instead of a project repo: the tool's store_kw was not

---

1 finding(s) downgraded to uncertain: the code they quote as evidence does not appear in the file at the reviewed head, nor in this PR's patch for it. A finding that cannot be grounded does not gate a merge (issue #25) — it still stands for a human to judge.

• loop.py (nit) — quoted evidence not found at this head: pc = self._project_cfg(feature); if "<key>" in pc: return ... ; return self.<instance_attr>