feat(board): delete the Plane adapter — the migration is complete - #521
Merged
Conversation
The board has run on Forgejo since #516, and #519 removed the last importer. This deletes what was left: adapters/plane (382 lines), its 1,068 lines of tests, PlaneSettings, Settings.plane, and plane_token(). Net -1453 lines. board_backend narrows to Literal["forgejo"]. A config still naming the retired backend would otherwise get "Input should be 'forgejo'", which reads as a typo — it was a real backend until 2026-08-18, so a model_validator answers the question the operator is actually asking, and the factory carries the same message for duck-typed settings that never reach pydantic. dependency_check trades its Plane service row for a Forgejo one. Deleting it outright would leave the report blind to the single service whose absence stops everything; /api/v1/version needs no auth and answers version and reachability at once. upstream_latest stays None with a comment: Forgejo publishes on Codeberg, not the GitHub releases API the other rows use, and inventing a second fetcher for one row is not this change. --create-plane-tasks becomes --create-board-tasks; it always went through make_board_client and was never Plane-specific, only Plane-named. The board-seam ratchet is retargeted rather than retired. The reason a caller must not name a concrete client never depended on which client it was, so it now guards ForgejoClient, with the setup wizard as the single allowlisted direct constructor (it validates credentials before a Settings object exists). A new test asserts the adapter package is gone and that nothing imports it — checking importers, not every mention, because several docstrings narrate the migration and that history is why the seam exists. Fallout, and one real find: 30 unit tests broke, all of them fixtures describing a Plane-shaped settings object. Fixing them surfaced a gap — `settings.forgejo` raised AttributeError on a settings object simply lacking the attribute, instead of the explained "no `forgejo:` settings block" error sitting three lines below. Both factory paths use getattr now, so every shape of missing config fails the same explained way. Verified: tests/unit 8623 passed 0 failed (twice, cold cache); the non-unit suites keep exactly their 6 known pre-existing failures, none added; ruff clean, ty at the 13-diagnostic baseline, custodian audit clean. Naming residue left deliberately, as follow-up: `_emit_plane_task`, `PlaneClientProtocol`, and several `plane_client` parameter names are all board-generic already. Renaming them is cosmetic churn that would triple this diff without changing behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Council review: LGTM (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council unanimous LGTM (claude_code/claude-opus-5 (correctness), claude_code/claude-opus-4-8 (security-capability), claude_code/claude-opus-4-7 (convergence-operational)). |
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
Owner
Author
Council concerns (attributed by member):
|
CI's `custodian-doctor --strict` failed on the deletion PR:
WARN: audit.exclude_paths.D11: glob 'src/operations_center/adapters/plane/**'
matches no files (stale exclusion?)
Correct, and exactly the residue the deletion should have swept. D11 is the
duplicate-logic detector; the Plane adapter was excluded there because it
shared an API-wrapper shape with the other adapters. With the package gone the
exclusion protects nothing and only hides that the list has drifted from the
tree.
Worth recording the local/CI gap: the pinned custodian in .venv reports this as
a WARN and exits 0, while CI installs `.[dev]` fresh and its --strict treats
the same warning as fatal. This is the one gate where running the exact CI
command locally still gave a green that CI would not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
1 similar comment
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
Owner
Author
Council concerns (attributed by member):
|
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
2 similar comments
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
Owner
Author
|
Council review: CONCERNS (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council concerns (attributed by member):
|
Owner
Author
The automated fix passes exhausted the Self-Heal Ladder (reached L2/2) without changing the branch; a fresh self-review on the same PR head still finds concerns. Further autonomous retries would repeat without progress. Latest concerns: Council concerns (attributed by member):
|
Council correctness review on the Plane deletion, and it is a real bug I
introduced.
The Forgejo row I added to replace the Plane service row called
`response.json()` with no guard:
if response.status_code >= 400:
return None, False
payload = response.json()
A 200 carrying something that is not JSON — a reverse-proxy error page, a login
interstitial, a captive portal — raises `json.JSONDecodeError` out of a
function whose entire purpose is to *report* health. One unreachable board
would take down the whole dependency report rather than showing up as the
unhealthy row it is.
Worth naming: the Plane probe I replaced only read `status_code` and never
parsed a body, so this failure mode did not exist before this change
introduced it.
Guarded now, and it answers unhealthy rather than healthy: something responding
on that URL that is not Forgejo's API means the fleet cannot use it as a board,
which is exactly what the report should say. A non-object payload is treated
the same way. The note widens to "unreachable, or not the API" so it stays true
in both cases.
Five tests cover the probe: a good version, a non-JSON body, a non-object
payload, an HTTP error, and a missing forgejo block.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Council review: LGTM (cross-family panel, guardrail paths: scripts/operations-center.sh)
Council unanimous LGTM (claude_code/claude-opus-5 (correctness), claude_code/claude-opus-4-8 (security-capability), claude_code/claude-opus-4-7 (convergence-operational)). |
This was referenced Aug 19, 2026
ProtocolWarden
added a commit
that referenced
this pull request
Aug 19, 2026
…res (#523) test_the_retired_backend_is_actually_gone failed in a working checkout while passing in CI, and the reason is worth writing down rather than just clearing. git removes tracked files, not directories that still hold an untracked __pycache__. So after #521, src/operations_center/adapters/plane/ survived as an EMPTY directory — and an empty directory is a PEP 420 namespace package. `import operations_center.adapters.plane` still succeeded, returning a module whose __file__ is None. Any `except ImportError:` fallback would have taken the wrong branch without saying so. CI never reproduces this (fresh checkout, nothing left behind). Anyone who pulls the deletion with a populated __pycache__ does. The assertion now says what happened, why an empty directory is not harmless, and prints the exact rmdir that fixes it. No behaviour change — the test already caught the condition; it just could not explain it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 19, 2026
…524) The Plane -> Forgejo migration ran off direct operator requests and was never tracked in the backlog, so nothing in `.console/` tells a future session it happened or why the codebase looks the way it does now. Adds the Done entry (with the recon finding that reshaped it: Plane was never live on this host, so the planned drain was vacuous) and the two items it left open: * **CI runs ~1,830 fewer tests than exist.** No longer hypothetical — #521 merged a NEW failing test because `tests/test_dependency_check.py` is in no CI job, and main was red until #522. #513 was the same story a week earlier. Recorded with the 6 currently-failing tests that make switching the gate on a decision rather than a chore, and the sensible order: fix those 6, then add the suites in the PR that proves them green. * **`audit` on Forgejo Actions** — the last line of the PR-adapter spec, blocked on an operator installing a runner. Everything upstream is done. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 19, 2026
… it (#525) * ci: gate the ~1,830 tests that had no job, and fix the 6 that blocked it Two dedicated jobs already exist for single files that rotted unnoticed (tests/test_pr_review_watcher.py, tests/integration/reviewer) — each added AFTER that suite had drifted. This generalises the fix instead of waiting for the next one. What the gap cost, concretely: #509 shipped a regression that broke tests/maintenance/, found a week later by hand in #513. #521 merged a brand new failing test in tests/test_dependency_check.py — CI green, reviewer verdict SUCCESS, main red until #522. tests/test_proposer_entrypoint.py had been failing since the board-seam migration weeks earlier and nothing noticed. Switching the gate on required fixing 6 failures, and they were not what they looked like: FOUR WERE BAD MOCKS, NOT BUGS. They raised FileNotFoundError("message") — with no errno. pathlib's predicates swallow only ignorable errnos (ENOENT, ENOTDIR, EBADF, ELOOP), so an errno-less error escapes is_file()/is_dir(), which a real vanished path never does. Verified identically on 3.11 and 3.12: deleting a directory mid-scan makes glob() return []. The mocks were testing CPython's exception plumbing, not the collectors' guards. They now raise the way the OS does. My first attempt "fixed" production by guarding the walk against deletion — defending against something that cannot happen — and is reverted. ONE WAS A REAL GAP THE BAD MOCK WAS HIDING. EACCES and EIO are NOT ignorable, so a log directory that becomes unreadable (rather than deleted) does raise straight out of the walk and takes the collector with it. latest_matching_file now guards the walk for OSError, with the distinction written down, because "return None when nothing is discoverable" is its contract. ONE WAS A REAL PRODUCTION BUG. custodian_sweep._emit answered zero-findings before its dry-run branch could run, so that branch was unreachable and a dry run reported "skipped-zero-findings" — the past tense, for work it had not done. Two tests in the same file asserted opposite labels for the same call; the module's own would-comment / would-create convention and the shadowed branch both say `would-` was intended. ONE WAS A STALE PATCH TARGET: proposer.main.PlaneClient stopped existing at the board migration. Patched at the seam now. Verified: tests/unit 8623 passed, the new job's exact command 1830 passed, both 0 failed. ruff clean, ty at the 13-diagnostic baseline, custodian doctor OK, audit clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(observer): walk dependency reports with iterdir, not glob The new test-rest job went red on its first CI run — which is the job doing its job. Three failures, all in dependency_drift, none reproducible on 3.12: CPython 3.11's glob() stats every matched path through exists(); 3.12's does not. Two of the tests counted Path.stat calls to prove the collector does not re-stat after discovery — a fair question, but the counter was also counting the interpreter's probing. The third needed one unreadable run directory to be skipped while the rest are still read. My first attempt made it worse: wrapping the walk in list(glob(...)) meant a single bad entry aborted discovery, turning "skip run1, use run2" into not_available. glob() is a generator, so the first error closes it — per-entry recovery inside it is not possible at all. _latest_dependency_report now walks with iterdir(), which stats nothing: * one unreadable entry is skipped, not fatal (EACCES/EIO are NOT among the errnos pathlib swallows, unlike ENOENT/ENOTDIR/EBADF/ELOOP) * the counting assertions see only the collector's own stat calls, on either interpreter, so the test-side glob patching I added is removed as both unnecessary and ineffective — it still called the real glob() Verified on both: 3.12 locally 1830 passed, 3.11 in a python:3.11-slim container 1828 passed with the 3 fixed. The container also reports 2 failures CI does not (test_resolve_repos_root_falls_back_to_checkout_layout, test_loader_reads_latest_snapshot_with_bounded_history) — it mounts a flat worktree with no sibling checkouts, which is what those two probe. CI is the authority for CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items (#529) half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The board has run on Forgejo since #516, and #519 removed the last importer. This deletes what was left.
Net −1453 lines:
adapters/plane(382), its tests (1,068),PlaneSettings,Settings.plane,plane_token().board_backendnarrows, and explains itselfLiteral["forgejo"]. A config still naming the retired backend would otherwise getInput should be 'forgejo', which reads as a typo — it was a real backend until 2026-08-18, so amodel_validatoranswers the question the operator is actually asking. The factory carries the same message for duck-typed settings that never reach pydantic.dependency_checkgets a Forgejo row, not a holeDeleting the Plane service row outright would leave the report blind to the single service whose absence stops everything.
/api/v1/versionneeds no auth and answers version and reachability at once.upstream_lateststaysNonewith a comment rather than a guess: Forgejo publishes on Codeberg, not the GitHub releases API the other rows use, and inventing a second fetcher for one row isn't this change.--create-plane-tasks→--create-board-tasks. It always went throughmake_board_clientand was never Plane-specific, only Plane-named.The ratchet is retargeted, not retired
The reason a caller must not name a concrete client never depended on which client it was. It now guards
ForgejoClient, with the setup wizard as the single allowlisted direct constructor (it validates credentials before aSettingsobject exists — the same reason it was allowlisted for Plane).A new test asserts the package is gone and that nothing imports it — importers, not every mention, because several docstrings narrate the migration and that history is why the seam exists.
Fallout, and one real find
30 unit tests broke, all of them fixtures describing a Plane-shaped settings object. Fixing them surfaced a genuine gap:
settings.forgejoraisedAttributeErroron a settings object simply lacking the attribute, instead of the explained "noforgejo:settings block" error sitting three lines below it. Both factory paths usegetattrnow, so every shape of missing config fails the same explained way.Verification
tests/unitruff/ty/ custodian auditLeft deliberately, as follow-up
_emit_plane_task,PlaneClientProtocol, and severalplane_clientparameter names are all board-generic already. Renaming them is cosmetic churn that would triple this diff without changing behaviour.🤖 Generated with Claude Code