gate-57: the groupfolder-storage-backend capability is inert end-to-end — Task 5 was ticked but never done

[rubenvdlinde]

What the gate found

The full-scope workflow_dispatch run reports:

[gate-57] orphaned-write-capability: FAIL — 1 orphaned write-capability method(s)
lib/Service/DashboardService.php:3189 method=writeDashboardContent rule=orphaned-write-capability class=DashboardService

This is a true positive, and the real scope is much larger than one method.

Measured scope: no producer and no consumer

symbol	callers outside its own definition
DashboardService::writeDashboardContent()	0
DashboardService::readDashboardContent()	0
DashboardService::deleteDashboardContent()	0
DashboardContentStorageFactory::getStorage()	0 (only the three methods above)
Dashboard::setContent()	only DbContentStorage::write() (unreachable, per above) and MigrateStorageToGroupFolder (which sets it to null)

Consequences that follow from that table:

• Nothing in the application ever populates oc_launchpad_dashboards.content. Widgets live in WidgetPlacement rows. Dashboard::jsonSerialize() emits 'content' => $this->decodeContent(), which is therefore always null.
• launchpad.content_storage is a no-op setting. An admin can switch it to groupfolder via the admin UI, launchpad:storage:toggle-backend, or the setup wizard, and nothing changes — content was never routed through the factory at either end.
• launchpad:storage:migrate-to-groupfolder migrates nothing. It reads $dashboard->getContent() (always null), decodes that, and writes the result to the GroupFolder. With --prune-source it then nulls a column that was already null.

Root cause: a phantom-ticked task

openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md line 9:

- [x] Task 5: Wire `DashboardContentStorageFactory` into `DashboardService` so
      `get/create/update/delete` route through the active backend; catch storage
      exceptions and rethrow with user-friendly messages

Task 5 is ticked. The three facade methods were written; the wiring named in the task was not. Tasks 1–4 and 6–12 did land, which is why the capability looks complete from every angle except the one that matters.

openspec/specs/groupfolder-storage-backend/spec.md carries status: done and REQ-GFSB-001 states that DashboardService::getDashboard() MUST delegate to getStorage()->read(). There is no getDashboard() method anywhere in lib/ — the spec's read scenario names a method that does not exist.

Why this is not being fixed unilaterally

Completing Task 5 requires answering a genuine architectural question that the spec and the code currently answer differently:

• The spec treats the content blob as the source of truth for a dashboard's widgets/layout (REQ-GFSB-002: "content MUST be read from and written to the content field").
• The code treats WidgetPlacement rows as the source of truth, and has done since long before this capability existed.

Wiring the blob in as a second source of truth invites divergence between the two. Wiring it in as a projection (write-through for human-readable admin inspection, reads still from placements) contradicts REQ-GFSB-001's read scenario. Picking between those is a product/architecture decision, not a mechanical gate fix, so it belongs in an ADR rather than in a quality-sweep PR.

Suggested resolution paths

1. Complete the capability — decide the source-of-truth question in an ADR, add content to the create/update API contract, wire get/create/update/delete through the factory, and land the deferred Task 13 Playwright coverage once the dev stack ships groupfolders.
2. Retire the capability — remove the storage layer, the factory, the admin setting, the two CLI commands, and the content column, and archive the spec as withdrawn.

Either way the archived tasks.md Task 5 tick and the spec's status: done should be corrected, because right now the project record asserts a wiring that does not exist.

Until this is decided, gate-57 stays red on development and is the only non-coverage gate finding left in this repo.

[rubenvdlinde]

Re-measured today on a full-scope workflow_dispatch run against development (31459702510), tip 943d1ec-era:

[gate-19] e2e-coverage:               FAIL — 249 scenario(s) missing @e2e
[gate-25] contract-coverage:          FAIL — 15 new public endpoint(s) missing a contract test
[gate-26] visual-coverage:            FAIL — 1 new page component(s) missing a visual baseline
[gate-57] orphaned-write-capability:  FAIL — 1 orphaned write-capability method(s)

53 PASS, 7 NOT APPLICABLE, 57 of 57 applicable gates ran, no gate skipped for a wiring/helper reason. Gates 19, 25 and 26 are the coverage gates. This issue is still the only non-coverage finding in the repo, and it is unchanged — the source-of-truth question between the content blob (spec) and WidgetPlacement rows (code) has to be answered before anything mechanical can be done, so it stays parked here rather than being decided in a quality sweep.

[rubenvdlinde]

Three facts that change the shape of this decision — posted so it is made on accurate ground, not to make it

I re-measured this finding against origin/development (b2000b4) while clearing launchpad's coverage gates. I am not proposing a resolution — this is a product call. But three things I found are not in the issue, and each one moves the decision.

1. deleteDashboardContent's own docblock claims a caller that does not exist

/**
 * Delete dashboard content from the active storage backend (REQ-GFSB-001).
 *
 * Called by the cascade delete path to clean up storage alongside the
 * entity row. When the factory is absent this is a no-op.
 */
public function deleteDashboardContent(string $uuid): void

There is a cascade delete path — DashboardService::deleteDashboard() with $cascade = true, which removes the subtree via DashboardTreeService::deleteSubtree() (REQ-DASH-030). It does not call this method:

$ grep -rnE "\b(write|read|delete)DashboardContent\b" lib/ src/ appinfo/ tests/
lib/Service/DashboardService.php:3161:    public function readDashboardContent(...)
lib/Service/DashboardService.php:3189:    public function writeDashboardContent(...)
lib/Service/DashboardService.php:3214:    public function deleteDashboardContent(...)

