Confluence page import: importer, worker, execution, reports, and wizard - #18
Confluence page import: importer, worker, execution, reports, and wizard#18Willyfrog wants to merge 30 commits into
Conversation
Implements the foundation of the restartable, report-driven Confluence v2
bundle importer described in implementation-plans/confluence-page-import.md.
Phase 1 — pure importer package (server/importer), no HTTP/DB/plugin deps:
- contract.go: v2 producer JSONL DTOs (version/space/page/page_comment/
resolve lines) mirroring mmetl's LineImportData.
- archive.go: secure ZIP inspection with named limit constants; rejects
traversal, backslashes, absolute/drive paths, symlinks, encrypted and
unsupported-method entries, and duplicate raw/normalized names; requires
exactly one root import.jsonl and import-manifest.json; permits but never
opens data/; enforces decompressed size limits while reading.
- inspect.go: strict v2 JSONL sequence/count/hierarchy validation, JSONL
checksum verification against the manifest, independent depth/cycle checks
(depth <= 10), page normalization into StagedPage, count reconciliation,
restricted-page intersection, and stable inspection issue codes.
- tiptap.go: TipTap validation, deterministic compact canonicalization,
SearchText extraction (block separators, hard breaks, whitespace collapse),
and placeholder link discovery limited to approved attrs.
- hash.go: versioned canonical source/applied-state SHA-256 hashing, stable
across map key order; 64-lowercase-hex validation.
- links.go: Confluence placeholder classification (page id/title/file/attachment).
- Full unit-test suite; go test ./server/importer/... passes.
Phase 2 (models + migration):
- model/import.go, model/import_report.go: persisted structs, API-safe views,
state/action/target/mode enums matching the DB CHECK constraints, mandatory
page-only fidelity disclosure, and IsValid validation. Unit tested.
- store/migrations/000005_create_imports.{up,down}.sql: DOCS_ImportSource,
DOCS_ImportJob, DOCS_ImportStagedPage, DOCS_ImportEntity, DOCS_ImportIssue,
DOCS_ImportResult with the plan's indexes and constraints. Applies cleanly
against the Postgres test database via the existing store test harness.
Remaining phases (store CRUD/claiming, worker, HTTP API, webapp wizard) are
not yet implemented; see the PR description.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses confirmed review findings:
1. Placeholder discovery: the mmetl producer emits braced tokens
({{CONF_PAGE_ID:101}} etc.), but classifyPlaceholder only matched the bare
prefix, so real links/images were never discovered. Rewrote links.go to
match the producer's "{{CONF_...:target}}" form via regex (link/attachment
kinds), and detect any "{{CONF_...}}" token in ordinary text. Fixed the
test fixtures to use the real braced format.
2. Hierarchy depth off-by-one: verifyHierarchy counted the root as depth 0 and
accepted an 11-page chain, which the app layer (MaxPageDepth=10, root=1)
rejects at execution. Now counts root as depth 1 (new exported
MaxHierarchyDepth const) so inspection matches execution; corrected the
depth tests (10-page chain allowed, 11-page rejected).
3. Unbounded issue allocation: a valid sub-2 MiB manifest could carry hundreds
of thousands of warnings, each materialized into an issue. Cap copied
warnings at MaxManifestWarnings (1000) and emit one aggregate suppression
issue for the remainder.
5. Overlong titles: normalizePage only checked for an empty title, so a title
over PageTitleMaxRunes passed staging and failed later at execution. Reject
it during inspection with page_title_too_long.
6. Structurally invalid TipTap: walkNode accepted nodes with no type and text
nodes with no text field. Now requires every node to carry a non-empty
string type (tiptap_missing_type) and every text node to have a string text
field (tiptap_bad_text). Unknown type values are still preserved.
7. Declared model limits unenforced: ImportJob.IsValid / ImportSource.IsValid
ignored the display-name/space-title/error-code length constants, so
over-long values failed only at the DB VARCHAR bound. Enforce them at the
model boundary and add ImportIssueRecord.IsValid (stage/severity/code
length/message), which also consumes the previously-dead
ImportIssueCodeMaxRunes constant.
Added unit tests for each fix. go test ./server/... , go build ./... , and
golangci-lint on the changed packages all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Renumber the import migration and fix seven inspector/model hardening issues that are independent of PR #5: - Migration renumbered 000005 -> 000006 to avoid colliding with PR #5's 000005_add_draft_lastactiveat_baseeditat (morph keys on the version number, so two 000005s would block plugin activation). Updated the model comment and the implementation plan references. archive.go: - Validate mode, encryption, and compression method for every file entry, including data/ payloads that are never opened (previously method/encryption were checked only for import.jsonl/import-manifest.json). - Genuinely normalize entry names via path.Clean before duplicate detection (after the raw ".." check), so "data//x" and "data/x" collide as duplicates instead of the "normalized" map being a no-op alias of the raw map. inspect.go: - Reject a manifest with trailing data after its JSON object (decoder stopped at the first value). - Reject a JSONL line that carries a payload not matching its declared type (e.g. type:"page" also carrying a "space" payload). - Reject a bundle whose manifest source has no space key, since it becomes the ImportSource's required ExternalSpaceKey. - Use attachments_not_imported (plan section 20.2) for the attachment-records issue, distinct from the attachment_placeholder_not_imported link code. - Judge future timestamps against InspectOptions.Now + a skew allowance when supplied (fixed year-2100 ceiling as the pure-function fallback). - Include the manifest advisory target team in the aggregate team-mismatch check. model/import.go: - Require BundleSha256 to be a valid 64-hex digest (never empty) at the model boundary; a persisted job always has it from inspection. Added unit tests for each. go test ./server/... , go build ./... , and golangci-lint on the changed packages all pass; the renamed migration applies cleanly via the store test harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…symlink dirs, counts, escaped placeholders, title normalization)
1. Trailing-data checks (tiptap.go, inspect.go parseManifest): json.Decoder.More()
returns false before a closing "]"/"}" delimiter, so "{...}]" and "{...}}"
were accepted and canonicalized. Replaced with a second Decode requiring
io.EOF (the pattern already used in api.go), which rejects any trailing token
while still tolerating trailing whitespace.
2. Expose preflight revision (model/import_report.go): added Revision to
ImportReportSummary so a client can build a valid confirmation request from
the public projection without access to the internal job model. Populated on
the preflight summary only.
3. Symlink directory entries (archive.go): an entry whose name ends in "/"
skipped the mode/symlink check, so a symlink named "data/" was accepted.
Now reject the symlink mode bit for every entry up front, before the
directory exemption for the regular-file/method checks.
4. Manifest count reconciliation (inspect.go): a checksum-valid JSONL whose
parsed page/comment/attachment counts disagree with the manifest is now
rejected (InspectErrCountMismatch) rather than warned. The producer writes
exactly one line per counted entity, so a mismatch can only mean a corrupt
bundle and can never reject a well-formed one. Removed the now-unused
warning issue code.
6. Escaped placeholder braces (links.go): the producer escapes literal braces
in placeholder targets ("{"->"\{", "}"->"\}"). The old "[^}]*" target
stopped at the first escaped "}", so links to titles/attachments containing
braces were omitted from discovery. The target pattern now matches escaped
braces (RE2-compatible alternation) and the captured target is unescaped
back to literal form; applied to both the link and any-token regexes.
7. Title normalization (inspect.go): imported titles were trimmed but not run
through mmmodel.SanitizeUnicode, unlike Page.PreSave. Since import bypasses
PreSave, unsafe Unicode controls could be staged/stored and the source hash
was computed on the unnormalized title (causing spurious reimport conflicts).
Now SanitizeUnicode-then-trim, matching PreSave, feeding the normalized title
into the length check, staging, and the hash.
Added unit tests for each. go test ./server/..., go build ./..., and
golangci-lint on the changed packages all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stence 1. Valid large confirmations exceeded StringInterface's hidden 1 MiB limit (model/import.go). mmmodel.StringInterface.Value() rejects any marshaled JSON over maxPropSizeBytes (1 MiB), but a valid confirmation can approve up to 5,000 conflict overwrite descriptors (~350 bytes each ≈ ~1.75 MiB), so it could never be persisted. Introduced a dedicated ImportConfirmation type (raw JSON, driver.Valuer/sql.Scanner) with its own deliberately higher bound (ImportConfirmationMaxBytes = 4 MiB) sized for that worst case, and switched ImportJob.Confirmation to it. The type is documented with the worst-case math and the note that the Phase-4 confirm handler must cap the request body to the same bound (Value() is only the last-line backstop). ImportJob.IsValid now rejects an over-cap confirmation. The bundle summaries keep using StringInterface (fixed-shape count structures, never near 1 MiB). 2. NUL characters passed inspection but PostgreSQL cannot store them (importer). A TipTap text node, title, author id, user proposal, or import_labels prop containing a decoded NUL (U+0000) would fail the staging insert — TEXT columns reject a raw NUL, JSONB rejects the escaped-NUL code point — even though the bundle inspected cleanly (SanitizeUnicode does not drop NUL). Added stripNUL / stripNULFromValue helpers and apply them to SearchText normalization, the title, the author account id, the user proposal, and (recursively) the source-props map. Stripping runs before hashing so the source hash matches the stored value. CanonicalBody was already safe (json.Marshal escapes NUL to a literal \u escape valid in TEXT). Added unit tests: ImportConfirmation Value/Scan round-trip incl. a >1 MiB payload that StringInterface would reject and an over-cap rejection; IsValid over-cap case; and an inspection test asserting NUL is stripped from title, search text, author id, user proposal, and import_labels while surrounding characters survive. go test ./server/..., go build ./..., and golangci-lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the synchronous upload/inspection half of the import flow. A valid mmetl
v2 bundle can now be uploaded, validated, and staged in PostgreSQL, and its
findings read back. No page is written and no Space is provisioned: an accepted
job stops at awaiting_source (existing target) or queued_preflight (new target),
which is the plan's stated end state for phase 3.
Store (server/store/import_store.go):
- CreateImportJobWithStaging inserts the job, every normalized staged page, and
every inspection issue in one transaction, so a job is never visible without
the staged input the worker will need. Staged pages are written in batches
bounded by both bind-parameter count and accumulated body bytes.
- GetImportJob, GetImportJobsForActor (actor-scoped only), GetImportIssues with
stage/severity filters, CountImportStagedPages, GetImportSourcesForSpace, and
CountImportSourceMappedPages.
App (server/app/import.go):
- CreateImportFromBundle: authorize, inspect, build the job/staging/issues, and
persist. Existing target requires Space membership and derives TeamId from the
Space (never the request); new target requires active team membership plus
PermissionCreatePublicChannel, and pre-generates the target Space id.
- Importer rejections map onto the plan's error contract: 400 malformed,
422 content-limit, 413 archive-size, each carrying the importer's stable code.
- Actor-only job/issue visibility (another user's job reads 404, not 403),
bundle-summary projection, bundle-derived required acknowledgements, and
ImportSource candidate suggestions with match reasons (never auto-selected).
API (server/api_import.go, routes in server/api.go):
- POST /imports/preflight streams the archive to a 0600 file in a fresh 0700
temp dir via MultipartReader (never ParseMultipartForm), computing SHA-256
while writing, and removes both on every return path. Exactly one request and
one bundle part are required.
- GET /imports, GET /imports/{job_id}, GET /imports/{job_id}/issues.
Manual verification affordances (the point of this milestone):
- server/cmd/genimportbundle writes a valid bundle ZIP so no mmetl run is
needed, with -with-findings for a bundle that produces issues and -corrupt for
five single-rule violations. It shares internal/importfixture with the tests,
so a hand-run check and CI exercise the same fixture shape.
- docs/confluence-import-manual-verification.md: end-to-end curl walkthrough,
expected responses, the rejection matrix, the log lines to look for, and SQL
to confirm staging landed and nothing was written.
- Operator-facing structured logs: accepted at info (job/actor/team/target,
bundle digest, and counts), rejected at warn with the stable error code and no
bundle content. Info was added to app.Logger for this.
i18n: 55 new keys added via `make i18n-extract-server`. Two helpers built their
message ids dynamically, which the extractor cannot see; both were refactored to
string-literal ids at the call site per the repo convention. `make i18n-check`
passes and re-extraction is idempotent.
Tests: 9 new API integration tests (15 subtests) over the real router and an
isolated Postgres schema, covering both target kinds, all five broken-bundle
rejections, multipart validation, every authorization gate, actor-only
visibility, issue filtering, listing/scoping, and that staging is persisted while
the page tree is untouched.
go test ./server/... , go build ./... , go vet ./... , and the repo's pinned
golangci-lint all pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ns, admission)
Reworks the Milestone A foundation for the revised plan: V1 is a single-node
restartable importer, inspection is memory-bounded, and content is separated from
structure. Applied the four decisions taken on review:
- NUL/invalid UTF-8 is now REJECTED, not stripped (reverses the earlier fix).
- Pagination deliberately keeps this repository's {items, page, per_page,
has_more} + limit+1 envelope rather than the plan's {jobs/issues, total}
shape, so the import API matches every other endpoint. No COUNT totals.
- No cluster guard added (import remains unsupported on clustered deployments).
- Confirmation adopts the plan's flat external-ID list at 8 MiB.
Schema (migration 000006, updated in place since it has not shipped):
- Dropped all HA machinery: ClaimToken/ClaimedBy/LeaseExpiresAt/HeartbeatAt,
ImportSource.ActiveJobId, the active-target unique index, and the
waiting_source_turn/canceling states.
- Added ImportSource.MappingRevision (optimistic invalidation replacing
full-lifecycle source ownership), the terminalizing state with TerminalIntent,
MappingInputsChanged, InvalidationPending, and staged/retained byte counters.
- New tables: DOCS_ImportChannelAttempt (durable channel-create ledger),
DOCS_ImportManifestUser (worker input that must survive the request), and the
DOCS_ImportCapacity singleton for admission accounting.
- Bounded every indexed external identifier (VARCHAR(512)/(255)) and renamed the
hash columns to *ContentHash, adding explicit parent baselines.
Importer:
- archive.go: Archive handle with bounded ReadManifest, streaming JSONLSha256,
and re-openable OpenJSONL; compressed-size check before parsing the central
directory; genuine-directory mode validation.
- inspect.go: rewritten as a streaming state machine that hands each normalized
page to an injected StreamSink instead of returning them all, so peak memory
no longer scales with bundle size. Adds manifest source-type and
resolve-line space-key checks, comment validation (known page, unique source
id, reply ordering), attachment source ids, manifest-user dedup/conflict
rejection, source-prop shape validation, bounded identifiers, and per-page
Restricted flags plus manifest-only restriction issues.
- hash.go: hash format v2 — parent and ordinal left the content hashes, and the
applied hash gained body_format for opaque (non-canonical) local bodies.
Store: streaming ImportStagingWriter that batches pages/users/issues inside the
upload transaction, plus admission accounting on the locked capacity row
(per-job/actor/target/global staged bytes and per-actor/target job counts) with
ErrAdmissionExhausted -> 429.
App/API: authorization now runs on the small request part BEFORE any bundle byte
is read; parts are required in order (request, then bundle) and trailing parts
rejected; a one-slot inspection semaphore with Retry-After bounds concurrent
inspections; the deactivation path closes that gate and waits for admitted
staging transactions before the store closes.
Typed JSONB: BundleSummary/PreflightSummary/FinalSummary/Confirmation are now
typed structs with explicit Value/Scan, since StringInterface's hidden 1 MiB
valuer is below the worst-case confirmation.
Tests updated throughout (streaming collector helper, rejection-instead-of-strip
NUL cases, capacity/manifest-user assertions); 34 new i18n keys extracted and
translated. go test ./server/... , go build ./... , go vet ./... , and the
repo's pinned golangci-lint all pass, and the bundle generator plus all five
corrupt modes still work end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Must fix:
1. Imported content now goes through the shared sanitizer. The importer calls
model.ParseTipTapDocument and canonicalizes/derives SearchText from the
*sanitized* document, so javascript:/vbscript: URLs, event-handler and style
attributes, and node/mark types outside the editor schema can no longer reach
storage. This deliberately reverses the plan's "preserve unknown node/mark
types" instruction — preserving them is exactly the bypass — and every type the
authoritative producer emits is on the allowlist. Also settles the deferred
Track B items: model exports MaxTipTapNodes/MaxTipTapDepth so there is one
node/depth limit instead of the importer's separate 250k, and hierarchy depth
already came from model.MaxPageDepth. Verified placeholders survive
sanitization (no scheme -> treated as relative).
2. Rejected multipart parts are no longer drained. multipart.Part.Close copies
the remainder to io.Discard, so closing a mis-ordered part made the server
consume a whole 250 MiB archive before refusing it — and before authorization
or the inspection semaphore. Rejected parts are now abandoned unread.
3. Retained-capacity accounting enforces its guarantee: added per-job/actor/global
retained limits checked on the same locked capacity row, RetainedBytes is now
computed from the manifest users and issues upload actually wrote, and the
reservation covers both result stages plus their issues per entity rather than
a single row.
4. Read APIs re-check entitlement. Ownership alone no longer suffices: a
deactivated actor, or one who lost access to the target team/Space, gets a
minimal job projection (id/state/error/timestamps only), has the job omitted
from listings, and gets 404 on issues.
5. The ZIP entry limit is enforced from the central-directory header (EOCD, with
ZIP64 follow-through) before zip.NewReader allocates a struct per entry, so an
archive whose 250 MiB is millions of tiny records is rejected first.
6. The workflow no longer dead-ends. Added POST /imports/{id}/cancel plus an
hourly maintenance sweep (one pass at activation) that expires stalled
pre-execution jobs as job_expired, purges terminal staged bodies after seven
days, and deletes jobs after ninety — each releasing the admission capacity it
held. Without this an abandoned upload permanently consumed the per-user job
budget. Expiry covers every cancelable pre-execution state, not just the
user-waiting ones: a new-Space job stalls in queued_preflight, which an earlier
draft of this fix would have leaked. Execution states are excluded so the
future terminalizer keeps its durable-outcome guarantee.
Should fix: manifest source.type is now required rather than only checked when
present; restriction ids/titles are validated before they can reach a column (a
NUL-bearing title was a 500); comments are bounded (MaxComments) since every
comment id is retained for uniqueness; the identifier pattern accepts '~' so
Confluence personal-space keys work (the plan's pattern was wrong too); result and
issue validation gained required outcomes plus local-id, title, details-size and
timestamp bounds; staged-byte accounting measures identifier columns instead of a
flat allowance that three 512-byte ids already exceeded; the store-admission 429
now carries Retry-After like the semaphore one; and the manual-verification doc no
longer claims a worker exists two lines after saying it does not.
Tests added for cancel/capacity release, the admission job cap and its recovery,
the maintenance sweep across all three retention stages, entitlement-loss
downgrade/omission/404, and sanitizer enforcement. 12 new i18n keys.
go test ./server/... , go build ./... , go vet ./... , pinned golangci-lint, and
make i18n-check all pass; the generator and all five corrupt modes still work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…able Closes three related defects in the terminal/capacity path. They are one change because they share a cause: the retained reservation was a guess, and cancellation neither spent it correctly nor gave it back. 1. Retained accounting was not conservative. The per-row budget was a flat 1 KiB, roughly a tenth of the largest issue row IsValid admits (bounded Details plus a message, a remediation, a title, and a 512-byte external id). RetainedBytes was a row *count* times that figure rather than measured content, and the bundle summary — durable JSONB up to 256 KiB — was not counted at all. Worst case the reservation was ~60x short, so the guarantee its own comment claimed was not backed by the arithmetic. Row-size bounds now derive from the model's validation constants, upload charges measured bytes, and the reservation separates the two classes of retained row: mandatory per-entity outcomes reserved at worst-case size, and a flat, explicitly-enforced allowance for the issues explaining them. Reserving the model's hard per-page issue cap instead would need hundreds of megabytes per bundle, which no admission budget could grant. 2. Cancellation leaked most of the retained reservation for ninety days. releaseStagedBytes only ever touched the staged reservation, and the per-actor retained sum has no state filter, so a canceled job kept a full execution's worth of budget until retention deleted it. About twenty upload/cancel cycles exhausted the per-user allowance — and because canceled jobs are terminal, the concurrent-job cap gave no protection: the user was locked out while holding no live job at all. Terminal jobs are now trued up to measured usage. 3. Cancellation deleted page identities before recording any outcome. The staged rows are the only record of which pages a bundle held, and they were dropped with no ImportResult and no FinalSummary behind them, so a canceled job's report could not name a single page. Every staged page now gets a durable not_attempted_canceled outcome, written in bounded batches before the rows go away, satisfying the plan's acceptance criterion that every staged page have an outcome before terminal state. Expiry inherits all three through CancelImportJob. Tests pin each: the upload/cancel regression test fails at cycle 20 against the old accounting, and a model test asserts the row-size constants bound the largest row IsValid actually accepts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Must-fix 1. The ZIP entry-count precheck read the trailer's declared total, which Go's reader does not enforce — it reads headers until one fails to parse and then compares only the low 16 bits, so an archive declaring 0 entries while carrying 65 536 passed. The precheck now counts actual central-directory records with a bounded walk (stopping the moment the cap is passed, reading only each record's 46-byte fixed header), and derives only the directory's *location* from the trailer. Both start offsets archive/zip may choose are walked, since over-counting a malformed archive costs a rejection whereas under-counting reopens the allocation hole. 2. Retained accounting split into two enforced pools. The final summary was charged after the reservation was trued up, so it was never counted; and RetainedRemaining exposed the mandatory-outcome reserve as headroom, letting discretionary issue writers spend the capacity a job needs to record what happened to its pages. RetainedIssueBytes now tracks the issue spend on its own, IssueBudgetRemaining is measured against the flat allowance rather than the unspent reservation, and cancellation charges the summary before trueing up. 3. Cancellation redacts on entitlement loss. Owning a job authorizes cancelling it, not reading its target; the response now uses the same minimal projection GET returns, so cancelling is not a way to read back the selected source's display name and other Space-side fields. Should-fix - ErrorCode is persisted on cancel, so canceled_by_user / job_expired survive to later reads instead of contradicting the constants' own documentation. - The returned job mirrors every column the cancel wrote (UpdateAt reproduces monotonicBump exactly, so no extra read is needed). - Temp-file write failures are 500, not 400: the destination is wrapped so a write-side fault is distinguishable from a truncated upload. The bundle part is abandoned rather than Closed, since Close drains the unread remainder of a body already refused. - Job listing filters entitlement before paginating, counting positions in entitled rows, so pages are dense and has_more is accurate. The scan is capped and reports has_more when the cap is hit. - manifest source.space_name is checked for storability and length, and the space title/description are bounded, so malformed input is a rejection rather than a column or summary write failure. - Absent source-namespace mirrors are rejected, not tolerated. - The importer's TipTap walk starts at depth 0, matching the shared sanitizer, so imported content is no longer one level stricter than content the editor accepts. - The sanitizer's size, depth, and node-count rejections carry sentinel errors and keep their specific codes, and InspectError preserves its cause, so the 422 mappings for over-limit content are reachable instead of collapsing into a generic 400. Before later worker phases - Cancellation records outcomes for preflight-classified entities with no staged page (the stale mappings), not only for staged pages. - Retention deletion exempts jobs holding a pending_compensation channel attempt, which cascades on job delete and is the only pointer to a channel the import must still clean up. Kept jobs are counted and logged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…firmation
Takes a job from an accepted upload to queued_import. Execution (phase 5) is
still absent, so a confirmed job waits there.
Source selection (POST /imports/{id}/source). An ImportSource is the local
identity a Confluence space's page history is tracked against, and it is always
chosen explicitly: two Confluence instances can share an organization id, a
space key, and a display name while being different sources, so candidate
scores only order suggestions. A "new" source reserves its id on the job but
inserts no row until execution — an unconfirmed job must not leave an identity
behind for later jobs to match against.
Worker. The maintenance goroutine becomes the single V1 worker: it drains
available work, then idles, and runs the hourly sweep from the same loop so
cleanup never overlaps the work whose capacity it reclaims. Selection is an
ordered read and every transition is a compare-and-set, so there are
deliberately no leases, claim tokens, or heartbeats — with one worker, losing a
CAS is the only contention possible and it resolves by re-reading. A job found
already preflighting was interrupted, and since preflight publishes
all-or-nothing there is nothing partial to salvage: it is requeued.
Preflight. Authors resolve once per distinct source identity against the
durable manifest rows (never the upload's in-memory manifest), falling back to
the importing actor with a stable reason rather than failing. Pages stream in
batches so a five-thousand-page bundle never holds five thousand bodies. The
reimport decision table lives in the pure importer package, with content and
structure deliberately independent: a parent move on either side is reported,
never applied, and never turns a safe body update into a conflict. A local body
that no longer canonicalizes hashes as opaque, which protects it as a local
edit instead of failing the run. Sibling-capacity and depth breaches block
creates after classification, since capacity depends on what the whole plan
adds. Publication is one transaction against the mapping revision the
computation used; a revision that moved discards the derived set and recomputes.
Confirmation (POST /imports/{id}/confirm). Rechecks the actor's access, the
exact preflight revision, the source's mapping revision, and each approved
conflict against persisted results. The browser never sends hashes: approval
carries intent, server-owned baselines carry safety, and there is no blanket
overwrite-all flag. A stale mapping revision clears the confirmation state,
returns the job to the preflight queue in the same transaction, and answers 409
preflight_stale_recomputing — so a client is never left holding a revision it
cannot confirm.
Also adds GET /imports/{id}/preflight-results (typed review rows, no hashes or
bodies), the docs_import props namespace with the canonical
source-mirroring subset the applied hash uses, and persisted issue text with
remediation for every preflight finding.
Tests cover the decision table as pure logic, both target kinds end to end,
author resolution and fallback, interrupted-preflight recovery, every
confirmation rejection, and the stale-preflight recompute.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
P0 — one confirmed job wedged the importer permanently. Work selection returns the highest-priority non-empty state, and queued_import outranked queued_preflight while the worker could not advance it: the same job was re-selected on every pass and every later preflight starved. Neither queued_import nor terminalizing is expirable, so it never self-healed, and a failed preflight triggered it by parking the job in the highest-priority state of all. Three changes close it. Selection now offers only states this release can advance, so an unhandled state cannot be picked at all. Terminalization is implemented rather than deferred, sharing one path with cancellation, so a failed preflight reaches a terminal state with a durable report — idempotently, via an anti-join, so a crash mid-way resumes instead of colliding with its own rows. And queued_import joins the cancelable states, so a confirmed job waiting for execution can be given back instead of holding its slot forever. A second starvation loop surfaced while testing this: a job whose selected source had been deleted failed BeginImportPreflight with a not-found error, which the worker treated as "the job advanced" and skipped — forever. Missing *source* now has its own error type, distinct from missing *job*, because the two demand opposite responses. P1 — preflight rows bypassed retained accounting. Publishing inserted results, issues, and a summary without charging any of them, so cancellation trued the reservation down to an understated figure while the rows stayed for ninety days. The charge is now measured and applied, and is *replaceable*: two columns record what the current plan costs so a recompute neither double-counts the plan it replaced nor loses it. The flat per-job issue allowance is enforced too, with visible truncation instead of a silent cap. P1 — structural projection approved impossible trees. Depths were queried only for existing parents, so a chain of new pages beneath an existing one could breach MaxPageDepth with no row revealing it, and blocking a parent left its descendants planning to nest under a page that would never exist. The projection now walks the staged tree in parents-before-children order, deriving each new page's depth from the plan, and cascades blocking through descendants. P1 — mapping capacity double-counted. plannedIDs holds an entry for every existing mapping seen in the bundle, so adding its length to the mapping count counted those pages twice: valid creates were blocked early and the outcome depended on bundle order. Planned new mappings are now counted on their own. P2 — author changes read as no-ops. Inspection hashed the page's own proposal while preflight hashed the manifest's, so with no manifest mapping a changed page author left the hash untouched even though resolution used the page fallback. Both now use one shared effective proposal. P2 — oversized request parts were drained before authorization. The decoder deliberately stops at its cap, so Close() drained the rest, letting a caller push most of the 250 MiB body limit through before any target check. P2 — oversized import_labels produced a 500. Shape was validated, size was not, so a contract-conforming bundle failed at a column limit. Serialized props are now bounded against a fraction of the page-props limit, since they end up nested inside it. P2 — final summaries were never returned. Cancellation and terminalization persist FinalSummary, but the job projection only set Preflight, so every terminal job reported final: null and the outcome work was invisible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the phase that actually writes pages. Execution applies one staged page per transaction, committing the page write, its mapping, and an immutable execution result together, so a crash resumes at page granularity instead of reimporting the bundle. Provisioning a new Space is the only step that reaches an external system, so it keeps its own durable trail: every channel-create attempt is recorded before the call is made, with a random name, and the Space row is written last as the point of no return. Terminalization compensates a channel whose Space never came into existence and reports the outcome either way. Terminalization is now one path for completion, failure, and cancellation. It verifies a completion claim against the staged pages, records not-attempted or stale outcomes as the intent requires, rebuilds the final summary from the committed outcomes rather than from memory, bumps the source's mapping revision exactly once, and publishes the tree invalidation. Two defects the tests exposed along the way: - A page the reviewed plan blocked was reclassified from scratch at execution. If the limit that blocked it had since cleared, execution decided to create it and then failed the entire job on the missing planned page id. The plan's decision is now honoured, reported as skipped_by_reviewed_plan. - MappingInputsChanged survived the revision bump it caused, so a second terminalization pass bumped the source again and invalidated every other job's reviewed plan for work that did not happen. Issue text moves from app to importer, because execution writes issue rows from inside the store's page transaction and a finding must not be worded one way at review time and another at execution. Cancellation now routes on state inside one job-row lock: a job with nothing committed still finishes immediately, while preflighting and importing go to the terminalizer so partial work is reconciled before any report is published.
…enance All ten were reproduced against the code before being fixed. Authorization now follows the target rather than what was true at upload. TargetSpaceExisted never changes, so a new-Space job kept asking the team question after provisioning its Space — letting an actor removed from that Space carry on writing into it, and keeping target-identifying fields visible to them afterwards. Both the write gate and the read projection now switch to Space membership once the Space row exists. Transient failures no longer masquerade as decisions: - Only a definitive 403/404 revokes an import. A 500 from the authorization recheck used to terminalize the job as authorization_revoked, destroying a possibly half-written import over a blip and labelling it with a reason that was not true. - A user lookup that fails no longer permanently reattributes a page to the importing actor, and a manifest read that fails no longer silently swaps the manifest's username for the page's own — which would write props diverging from the reviewed plan. - A channel lookup that fails no longer counts as successful compensation. That let a job report "this channel was cleaned up" while a live orphan remained. Failed compensation is now retried by the maintenance sweep, which also corrects the finding in the report. Previously one transient archive failure stranded the job forever: it is terminal, so it never re-enters terminalization, and retention refuses to delete it while the attempt row points at an orphan — leaking both the pointer and the retained reservation. Smaller correctness fixes: - An imported update could move EditAt backwards, because the timestamp was read before the page lock. EditAt is an optimistic-lock token, so a stale client could then pass a compare-and-swap it should fail. - Merged page props are validated before the direct update. A page near the props limit would otherwise be written over it, leaving a page model validation refuses and the normal API can no longer edit. - The sibling projection blocked every create under a parent as soon as one did not fit. Ninety-nine existing children plus two new ones lost both, though there was room for one — and a plan-blocked page now stays blocked, so that pessimism was permanent. - The issue allowance is one job-wide budget again. Preflight started a fresh 16 MiB counter instead of subtracting what inspection had spent, and the stale and structural-projection producers bypassed it entirely. - A failed worker pass now backs off. RunImportWork reports worked=true whenever it selected a job, error or not, so the loop retried a persistently failing job as fast as the database could answer.
GET /api/v1/imports/{job_id}/report?stage=preflight|final, closing the §20.3 gap
that was Phase 3 scope and got missed. The final report is now meaningful, so
Phase 7 needs these to validate anything.
The response is streamed key by key rather than marshalled from a filled-in
struct: a report covers every entity a job touched, so building one in memory
would scale with the bundle. Result and issue rows are read keyset-paged on
Ordinal rather than by offset, because a downloaded report is one logical read
spanning thousands of rows and must not skip or duplicate any of them.
Authorizing and writing are separate steps. Every reason to refuse — not the
caller's job, access lost, stage not ready — is decided before the first byte,
because an HTTP status cannot be revised once the body has started.
The two stages answer different questions and are kept apart deliberately: the
preflight report is the plan that was reviewed, the final report is what
happened. Both carry the same immutable inspection findings, which describe the
bundle rather than either. Neither carries hashes, bodies, SearchText, bundle
digests, archive paths, or manifest user rows.
"final" is the public name for the execution stage. The internal stage name is a
pipeline detail, and a reader is asking for the final outcome rather than for the
worker phase that produced its rows.
All eight were reproduced against the code. Two are design decisions and are reported rather than changed; see below. A definitively missing actor is now a denial rather than an inconclusive lookup. This was a starvation loop I introduced with the retryable-error work: a 404 from the actor lookup was reported as a 500, which the execution path classifies as retryable, which leaves the job in importing — the highest-priority state — so the worker re-selected that same job on every pass and nothing behind it ran. actorStillEntitled already got this right; the two had diverged. Preflight's author resolution now distinguishes transient from definitive too. I fixed this at execution and missed the preflight side, where it matters more: the fallback is persisted on the staged row and execution deliberately does not re-resolve it, so a lookup that merely failed permanently credited someone else's pages to the importing actor. An inconclusive lookup now returns the job to the preflight queue, which costs nothing because preflight publishes all-or-nothing. A 'provisioned' channel attempt on a terminal job now counts as uncompensated. Recognizing only pending_compensation made one case invisible twice over: when the archive failed *and* recording that failure also failed, the attempt stayed provisioned, so it was never retried and was eventually deleted along with the job — taking the only pointer to a live channel with it. Admission rechecks the per-actor and per-target job counts under the capacity lock. The pre-streaming check is a courtesy that rejects an over-subscribed actor before a large bundle is parsed, but on its own it is a read outside any mutual exclusion, so two uploads could both claim the same last free slot. The job listing memoizes entitlement per target instead of per row, which is what makes a generous scan cap affordable, and no longer promises a next page it cannot deliver: a request past the cap's reach returned an empty page with has_more=true, so a client that trusted the flag paged forever. Report summaries are counted from the rows the report actually emitted, and written after them. Taking them from a separate job snapshot let a document state totals its own contents contradicted.
Phase 6 starts here, and it is the webapp's first server-backed code: every existing hook reads from a synchronous mockDataSource, so there was no HTTP layer to extend. This adds one scoped to imports rather than migrating that seam, because the import flow is inherently async, polled and long-running — bending it into a synchronous mock interface would fit neither. types/imports.ts mirrors the server's wire shapes. What it omits is deliberate: the server never sends content hashes, page bodies or mapping baselines, so a client cannot reason about — or forge — an approval. The comments record which absences are decisions. rest_client.ts handles the two things the shared Client4 helper cannot express. A 409 nests its AppError inside the conflict envelope every Docs conflict uses, so reading it as a bare AppError loses the id — including preflight_stale_recomputing, which a client must recognize to know the job has already been requeued and this revision can never be confirmed. And a 429 carries Retry-After, without which an admission rejection can only invite an immediate retry that fails the same way. Bundle rejections share one message id and distinguish themselves by a code passed as a message parameter, because DetailedError is scrubbed before it is sent. The upload sends its target part before the bundle, matching the order the server reads them: it authorizes the target before spending disk and parser work on the archive, so the reverse order makes it buffer an upload it is about to reject. Tests assert against the envelopes the server actually produces, including a non-JSON failure body — a proxy's HTML 502 must not surface as a JSON syntax error, since the status is the actionable part.
useImportJob drives a job for as long as it can still change, at a cadence taken from what the server is actually doing: brisk while the worker owns the job — which is the only feedback a user gets while thousands of pages are written — slow while it waits on a person, and not at all once it is terminal. Polling rather than the WebSocket event is deliberate for this pass. The server does publish an actor-scoped import_job_updated, but best-effort and at most once per second or 25 pages, so a client that only listened could sit on a stale view if one were dropped. Polling is the correctness floor; the event is a latency optimization to layer on top. Three things the loop has to get right, each covered by a test that fails without it: - It stops on a terminal state, or it polls a finished job forever. - refresh() does not issue a read while one is in flight. Two overlapping reads can resolve out of order, and during an import that shows progress running backwards. - It stops on unmount, and ignores a response that arrives after the job id changed. A DocsApiError is a real answer about the job — gone, or no longer visible — and is surfaced. Anything else is a transport failure that says nothing about the job and that the next poll may recover from, so the last known state is kept rather than replaced by an error screen. requiredAcknowledgementsSatisfied takes the required set from the job rather than from the client's own reading of the bundle: the server derives it from persisted counts, refuses a confirmation missing any key, and equally refuses keys it did not ask for — so a fixed list of checkboxes would eventually make an import unconfirmable.
PR #12 ("Baseline Spaces and Pages UI") already adds webapp/src/client/rest.ts, so my own rest_client.ts would have been a second HTTP client in the same directory doing the same job. Theirs is the better foundation — it uses Client4.getOptions for credentials and CSRF rather than hand-rolling headers, extends ClientError so callers keep matching on it, keeps the raw error payload, and threads AbortSignal throughout. rest.ts and rest.test.ts are taken from #12 *verbatim* and are byte-identical to it. That is the point: when #12 merges, resolving these two files is "take theirs" and nothing else moves. They are not mine to change — anything I needed that they do not do is handled on my side instead. Stacking this branch on #12 was the alternative and was worse: 96 commits over ~200 files, and this work could not merge until that did. rest_client.ts is deleted and imports_client.ts becomes client/imports.ts, matching the client/<family>.ts convention #12's own comments describe. Two things about this API do not fit the shared helper, and both stay local: - The bundle upload is multipart, so its body must not be JSON-serialized and its Content-Type must be left to the browser. - Only that upload can be rejected for admission, and that rejection carries Retry-After in a header the shared helper does not surface. Since the one call needing response headers is also the one needing a custom body, it gets its own request path and nothing else changes. The conflict envelope needed a different answer. A 409 nests its AppError inside the envelope every Docs conflict shares, and the shared helper reads the id from the top level — so for a conflict it finds nothing. importErrorId/importErrorCode read through the envelope, which is how isPreflightStale can recognize that the job is already back in the preflight queue and this revision can never be confirmed. That required no change to their file.
Upload, source selection, review, progress and report, driven by the job rather than by local step state. That is the central decision: an import outlives the page that started it — it survives a reload and a server restart — and the server can move it *backwards*, since a confirmed plan whose source changed is returned to awaiting_confirmation on its own. Local step state would drift from all of that, and worst exactly when it matters, so stepForJob derives the step and nothing else tracks it. Every worker-owned state is one "running" step. The difference between computing a plan and writing pages is a phase label, not a different thing for the user to do, and the progress bar only appears once there is a total to measure against rather than sitting at zero pretending otherwise. Two things the review step must not get wrong, each pinned by a test that fails if it does: - The acknowledgement checkboxes are exactly the job's required set. A fixed list would eventually miss a key the server demands or offer one it refuses, and either makes the import unconfirmable. - Conflicts are approved per page and default to off, because each approval discards a specific person's edits. There is no approve-all. The revision is echoed back exactly as received, so a confirmation can only apply to the plan that was displayed. A stale-plan refusal is explained rather than retried: the server has already requeued the job, so a retry button would be one that cannot work. The result step never says "success" unqualified — completed_with_issues is the common outcome for a real Space, and presenting it as clean would hide the findings worth reading. Both reports stay downloadable, since comparing the plan against the outcome is how a user checks they got what they approved. No dependency on components/form-controls or generic_modal: PR #12 deletes the former outright and modifies the latter, so the wizard uses native controls and its own stylesheet to keep the merge surface at zero.
…nd wizard Ten findings, all validated against the code before changing anything. Server: - The ZIP64 entry-count precheck measured the central directory's end from the ordinary EOCD, but archive/zip measures it from the ZIP64 record 76 bytes earlier. In a prefixed ZIP64 archive the raw declared offset is wrong too, so both candidates counted zero and any number of entries passed the cap the reader then allocated. Both bases are now offered. - A retryable worker failure left its job in the state it was selected from, and selection is ordered by state, so the same job won every pass forever while unrelated imports waited on a fault of someone else's. Failed jobs now serve a cooldown, and one that keeps failing is failed with a report rather than retried indefinitely. - Confirmation left the upload-time expiry deadline in place, so confirming near the end of the review window let maintenance cancel an approved import. The clock restarts with the state change. - A channel archived by a pass whose terminalization did not commit was skipped by the next pass, dropping the cleanup from the report permanently. The finding is now rebuilt from the attempt row. - A report's rows are read by several queries, so a plan republished mid-stream produced a document that parsed, counted correctly, and described two plans. Reports now state whether they describe one version of the data. - A source display name carrying an escaped NUL reached PostgreSQL as a 500 instead of being refused as the bad request it is. Webapp: - Polling stopped for good on a transient read failure, and refresh() replaced the recurring loop with a single read — so normal use (selecting a source, confirming) froze the view for the rest of an import that was still running. Only a terminal or absent job stops the loop now. - The wizard held its job id in component state alone, so closing or reloading stranded a running import out of reach. It is recovered from the server, and the wizard is reachable: a sidebar entry beside "Create a space". - Approvals and acknowledgements survived a plan being recomputed, letting consent given for one plan be submitted against another. They are withdrawn with the plan, and confirmation waits for the new rows. - Conflicts past the first hundred rows could not be approved at all. They are fetched by action, with paging, so every approvable conflict is reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The wizard existed but could only be reached by whoever clicked a button, in
that tab, until they navigated away — which is the wrong shape for something
that runs for minutes on the server. Where you are in an import now lives in
the URL:
/{team}/spaces/import import into a new Space
/{team}/spaces/{spaceId}/import import into that Space
So a reload lands back on the running import, and the address is shareable.
Like `drafts`, this reserves a word: a Space or page whose custom slug is
literally `import` is shadowed by it, since both import routes are matched ahead
of the id-bearing ones. Nothing validates slugs against these words yet.
Two entry points, matching the two targets. "Import from Confluence" sits beside
"Create a space" in the sidebar, because an import into a new Space is another
way of making one; and it is in a Space's own menu, because importing into a
Space that already exists is a decision about that Space — which of its pages
the bundle adopts, and whose edits an overwrite would discard.
Finishing an import into a new Space now lands on the Space it created rather
than on an empty product home. Only a completed import does: a failed or
canceled one may have had its half-built Space cleaned up, and sending someone
to a Space that is gone is worse than not offering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Serialization (P1, introduced by the retry cooldown two commits ago). Passing a failing job over let the job behind it start against a source the first was halfway through writing. The revision that invalidates other plans is only bumped at terminalization, so the second job's staleness check passed and it would have rewritten pages its reviewed plan called creates — without the acknowledgement reimporting existing pages requires. Selection now fences a source and Space held by any job that may have written: a deferred job keeps holding them, and the fenced job comes back for review instead. ZIP64 (P1). With a maximum-length comment the end-of-directory record sits at the first byte of the trailer window, so the locator preceding it is outside the window entirely and a buffer-relative read concluded "not ZIP64" and counted no entries — while the reader, reaching the locator through ReaderAt, allocated every one. The locator is now read from its absolute offset. Retention. The expiry sweep selects without a lock, so a job could be confirmed between selection and cancellation and be canceled as expired anyway; the deadline is now rechecked under the same lock as the state. And a stale-plan reset left the old deadline in place, so a plan the server itself invalidated came back already expired — it restarts the clock, as confirmation does. Retry budget. Every failing pass counted, including ones that committed a hundred pages before an inconclusive recheck, so ten progressing passes could terminate a partly finished import as retries_exhausted. Progress now takes the cooldown but not the attempt. Webapp: - The import URL used a bare `import` segment, which shadowed a Space of that name and a page of that name in every Space, with nothing validating either. It is now `_import`, which no slug can produce. My earlier claim that this was the same trade `drafts` makes was wrong: `drafts` only claims a three-segment shape no content route uses. - Switching target left the previous target's job on screen, with its cancel button live, until the new lookup returned — and forever if it failed. State is now keyed by target identity and reset during render. - A page of conflicts could land after the plan changed and become approvable under the new revision. Requests now verify the revision through a ref, since a closure can only ever compare its own value with itself. - Discovery read one page of jobs and treated a failed lookup as "none running", either of which starts a duplicate import. It now pages to a bound and reports a failure with a retry instead of offering an upload. - A transient review-table failure disabled confirmation permanently, since the load only re-ran on a revision change. It states the failure and offers a retry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An import writes Spaces and pages straight to the database, so the store holding this product's view of them is stale the moment one finishes: the sidebar does not list the new Space, and following the link to it lands on an empty product home instead of the thing the user just spent minutes creating. This is a no-op until the API-backed data source lands (PR #12) — today the store is fed from mock fixtures that know nothing the server wrote, which is also why the import UI cannot yet show any imported content at all. Wiring it now means that when the real data source arrives, landing on a freshly imported Space is not a step someone has to remember. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every segment naming something other than content now begins with an underscore: `_drafts` (renamed from `drafts`) and `_import`. A space id or page id can never start with one, so these cannot collide with anything a user names — no reserved-word list to keep in step with the routes, nothing to enforce at creation time, and no migration if custom slugs arrive later. The alternative, a bare word matched ahead of the id routes, is safe only circumstantially. `drafts` got away with it by arity: no content route has three segments after `spaces`, so a page named `drafts` stayed reachable. A two-segment reserved word does not get that reprieve — it sits exactly where a page id goes and hides any page addressed that way, which is what `import` did before it was underscored. RESERVED_SEGMENTS and paths.test.ts make the rule mechanical rather than a thing to remember: every entry is asserted unmatchable as an id, so a segment added without the underscore fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the upload Refusing to proceed when the resume lookup fails made an unrelated GET a hard dependency for the whole feature: one failing read left the wizard showing an error and a retry button, with no way to upload anything. That is a worse outcome than the duplicate it was guarding against — and the guard was never the real one, since the server enforces the per-target job limit itself. The check saves a wasted upload; it does not prevent a bad one. It also could not say what went wrong, which matters because the likely causes need different actions and are indistinguishable otherwise: 501 is Docs switched off server-side, 404 is a plugin build older than the page talking to it, 401/403 is an expired session, and no status at all is a network that did not answer. The warning now names the cause, keeps the retry, and offers the upload regardless. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The panel inherited its text colour instead of setting one, and the host's main area deliberately provides no surface — every content component establishes its own, as docs_home does. On a dark theme that left dark text on a dark panel: headings, labels, counts and inputs were all but invisible, because those were exactly the rules with no explicit colour. The wizard is now a surface of its own (background and base colour from the theme), with an explicit token on every piece of text, a reading column capped so the review prose and table stay scannable, and the muted greys lifted from 0.56 to the 0.64/0.75 steps the rest of the product uses. The file input is also visually hidden behind a styled control now. Its native "No file chosen" renders in a colour the page cannot set — the worst offender in the screenshot — and it offered nothing that read as a button. The input still does the work, so the label, keyboard and screen readers are unchanged, and the chosen filename is now our text, with a test to keep it that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed segments Master now carries #12's Spaces/Pages data layer and #13's editor, which is what the import UI was waiting for: fetchSpaces/fetchPages read the plugin REST API, so a Space the importer creates finally appears in the sidebar, and the refresh this branch already dispatched on completion stops being a no-op. Resolved: - client/rest.ts, rest.test.ts: master's, wholesale. This branch carried a copy of the same file so there would be one HTTP layer rather than two; master's has since grown a basename fallback in siteRoot, and apiUrl is still exported, so client/imports.ts needed nothing. - routing/paths.ts: applied the underscore rule the same way to every non-content segment, so master's `overview` joins `_drafts` and `_import` as `_overview`. Its safety was circumstantial — it sits exactly where a page id goes and would hide any page addressed that way the moment user-chosen slugs return, which the URL scheme still advertises. RESERVED_SEGMENTS now covers all three and paths.test.ts asserts each is unmatchable as an id. - hooks/navigation.ts: master's overview/edit/RHS state plus this branch's isImport and goToImport, with the import routes matched ahead of DOCS_ROUTE. - docs_root.tsx: re-applied the routed import panel into master's restructured root (modal controller, readout, toaster, resizable sidebar, fullscreen). - space_item_menu.tsx: master rewrote the menu from a MenuItemSpec array to declarative Menu.Items; the import entry is re-expressed in that form. - navigation.test.tsx, paths.test.ts: both suites kept, master's URLs moved onto the renamed segments. Its "not editing on the overview URL" case was failing for the right reason — a bare `overview` now parses as a page id — which is the collision the rename removes. 550 webapp tests across 76 suites, server suite green, golangci-lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The wizard was built with native controls and its own button styles specifically to keep the merge surface with #12 at zero while form-controls was being renamed under it. That is over, so the hand-rolled versions go: PrimaryButton, SecondaryButton and TertiaryButton replace the three button classes, and TextInput replaces the new-Space title/description fields and the new-source name field — each of which was a label, a span and a bare input reimplementing what TextInput already does, including the focus ring the dark theme needed. The file picker keeps its hidden input, but the affordance is now a real SecondaryButton that opens it rather than a span styled to look like one. The accessible name still comes from the input, so nothing changes for a screen reader; what changes is that there is no second, slightly-different button style in the product. It also announces itself now. An import is the one flow here that changes on its own for minutes, and every one of those changes was conveyed by sight alone — the step highlight moves, a bar fills, a report appears — so after pressing Upload a screen reader user was told nothing at all. Announcements are keyed on the step rather than the job: the job is polled every couple of seconds and its progress counter moves constantly, which would talk over the rest of the page. The terminal announcement carries the outcome, because finishing with things to review is the common case and is not the same news as finishing clean. Deliberately not done: a toast when an import finishes. The wizard unmounts when you navigate away, and the polling lives in it, so a toast fired from here can only appear while you are already looking at the result. Telling someone their import finished after they left needs a watcher that outlives the route, which is its own piece of work rather than a component swap. 553 webapp tests across 76 suites; the three announcement tests fail without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps left over from the plan, both small. Link counts (§16.5, §20.1, §771): both report summaries have carried a `links` block since the models were written and nothing ever filled it in, so every report stated zero same-source links, zero unresolved and zero file placeholders for bundles full of them. The importer measures all three during inspection, so they now travel on the bundle summary — links are a property of what the bundle contained, which executing it does not change — into the preflight summary, the final summary and both report `counts.links` maps. The plan also describes cross_source_unique and ambiguous counts. Those need a placeholder resolved against the durable mappings of *other* ImportSources in the same Space (§14, §1067) with its own issue codes, which is not this change, so those two categories are absent from the report rather than present and always zero. A count of zero has to mean "none found", or none of them can be trusted. WebSocket updates (Phase 6 item 3): the worker has always published an actor-scoped import_job_updated; nothing listened. The webapp now registers a handler at init — a plugin gets one chance to do that — feeding a module transport the polling hook subscribes to, mirroring how page presence already works. The event is treated as a nudge to read, never as the new state: it carries five fields where the job view carries the plan, the counts and the acknowledgements, and it is best-effort at most once per second, so applying it directly would build a partial view that a single dropped event leaves wrong. It re-enters the polling loop early instead, which also means the cadence is recomputed from what the read finds and a terminal job stops the timer. Polling remains the correctness floor; this removes the wait — a published plan no longer sits unseen for up to fifteen seconds. 557 webapp tests across 76 suites, server suite green, golangci-lint clean. Both changes verified by reverting them: five tests fail without the subscription, and the link-count test fails without the wiring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Split into a reviewable stackThis branch is +27,772 across 79 files, which GitHub will not serve as a diff at all ( Nothing was lost in the split:
#30 is deliberately outside the stack: the Why not review this PR commit-by-commitThe 29 commits alternate feature work with five rounds of Every layer was verified independently
This PR stays openNot as something to merge — as the design narrative. The rationale, the accepted risks (single-node HA, authorship trust), and the deliberately-out-of-scope list are all here and don't survive being split six ways. Each layer links back to it. Two notes
🤖 Generated with Claude Code |
Imports mmetl-produced Confluence v2 bundles into Docs Spaces and pages, from upload through a reviewed plan to a durable report. 23 commits, ~27k lines including tests.
Supersedes #9, which holds an earlier branch with only phases 1–2. This branch is rebased onto master and contains that work plus everything since; #9 can be closed once this is reviewed.
What a user does
Design decisions worth reviewing
Testing
Server suite green across all packages (
api,app,store,importer,model);golangci-lint0 issues. 168 webapp tests across 28 suites;tsc --noEmitandeslint --quietclean; i18n extracts with no empty strings.Four rounds of external review are folded in (13, 10, 8, 10, then 9 findings). Every fix has a test, and each was verified by reverting the fix and confirming the test fails — including two regressions this branch introduced and then closed: a retry cooldown that broke same-source serialization, and a ZIP64 locator read that missed archives with long comments.
Deliberately not in this PR
import_job_updatedas a latency layer over the polling. Polling is the correctness floor and works alone.listImportJobsalready backs a list if one is wanted.Two accepted risks
URL convention decided here
Every segment naming something other than content now begins with an underscore:
_drafts(renamed fromdrafts) and_import. A space or page id can never start with one, so these cannot collide with anything a user names — no reserved-word list to keep in step with the routes, and no migration if custom slugs arrive later.RESERVED_SEGMENTSplusrouting/paths.test.tsmake it mechanical: every entry is asserted unmatchable as an id, so a segment added without the underscore fails.The bare-word form is safe only circumstantially.
draftsgot away with it by arity — no content route has three segments afterspaces, so a page nameddraftsstayed reachable — but a two-segment reserved word sits exactly where a page id goes and hides any page addressed that way.#12's
overviewis that second case (DOCS_SPACE_OVERVIEW_ROUTE, matched ahead of the page route). It is harmless on that branch, because #12 also removes user-chosen space slugs — butpaths.tsstill advertises "a human-readable custom slug" as a supported id form, so it becomes a live bug the day slugs return. Renaming it_overviewthere would put both PRs on one rule.Note for #12
webapp/src/client/rest.tsis taken byte-identically from #12 so there is one HTTP layer rather than two. Whichever merges second should drop its copy. Separately, #12'srequestreads the error id from the top level, soserver_error_idis empty for every Docs 409 — including its own draft-publish conflict.🤖 Generated with Claude Code