feat(tools): board_register_project — register a repo at runtime (#167)

[mabry1985]

Closes #167.

An agent could clone a repo and gain filesystem reach (protoAgent's onboard_project,
#2555) and then stop dead. Onboarding writes only filesystem.projects, so without a
board entry no feature can be dispatched there — the agent got a repo it could read
and never one it could ship to. Every board-managed repo cost an operator a YAML edit
plus a member restart.

Observed live onboarding pr-reviewer-plugin to protoEngineer: repo cloned, venv built,
conventions drafted, then a hard stop with "I can't write the agent config directly."
Correct behaviour, and the feature failing to land.

project_board.projects had no runtime write path at all — a nested map, not a declared
settings: field, so the settings API refuses it (unknown setting: project_board.projects) and the console can't render it.

The tool

board_register_project(name, repo, base_branch, local_gate_cmd, repo_conventions) —
mirroring onboard_project's shape on purpose, including its refusal posture:

• Consent is the operator's onboarding space, not the tool's own. Refuses unless
  onboarding.enabled, and the repo must resolve under onboarding.root. Both sides
  resolve before comparing, so a .. escape or symlink can't slip past a string-prefix
  check. Registering is strictly narrower than the clone before it — the path is already
  on disk, already inside the declared space.
• Superset merge — a register can never drop a sibling. That's the protoAgent #2556
  shape: a replace-all write that silently dropped roots and still answered ok.
• Idempotent by name — re-registering updates in place.
• Reads the configured map, not the resolved one. resolve_projects synthesizes an
  implicit project from flat keys when no map is declared; writing that back would
  persist a default the operator never wrote, turning an additive register into a silent
  config rewrite.
• Writes through HOST.apply_settings (nested dicts — the dotted form is an HTTP-route
  convention expanded before the seam), never a server import.

It also nudges: registering without repo_conventions says so in the return value,
because omitting them is the single most common cause of a coder inventing the wrong
convention for a repo.

One trap worth recording

The module is project_registry.py, not register.py. Importing .register binds
the submodule as a package attribute over the package-level register() the host calls,
and plugin load dies with 'module' object is not callable. The existing
test_register_wires_routers_surface_and_tools caught it in the full suite while passing
in isolation; a note in the module records why the name is what it is.

Verification

736 passed, ruff check + ruff format --check clean. 13 new tests cover the bound
(outside-root refused by name, .. escape, non-checkout, missing dir, and an unset
root refusing rather than reading as unbounded), the configured-vs-synthesized read, and
the superset/idempotency invariants. Both bound tests were mutation-checked — they go red
with the containment check removed.

Left for a follow-up, since it's a consent question rather than a mechanism one: whether
onboard_project should call this automatically. A PM that can add board projects can
point coders at any repo inside the space, which is a larger grant than "clone and read"
— keeping them separate lets an operator enable reading without granting dispatch.

[protoreview]

QA panel review — FAIL

code-review-structural · head 44bc1d84f781 · formal

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

All seven findings verified as confirmed against the actual code; none are refuted, none are dropped. No prior requests, so no dispositions block. The verifier's re-reads settle the one panel disagreement (on #3 the cross-file "boot-time snapshot" reading wins; the "removed-behavior next-dispatch pickup" counter-premise is unsupported). All severities stand as graded.

Overall risk is high: the PR's header case (registering a name that already exists) silently replaces the operator's whole per-project entry, missing field drops and force-resetting base_branch, and registration is ineffective until a member restart because every in-process consumer resolved the map once at boot — while the flat-config default re-homes project-less creates on first write. Fix the replace-clobber in project_registry.py first (it breaks the advertised idempotent-in-place path and is the data-loss defect), then wire a re-read path for dispatch. The panel did not disagree on any kept finding; the one dispute the verifier resolved: finder #3's "removed-behavior assumed next-dispatch pickup" counter-premise is unsupported — the cross-file boot-time-capture reading is correct. Verification changed nothing structurally: all confirmed, with the only unverifiable link being the cross-repo HOST.apply_settings seam (whether the host re-invokes register() — nothing in this repo does), and the PR-head SHA 404'd so reads ran against default-branch files verified byte-for-byte against the diff. Coverage: both changed files (project_registry.py at 3 majors/3 minors/1 nit, its test file at 1 minor) were heavily read — no under-read file; no coverage gap beyond the cross-repo seam.

Findings

	Severity	Location	Finding	Verified
🟠	major	project_registry.py:171	Re-registering an existing project name on the tool's advertised 'idempotent by name: updates in place' path wholesale-replaces its entry with a shrink-wrapped…	confirmed
🟠	major	project_registry.py:181	On a board configured with flat keys and no projects: map (the shipped back-compat default, where _raw_projects() deliberately returns {}), the first regis…	confirmed
🟠	major	project_registry.py:195	The register writes only into host config, but every in-process consumer of the projects map (BoardLoop and the sibling board tools) resolved it ONCE from the …	confirmed
🟡	minor	tests/test_project_registry.py	No test exercises the tool at all: board_register_project's consent/refusal branches (onboarding enabled, root bound), entry construction, the HOST.apply_setti…	confirmed
🟡	minor	project_registry.py:175	The 'never DROP a sibling' (#2556) safety guard is a tautology that can never fire — merged = dict(existing) then merged[project] = entry guarantees the me…	confirmed
🟡	minor	project_registry.py:6	The module docstring falsely claims the packaging test catches the register-naming clobber: the clobber of project_board.register happens mid-call inside the t…	confirmed
⚪	nit	project_registry.py:48	Dead code left in: _ENTRY_FIELDS is defined but never referenced anywhere, and build_register_tool's cfg parameter is threaded from _board_tools(cfg) yet n…	confirmed

findings JSON (machine-readable)

[
  {
    "file": "project_registry.py",
    "line": 171,
    "severity": "major",
    "category": "removed-behavior",
    "claim": "Re-registering an existing project name on the tool's advertised 'idempotent by name: updates in place' path wholesale-replaces its entry with a shrink-wrapped dict built only from this invocation, silently dropping every operator-configured per-project field (repo_conventions, local_gate_cmd, coders, gate_files, format_cmd, env_passthrough, coder_solve_*) and force-resetting an omitted base_branch to 'main', because the superset guard compares project NAMES only, never entry fields (flagged by both removed-behavior and cross-file review).",
    "evidence": "        merged = dict(existing)\n        merged[project] = entry\n\nwith `entry` built as\n        entry = {\"repo\": str(repo_p), \"base_branch\": (base_branch or \"main\").strip()}\n        if local_gate_cmd.strip():\n            entry[\"local_gate_cmd\"] = local_gate_cmd.strip()\n        if repo_conventions.strip():\n            entry[\"repo_conventions\"] = repo_conventions.strip()\n\nThe superset guard `if not set(existing) <= set(merged):` compares project names only, so an entry carrying operator-written fields ({repo, base_branch, coders, gate_files, worktrees_root, format_cmd, env_passthrough, coder_solve_*}) passes it and is overwritten by the 2–4-key entry while the tool answers 'Updated board project …'.",
    "verdict": "confirmed",
    "note": "Read project_registry.py: `merged[project] = entry` replaces the entire old entry; `entry` only ever carries repo/base_branch (+optional gate/conventions); `base_branch: str = \"main\"` default clobbers an omitted operator value. Guard checks key-set only, so it cannot catch the field drop. Dropped fields match projects.py's _PROJECT_SETTING_KEYS + coder_solve_* operator surface."
  },
  {
    "file": "project_registry.py",
    "line": 181,
    "severity": "major",
    "category": "cross-file",
    "claim": "On a board configured with flat keys and no `projects:` map (the shipped back-compat default, where `_raw_projects()` deliberately returns {}), the first register writes a `projects:` map containing only the new entry, after which projects.py's `resolve_projects` stops synthesizing the implicit flat 'default' project and `default_project()` re-homes all project-less feature creates to the newly registered repo instead of the operator's configured repo — defeating the PR's own 'never drop a sibling' invariant, whose check only inspects the explicit map (flagged by correctness, cross-file, and removed-behavior review).",
    "evidence": "project_registry.py: `merged = dict(existing)` / `merged[project] = entry`; the write is `ok, messages = await asyncio.to_thread(HOST.apply_settings, {\"project_board\": {\"projects\": merged}})`. projects.py: `if isinstance(raw, dict) and raw: return {str(name): _resolve_project_entry(str(name), settings) for name, settings in raw.items()}` — once the map is non-empty the implicit flat project is no longer synthesized, and default_project() `return next(iter(projects)) if len(projects) == 1 else \"\"` falls to the sole new entry.",
    "verdict": "confirmed",
    "note": "Verified both halves verbatim: _raw_projects() returns {} for flat config (codified by test_raw_projects_does_not_synthesize_an_implicit_project); after the first write the `projects:` map is non-empty, so resolve_projects' `if isinstance(raw, dict) and raw:` branch stops synthesizing 'default' and default_project()'s `next(iter(projects)) if len(projects) == 1 else \"\"` returns the sole new project — project-less creates re-home to the new repo. Caveat: assumes the write succeeds (cross-repo apply_settings seam), which is the tool's own success path."
  },
  {
    "file": "project_registry.py",
    "line": 195,
    "severity": "major",
    "category": "cross-file",
    "claim": "The register writes only into host config, but every in-process consumer of the projects map (BoardLoop and the sibling board tools) resolved it ONCE from the boot-time plugin cfg, so a just-registered project is not dispatchable until a member restart — directly contradicting the tool's success message and the PR's stated purpose of eliminating the restart, and freshly-registered work finds the name absent (falls back to the base repo) in the stale snapshot.",
    "evidence": "The tool's success message \"The board picks it up on its next dispatch.{note}\" — while __init__.py's _board_tools captures the map at registration: \"projects = resolve_projects(cfg)\" and \"entry = projects.get(name) or {}\", and the runtime write goes only to \"HOST.apply_settings, {\"project_board\": {\"projects\": merged}}\". No path re-reads host config at dispatch, so the new project name resolves to {} in the stale snapshot and its store falls back to the base repo.",
    "verdict": "confirmed",
    "note": "Re-read all three consumers: _board_tools(cfg) computes `projects = resolve_projects(cfg)` once at registration; BoardLoop.__init__ sets `self._projects = resolve_projects(self.cfg)` once at construction; api._store_kw computes it once at router build. `_store_kw_for` does `entry = projects.get(name) or {}` -> unknown name keeps the base repo. The only write is to HOST.apply_settings (config), and the plugin has no re-read path, so the snapshot stays stale until restart. The only unverifiable link is whether the host re-invokes register() on apply_settings (not standard, and nothing in this repo does)."
  },
  {
    "file": "tests/test_project_registry.py",
    "line": 0,
    "severity": "minor",
    "category": "tests",
    "claim": "No test exercises the tool at all: board_register_project's consent/refusal branches (onboarding enabled, root bound), entry construction, the HOST.apply_settings write, and the Updated/Registered responses are all unexercised — the suite imports only the two pure helpers, so the feature's security core has no regression guard.",
    "evidence": "from project_registry import _raw_projects, _resolve_under",
    "verdict": "confirmed",
    "note": "Read tests/test_project_registry.py: the only import from project_registry is `from project_registry import _raw_projects, _resolve_under`; no test calls build_register_tool()/board_register_project() or touches the consent/refusal/apply_settings paths. Quote matches exactly."
  },
  {
    "file": "project_registry.py",
    "line": 175,
    "severity": "minor",
    "category": "conventions",
    "claim": "The 'never DROP a sibling' (#2556) safety guard is a tautology that can never fire — `merged = dict(existing)` then `merged[project] = entry` guarantees the merged key set is always a superset — so `if not set(existing) <= set(merged):` is dead code, and the parity test re-implements the same construction locally instead of invoking the tool, leaving the invariant with zero independent verification.",
    "evidence": "        merged = dict(existing)\n        merged[project] = entry\n\n        # A register must never DROP a sibling — the #2556 shape, where a replace-all\n        # write quietly removed roots and still answered ok.\n        if not set(existing) <= set(merged):\n            log.error(\"[project_board] refusing to apply: merge would drop an existing project\")",
    "verdict": "confirmed",
    "note": "Verified: `merged = dict(existing)` copies existing keys, `merged[project] = entry` can only add/overwrite one key, so `set(existing) <= set(merged)` is always True and the guard can never fire — dead code. The test `test_the_merge_is_a_superset_and_idempotent` rebuilds `merged = dict(existing); merged[adding] = {...}` locally (same tautology) and never invokes the tool."
  },
  {
    "file": "project_registry.py",
    "line": 6,
    "severity": "minor",
    "category": "conventions",
    "claim": "The module docstring falsely claims the packaging test catches the register-naming clobber: the clobber of project_board.register happens mid-call inside the test's single project_board.register(reg), so the test passes regardless and would not detect a rename to register.py — breakage only surfaces on a subsequent register call.",
    "evidence": "``tests/test_packaging.py::test_register_wires_routers_surface_and_tools`` catches it.",
    "verdict": "confirmed",
    "note": "Traced the flow: the clobbering import happens inside _board_tools(cfg) (called from register()), i.e. mid-flight within the test's single `project_board.register(reg)` call, after the function object was already bound to the running frame. The test asserts only that routers/surfaces/tools registered, all of which still happen — so a rename to register.py passes. test_packaging.py confirms the single call and assertions. Docstring's 'catches it' claim is false; 'module' object is not callable' would only surface on a second register() call."
  },
  {
    "file": "project_registry.py",
    "line": 48,
    "severity": "nit",
    "category": "conventions",
    "claim": "Dead code left in: `_ENTRY_FIELDS` is defined but never referenced anywhere, and build_register_tool's `cfg` parameter is threaded from _board_tools(cfg) yet never used in the body (the tool reads host config directly through `_host_onboarding()`/`_raw_projects()`).",
    "evidence": "#: Fields an operator writes by hand today, and the only ones this tool sets.\n_ENTRY_FIELDS = (\"repo\", \"base_branch\", \"local_gate_cmd\", \"repo_conventions\")\n\n\ndef _host_onboarding() -> tuple[bool, str]:",
    "verdict": "confirmed",
    "note": "In project_registry.py, `_ENTRY_FIELDS` appears only at its definition (no other reference in the module); `def build_register_tool(cfg: dict):` never references `cfg` in its body (body only imports langchain tool, defines board_register_project, returns it). __init__.py calls `build_register_tool(cfg)` so the param is threaded but unused. 'Anywhere' verified within the module; tests import only _raw_projects/_resolve_under, so no test-side use."
  }
]