ENG-858 - An extension that declares a subject must satisfy the subject read-side at load, not at request time - #281
Conversation
There was a problem hiding this comment.
Verification (round 1)
Verdict: PASS. All six acceptance criteria are satisfied and CI is green at bed65b1.
- AC1 — pass.
ExtensionSubjectContractError(ExtensionLoadError)added inbackend/druks/extensions/exceptions.py;Extension.subject_classes()inbackend/druks/extensions/base.pyraises it whensubject_class.list_summaries.__func__resolves to theSubject/StoredSubjectplatform stub. Covered by dedicated tests for both subject families. - AC2 — pass. Error message includes extension name, subject class name,
list_summaries(), and an implementation direction; asserted by tests. - AC3 — pass. Both
loader.load(app)andload_extension()rundiscover()→subject_classes()directly from the loader, ahead of the author-overridableExtension.load(app)/get_routers(). A dedicated test proves an override ofload()cannot bypass the gate. - AC4 — pass. A concrete inherited
list_summaries()(with a body that would fail if actually called) passes without executing it, proving identity-only inspection. - AC5 — pass. The
transcriptscollision now raises the same typed error (was bareTypeError);load_extension()'s docstring documents the full exception taxonomy including the new type. - AC6 — pass.
docs/writing-an-extension.mdand the scaffoldingmodels.py-tplboth state thelist_summaries()requirement and its load-time consequence.
Verification commands:
uv run ruff check backend/uv run ruff format --check backend/uv run pytest backend/— pass, via CI check "On Pull Request Backend / checks" (SUCCESS atbed65b1).npm --prefix frontend run lint/build/test— not_run; no frontend check registered for this backend-only diff, consistent with the plan's stated scope.
No blocking findings. Open findings: none.
Code review
The clean-room code-review lens read every changed file end-to-end and found the diff otherwise well-scoped (no out-of-scope files touched) and idiomatic. One finding:
Medium — backend/druks/extensions/base.py:212: subject_class.list_summaries.__func__ assumes the method is always accessed as a bound classmethod. An author who overrides list_summaries without @classmethod gets a raw AttributeError at load instead of the intended actionable ExtensionSubjectContractError — the load-fast guarantee still holds, but the actionability goal this PR is built around doesn't, for that adjacent mistake. Left as an inline comment with a suggested fix shape.
No other findings (reuse, test shape, foot-guns, secrets, naming — all clean).
Filed as follow-up: ENG-865 (child of ENG-858), since it's medium severity and out of scope for this round.
Validate each subject a workflow declares in Extension.subject_classes(): reject one whose list_summaries() resolves to the Subject/StoredSubject platform stub, inspecting method identity without calling it. Both loader entry points (full boot and app-less load) discover then validate directly, so an override of load()/get_routers() cannot bypass the gate. The failure and the reserved-transcripts collision now share one ExtensionSubjectContractError under ExtensionLoadError. Docs and the extension model template state the contract and its load-failure consequence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
793d1df to
27a8e25
Compare
Linear ticket: ENG-858
The change
A workflow-declared subject must satisfy the read-side contract the platform calls on it.
subjects()now checks thatlist_summaries()resolves to a real implementation, not the platform stub — inspected by method identity, never called, so a concrete inherited override passes. A violation raisesExtensionSubjectContractErrorat load, naming the extension, the subject, and the method. The reserved-transcriptscollision joins the same typed family instead of raising a bareTypeError.Both load paths gate:
load_extension()(app-less) validates after capability discovery and preserves the typed failure rather than folding it intoExtensionImportError; full API boot validates before anything mounts. A subject that would 500 the board on first click now stops the extension from loading instead.Getting the full-boot gate airtight exposed a misplaced seam, fixed here rather than worked around:
Extension.load(app)bundled discovery with mounting, and its body was platform policy — the/api/<name>prefix that stops extensions shadowing each other, the identity gate on every route, frontend serving. An extension hook must not be able to override any of that. The method is gone; the loader owns the boot pipeline in one visible sequence —discover(),subjects(),mount()— withmount(app, extension, modules)as a loader function. Extensions customize through the hooks meant for it,discover()andget_routers(), which stay gated either way. This also removes the double discovery the loader-level gate would otherwise force, and it shrinks the extension surface: no first-party extension overrodeload(), and the author guide never documented it.Acceptance criteria
ExtensionSubjectContractErrorwhen a workflow-declaredSubjectorStoredSubjectresolveslist_summaries()to the platform stub, on both load paths.list_summaries(), and directs the author to implement it.list_summaries()is declared directly or inherited from a concrete parent, and validation never executes the method.ExtensionLoadErrorfamily, andload_extension()documents every concrete failure family it can raise.list_summaries()requirement and that a violation stops loading.