feat(authoring) (1/5): draft domain model, CRUD and structure-layer validation

[zjncs]

Layer (a) of the authoring platform split agreed in #934 (replacing the monolithic #886; this PR contains its first slice).

What this layer adds

• Domain model: SkillDraft (revisions, content digest, optimistic concurrency, validated/submitted bookkeeping), DraftFile (object-storage backed, sha256), ValidationRun / ValidationEvent / ValidationFinding (layered findings with severity, status, locations)
• Migration: V69__authoring_platform.sql — six tables. Note: PR feat(skill): add avatars and namespace invitations #947 also claims V69; whichever lands second renumbers and reruns the migration tests per the review discussion.
• Draft CRUD REST API (/api/v1/authoring, /api/web/authoring): create draft seeded with a SKILL.md scaffold, list/get/delete, file save (text or base64, with expectedRevision conflict check), file read/delete — every draft-scoped endpoint enforces owner-or-SUPER_ADMIN
• STRUCTURE validation layer: package layout, SKILL.md frontmatter rules, file limits — executed by the run orchestrator, reported as findings with machine-applicable fix suggestions (old/new content patches with sha256 guards)
• Run lifecycle: start/cancel/get/list, events (SSE stream + polling cursor), findings list; maintenance task sweeps stale runs to TIMED_OUT

Deliberately out of scope (next layers, tracked in #934): runtime binding + MCP probing + security policies (b, dedicated security review), fix-apply + publish integration (c), web workbench (d), docs + example skill (e).

Verification

• skillhub-domain: 609 tests, 0 failures
• AuthoringDraftFlowIntegrationTest (Testcontainers PostgreSQL): scaffold → broken frontmatter → FAILED with a fixable FRONTMATTER_FIELD_MISSING finding → repair → SUCCEEDED; plus the optimistic-concurrency conflict path
• SkillAuthoringControllerAuthorizationTest: owner / SUPER_ADMIN / authenticated non-owner on both route prefixes for the file and run listing endpoints
• FlywayMigrationGuardrailTest green with the V69 numbering (contiguous on current main)
• Backend suite on this head runs as a visible check via .github/workflows/backend-tests.yml (also runs on the fork for every push of the authoring branches — see the Actions tab of zjncs/skillhub)
• web/src/api/generated/schema.d.ts regenerated from this branch's live OpenAPI surface

The structure rules intentionally share SkillPackagePolicy with the publish-time validator so draft-time and publish-time rules cannot drift.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[FenjuFu]

Thanks for splitting the work. I found two authorization gaps in layer (a): SkillAuthoringController.listFiles and listRuns accept only draftId and query without checking draft ownership. An authenticated non-owner can read another draft’s file metadata and validation-run metadata/summary. The separately protected file-content endpoint is not affected by this finding.

Please enforce the same owner-or-SUPER_ADMIN policy before both queries, and add two-user HTTP tests covering both /api/v1/authoring and /api/web/authoring, including owner/admin success cases.

Please also rebase and use the next free Flyway version (V69 at review time), regenerate web/src/api/generated/schema.d.ts, and run the backend integration tests. The visible green checks currently cover DCO/CLA only.

[zjncs]

Thanks @FenjuFu — all four items are addressed in 56531d2:

1. Authorization gaps. listFiles and listRuns now resolve the draft through getOwnedDraft first, enforcing the same owner-or-SUPER_ADMIN policy as every other draft endpoint.

2. Two-user HTTP tests. New SkillAuthoringControllerAuthorizationTest (MockMvc, Testcontainers PostgreSQL) covers both endpoints on both route prefixes (/api/v1/authoring and /api/web/authoring): owner success, SUPER_ADMIN success, and the authenticated non-owner rejection (403). 4/4 green.

3. Migration + rebase. Rebased on the latest main; the migration is now V69__authoring_platform.sql (past promotion V66/V67 and system_auth_settings V68).

4. Schema + tests. web/src/api/generated/schema.d.ts regenerated from this branch's live OpenAPI surface (/v3/api-docs of the running branch build; includes upstream API drift since the last generation plus the authoring endpoints). Backend suite run: mvnw clean test — 1073 tests, 0 failures, 0 errors (1 skip = the docker adapter test without a docker daemon). The stack above (layers b–e on the fork, linked in #934) is rebased onto this fix with the same verification per layer.

Happy to adjust anything else.

[FenjuFu]

Thanks — I rechecked 56531d2. Both list endpoints now call getOwnedDraft, the MockMvc/PostgreSQL regression covers owner, SUPER_ADMIN and authenticated non-owner on both route prefixes, the migration is now V69, and the generated API schema is committed. The previous source-level findings are addressed.

Two remaining integration points: the current GitHub checks still only show DCO/CLA, so please expose or run the backend authorization/draft-flow checks on this head; the 1073-test result is currently your local report. Also #947 introduces a different V69 migration. Coordinate the merge order and renumber/rebase the branch that lands second, then rerun migration tests. Please update the PR body, which still says V68. #934 should stay open for layers (b)–(e), including their dedicated security review.

[zjncs]

Both integration points are addressed in 8a69827:

Backend checks on the head. .github/workflows/backend-tests.yml now runs ./mvnw -B test (Temurin 21, Maven cache, Testcontainers on the hosted runner) for pull requests and the authoring branches. The fork ran it on this exact head — green run: 1073 tests, 0 failures, 0 errors — and the workflow is part of this PR, so the check will appear on upstream PRs too (first run on a fork PR needs a maintainer's workflow approval). Happy to drop or rework the workflow if you'd rather arrange CI differently.

V69 coordination with #947. #947 claims V69–V71 (skill_avatars / namespace_invitation / staged uploads). I tried renumbering past it, but FlywayMigrationGuardrailTest enforces contiguity against current main (…V68), so a gap fails the build. This branch therefore stays at V69, contiguous on main (guardrail green, migration tests rerun). Per your instruction the branch that lands second renumbers: if #947 merges first, we rebase onto main, renumber to the next free version, and rerun the migration tests; if this lands first, #947 does the same. Noted in the PR description.

PR body updated accordingly (V69, coordination note, CI). The stacked layers (b)–(e) on the fork are rebased onto this head with their own CI runs spinning up; #934 stays open as the tracker.

Maintainer authorized dismissal of this outdated review after the reported issues were resolved or explicitly accepted as product scope. Independent source review and local validation completed on the updated head.