Definitions only — no callers anywhere, and no tests either. So this is not simply "a capability nobody wired". It is a capability that documents a wiring it does not have, which is how a reader (or a future agent) concludes the cascade path is cleaning up storage when it is not. Whatever is decided, that docblock should not survive it.

I re-ran this with word boundaries deliberately. A substring collision in exactly this kind of grep (ChecklistService matching inside InspectionChecklistService) produced three false "no consumer" verdicts elsewhere tonight. These three names have no collisions.

2. The @spec anchors point at a path that no longer exists

All three methods carry:

@spec openspec/changes/groupfolder-storage-backend/tasks.md#task-5

That directory is gone — the change was archived to openspec/changes/archive/2026-06-14-groupfolder-storage-backend. Meanwhile the canonical spec is right there at openspec/specs/groupfolder-storage-backend/ and is the source of 35 of this repo's gate-19 findings. So the annotations point into an archive path while the live spec sits next door unreferenced. (The anchor resolver follows archived directories, which is why this produces no gate finding — it is silent debt, not a caught one.)

3. It fails open, silently, when the factory is absent

Each method begins:

if ($this->contentStorageFactory === null) {
    return;   // read: `return [];`
}

and the dependency is declared private readonly ?DashboardContentStorageFactory $contentStorageFactory = null. I found no explicit registration for it in lib/AppInfo/Application.php. I have not established whether Nextcloud's autowiring supplies it anyway for a nullable-with-default parameter — that is worth checking before anyone concludes either way.

It matters because of the direction the failure takes: if the factory is ever null in production, writeDashboardContent() returns success having written nothing, and readDashboardContent() returns [] — indistinguishable from "this dashboard has no content". The docblock calls this "e.g. in legacy test doubles", but the guard does not know it is in a test. If the capability is wired up rather than removed, this guard should become explicit — throw, or log — before it goes anywhere near real data.

What this does not change

The core of the issue stands: no producer, no consumer, and the spec/code disagreement about whether the content blob or the WidgetPlacement rows are the source of truth is real and is the actual question. The gate finding is correct.

What the three points above add is that the "remove it" option is cheaper than it looks (nothing calls it, nothing tests it, and its only documentation of a caller is inaccurate), while the "wire it up" option is more work than it looks (the fail-open guard and the source-of-truth contradiction both have to be settled first, and the @spec anchors repointed at openspec/specs/).

For the record, so nobody has to re-derive it: this finding is the only thing keeping launchpad off all-gates-green. Gates 19, 25 and 26 are being cleared separately; gate-57 is this. It has not been worked around — no annotation, no deletion.

[rubenvdlinde]

Path 2 (retire) landed in #100 — merged as 19e9b490

Leaving this open deliberately: the capability is retired, but the setup-wizard follow-up is not done, and it is tracked here.

What landed

lib/Service/DashboardContentStorage/ (interface, both backends, three exceptions), DashboardContentStorageFactory, the three DashboardService facade methods and the factory constructor parameter, DashboardApiController::storageUnavailableResponse(), DashboardMapper::findAll(), both CLI commands and their <command> registrations, the $content/$locale properties with decodeContent() and their jsonSerialize() keys, and the five PHPUnit classes covering the removed tree. −3004 lines.

Version002009Date20260811000000 drops the content and locale columns, hasColumn-guarded and idempotent. No data lost — the only writer was reachable solely through the orphaned facade methods, so both columns were NULL on every row of every install.

Version002001Date20260603000000 and DashboardTableBuilder::addContentStorageColumns() are kept on purpose: a shipped migration is part of the version ledger, and deleting it would desynchronise instances that have already run it.

The spec is marked withdrawn rather than deleted. That also retires its 16 @e2e exclude markers, whose reasons cited DbContentStorageTest.php — a file #100 deletes — which would otherwise have left sixteen exemptions resting on a class that no longer exists.

Both phantom ticks in the project record are corrected: Task 5 of the archived change is now - [ ] with a note explaining it was never done.

Verified

gate-57  1 -> 0   on the MERGED development tree (19e9b490), not on the branch
                  plant control: a tracked post* write method with no caller,
                  0 -> 1 naming exactly the plant -> 0
                  (post* deliberately — WRITE_VERB_PREFIXES excludes delete*)
PHPUnit  1570 tests, 4032 assertions, 0 failures
phpcs    0 errors, 468 warnings, 211 files — identical to the pre-change baseline

⚠️ Reproducing gate-57 locally: its checker takes a file list, not a directory. check_orphaned_write_capability.py . prints nothing and exits 0 whatever the tree contains — which reads exactly like a clean repo.

What remains here — Task 8

The reader is gone; the writer is not. Still present and now writing a setting nothing reads:

• launchpad.content_storage and AdminSettingKey::CONTENT_STORAGE
• SetupWizardService::{getContentStorage,setContentStorage,hasGroupfolderApp}
• AdminController::setWizardStorage and its route
• setup-wizard step 2

These belong to the setup-wizard capability, so retiring them edits a different spec and deserves its own review rather than being folded into a gate sweep. Note the setting was already inert before #100 — nothing read it, because the readers were orphaned — so removing the reader did not make this worse.

⚠️ Whoever takes it: tools/spec-annotations-allowlist.txt carries entries for those three SetupWizardService methods and for AdminController::setWizardStorage; they will need removing too.