feat(artifact-format): pure structural artifact file format codec - #174
Open
stonexer wants to merge 7 commits into
Open
feat(artifact-format): pure structural artifact file format codec#174stonexer wants to merge 7 commits into
stonexer wants to merge 7 commits into
Conversation
…tter + Markdown) The canonical form of a Graph-Engineering-v3 artifact: ONE YAML front-matter block (the machine head) plus a Markdown body. Sanitized HTML is a projection of that, never storage. New pure package `@loopany/artifact-format` — no I/O, no server internals, not yet wired into server or UI. - Parse: strict split (only the FIRST closing `---` closes the head, so a `---` in the body is inert, not an injection point) + YAML 1.2 CORE schema (no !!timestamp coercion, no YAML 1.1 booleans, duplicate keys and unresolved tags rejected, alias expansion capped). There is no lenient path: a file that opens a front-matter block and then malforms it is a typed ArtifactFormatError, never a document that silently becomes "all body". Size/depth/node ceilings fail loudly rather than clipping. - Core schema validates type/status/title/format/source/externalId/sourceUrl/ createdAt/updatedAt/attachments and PRESERVES every unknown field, so the type registry can add per-type fields without this library changing. An explicit unknown `format:` is an error, not a silent fallback to markdown. - Serialize is a pure function of the data: core fields in declared order, all other keys lexicographic at every depth, body bytes exact. parse(serialize(x)) deep-equals x and the bytes are stable under repetition. - Render: Markdown (CommonMark + GFM) to sanitized, style-free semantic HTML. Two independent defenses — raw HTML is escaped (or dropped) before the HTML layer, and marked's output is then filtered through a strict sanitize-html allowlist — so a behavior change in either dependency cannot open a hole. GFM column alignment rides as data-align rather than a presentational attribute. - 73 tests: round-trip determinism, unknown-field preservation, status read/update/re-serialize with a byte-identical body, a hostile-input suite (script tags, raw HTML, event handlers, javascript:/data:/protocol-relative URLs, front-matter injection, alias bombs, deep/huge YAML) and typed malformed-file behavior. Wiring: root `pnpm test` + tsconfig references, and the Dockerfile now copies every workspace manifest into the install layer.
…odec Tests only — no implementation. Expresses the narrowed role: a pure format codec with ZERO domain knowledge. A red suite is the expected state of this stage; 48 of 158 already pass, and those are exactly the structural behaviors that survive the rewrite unchanged. What the suite pins: - round-trip laws (inverse, byte-stable under repetition, fixed point) and body byte-exactness across 14 body shapes (leading blank line, trailing newlines, CRLF, lone CR, empty, delimiter-looking lines, non-ASCII). - unknown-key pass-through as the CORRECT behavior rather than a fallback, including the keys v1 used to police (type/status/source/externalId/ createdAt/attachments now carry no rules at all) and prototype-shadowing keys landing as own properties. - determinism: insertion-order independence at every depth, code-unit rather than locale collation (with a guard proving the case discriminates), and the new parametrized `keyOrder` — listed keys first in order, rest lexicographic, default pure lexicographic, top-level only, absent keys skipped, duplicates collapsed, presentation-only. - every hostile-input ceiling failing loud, never clipping. - structural rejection: non-mapping heads, duplicate keys, host objects (Date/Map/Set/class/RegExp/function/symbol/bigint), with issue accumulation reporting every offending path in one error. - `format` as an enum only — markdown and html both legal, absence means markdown, anything else UNSUPPORTED_FORMAT; an html body stays opaque bytes. - the RFC 3339 helper surviving as an exported `checkTimestamp` applied to no key by the library, composing into a caller's own issue accumulation. - safeParseArtifact result semantics for batch ingress. - the ABSENCE of the render projection: export-surface pin plus a check that the markdown/sanitizer dependencies are gone. The v1 test files (parse/serialize/render) are removed rather than left alongside: they encode the superseded domain contract, so keeping them would make a red result unreadable — a reviewer could not tell "not implemented yet" from "deliberately deleted". They remain in history on fm/artifact-format-a1. test/target.ts is the executable statement of the target surface and lives outside src/ so the package tsconfig neither includes nor typechecks it.
The first draft asserted acceptance at maxFrontMatterDepth - 1 lists, which is one past the ceiling: depth counts from the root mapping, so N nested lists put the innermost scalar at N + 2. Pin the boundary from both directions so an off-by-one in either fails.
Implements the stage-2 target suite; all 158 cases green. The package survives
but its role narrows: it now knows that the head is a YAML mapping and that the
body is bytes, and nothing else.
- Domain schema OUT. `type`/`status`/`source`/`externalId`/`sourceUrl`/
`attachments`, the required-`type` rule and the source+externalId pairing are
gone, along with `ArtifactCoreFields`. Structural validation is now exactly
two rules: the head parses to a mapping, and a declared `format` is a known
enum. Every other key is preserved verbatim — pass-through is the CORRECT
behavior at this level, not a fallback, since an unknown key is not one the
codec failed to understand but one that is none of its business. Object kinds
and their closed key sets belong to the server seam.
- Key order is PARAMETRIZED. `CORE_FIELD_ORDER` is removed; no key is
privileged. Default is pure lexicographic at every depth, and
`serializeArtifact(doc, { keyOrder })` puts the named keys first. It applies
to the top level only, so a caller's intent for the head cannot reach down and
reorder a nested value that merely shares a key name; absent keys are skipped
rather than invented, duplicates keep their first position.
- Render module DELETED, with `marked` and `sanitize-html`. The body is opaque
text; a consumer that displays one renders it under its own policy. `yaml` is
now the entire dependency budget.
- `SUPPORTED_BODY_FORMATS` becomes markdown + html, validated as an enum with no
rendering semantics — which kinds may use html is server-side policy.
- The RFC 3339 checker survives as an exported `checkTimestamp(value, path)`,
applied to NO key by the library and returning an issue-or-null so it composes
straight into a seam's accumulation.
- Issues now ACCUMULATE with real paths (`nested.third`, `list[1]`): one pass
reports every unrepresentable value instead of marching the caller through
them one round trip at a time. Host objects (Date/Map/Set/class/RegExp/
function/symbol/bigint) are still rejected rather than flattened to `{}`.
Kept unchanged: the error-code system, `safeParseArtifact` result semantics, the
five hostile-input ceilings, byte-exact bodies, and the deterministic
code-unit (never locale) ordering.
README rewritten for the codec contract; the AGENTS.md entry now warns against
adding a field rule here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Stage 3 of a captain-directed rewrite: narrow @loopany/artifact-format from the v1 domain-aware format library into a PURE STRUCTURAL CODEC with zero domain knowledge, implementing a test suite that was authored and captain-approved in stage 2 on this same branch.
BACKGROUND THE DIFF DOES NOT SHOW. The platform design (Graph Engineering v3) was rewritten. The v1 package survives but its role narrows. This branch has three commits by design and that layering is deliberate, not accidental: (1) a tests-first commit authoring the target suite while src was untouched (a red suite was the expected and directed state), (2) a fix to one off-by-one in my own depth-boundary test case, (3) the implementation that turns the suite green. Do not flag the tests-before-implementation ordering as a mistake; it was the directive.
THE EIGHT DESIGN DECISIONS BELOW WERE EXPLICITLY REVIEWED AND APPROVED BY THE CAPTAIN before implementation, from a written test catalog. They are settled. Do not relitigate them:
WHAT CHANGED AND WHY:
Kept unchanged: the error-code system, safeParseArtifact result semantics for batch ingress, the five hostile-input ceilings (document bytes, front-matter bytes, depth, node count, alias/billion-laughs) each failing loud rather than clipping, byte-exact body preservation, and strict YAML 1.2 core-schema parsing (no timestamp coercion, no YAML 1.1 booleans, duplicate keys and unresolved tags rejected).
METHOD NOTE: before submitting the stage-2 suite for captain review I wrote a throwaway implementation of the target, confirmed the suite reached 158/158 against it, and then discarded it uncommitted - so the suite was known achievable rather than merely red. That verification pass is what caught the depth-boundary off-by-one fixed in commit 2.
VERIFIED BEFORE VALIDATION: pnpm -r typecheck green; full repo pnpm test green (845 server + 360 daemon + 158 artifact-format); a clean tsc build emits dist with no render module and the emitted JS smoke-tests end to end (correct export set, default lexicographic order, keyOrder honored, checkTimestamp both ways). The package remains unwired into server or UI by design - this branch ships the library only.
What Changed
packages/artifact-format(@loopany/artifact-format, private): a pure structural codec for artifact files — YAML front matter plus a byte-exact opaque body.parseArtifact/serializeArtifactare deterministic inverses; structural validation is exactly two rules (the head resolves to a YAML mapping, a declaredformatis one ofmarkdown/html) and every other key passes through verbatim, with object kinds and their key sets left to a future server-side seam. Parsing is strict YAML 1.2 core schema (no timestamp coercion, no YAML 1.1 booleans, duplicate keys and unresolved tags rejected); key order is pure lexicographic by code unit at every depth, with an optionalkeyOrderthat reorders the top-level mapping only.yamlis the whole dependency budget — the package renders nothing.safeParseArtifactfor batch ingress, path-carrying issue accumulation that reports every offending value in one error (host objects such asDate/Map/class instances are rejected, never flattened),checkTimestampreturning an issue ornull, andsplitArtifact/updateArtifactFrontMatter/bodyFormatOf/ARTIFACT_FORMAT_VERSION. The five ceilings (document bytes, front-matter bytes, depth, node count, alias expansion) fail loud rather than clip, and are now applied on the serialize path as well as on parse, so the library cannot emit a file it would refuse to read back;updateArtifactFrontMattertakes the caller's limits, andcheckTimestamprejects impossible calendar dates like2026-02-30thatDate.parsesilently rolls over.tsconfig.jsonproject reference, theDockerfileinstall layer now copies its manifest (pnpm silently skips an importer whosepackage.jsonis absent), rootpnpm testruns its suite, andREADME.md/CONTRIBUTING.md/AGENTS.mddescribe the third package. The package README owns the format contract, error codes and ceilings. 173 tests across 5 files cover round-trip, determinism, structure, limits and the exported surface — the last pinned againsttest/target.tsso an export cannot appear or vanish unnoticed. Nothing inserveror the UI imports it yet, by design.Risk Assessment
✅ Low: The round-1 warnings are fixed durably at the correct shared boundary (a single guardShape walk both directions call) with the round-trip law now provably unconditional, each behavior change is pinned by a new test, and the public export surface, build output and captain-approved design decisions are all untouched.
Testing
Ran the package's stage-2 suite (173 tests, 5 files) and its typecheck green, then produced the real product-level evidence by building the package with tsc and driving the emitted dist from a standalone Node consumer script resolved through the package's exports map - that transcript shows the 15-export surface with the render module and marked/sanitize-html gone, arbitrary front-matter keys passing through verbatim while format/mapping rejection still fires, keyOrder behaving per D2/D3 (nested same-name key untouched, absent key skipped, duplicate keeping first position, [] equivalent to omission), D4 issue accumulation reporting all eight host-object offenders in one error with real paths, all five ceilings failing loud on serialize as well as parse, strict YAML 1.2 behavior, and checkTimestamp/splitArtifact/updateArtifactFrontMatter/bodyFormatOf/safeParseArtifact intact. I also proved the locale-independence claim by serializing under tr_TR vs en_US for byte-identical output where localeCompare would reorder, and ran the Dockerfile's own frozen-lockfile install to confirm the new workspace package is wired correctly. No visual artifact applies - this change ships a headless library with no UI surface and is deliberately unwired from server or UI. Everything passed and the transient dist build output was removed, leaving the worktree clean.
Evidence: Consumer smoke transcript against the emitted dist (30 checks + locale determinism)
Evidence: Consumer smoke script (imports dist/index.js via the package exports map)
Evidence: Host-locale determinism: identical bytes under tr_TR vs en_US where localeCompare differs
Evidence: Zero domain knowledge + rejection moved, not removed (excerpt)
Evidence: A real editing session: parse → updateArtifactFrontMatter → serialize({keyOrder}) → re-parse
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
packages/artifact-format/src/serialize.ts:156- serializeArtifact enforces only maxFrontMatterDepth. canonicalize() checks depth (serialize.ts:93) and nothing else, and no other ceiling name appears anywhere in serialize.ts, so maxDocumentBytes, maxFrontMatterBytes and maxFrontMatterNodes are parse-side only. Failing sequence: serializeArtifact({frontMatter:{kind:'note'}, body:'y'.repeat(510241024)}) succeeds, and parseArtifact of that exact output throws DOCUMENT_TOO_LARGE; likewise serializeArtifact({frontMatter:{list:Array.from({length:6000},()=>1)}, body:''}) succeeds and re-parses as FRONT_MATTER_TOO_MANY_NODES, and a single 70KB string value re-parses as FRONT_MATTER_TOO_LARGE. Such a document cannot have come from parseArtifact (it would have been rejected), so this is reachable only from caller-built front matter - which is the normal write path for the server seam this library is being narrowed for, and the result is a file written to disk that the same library can never read back. This contradicts the inline claim on serialize.ts:158 ('Serializing never emits a file this library would refuse to read back') and the unconditional round-trip guarantee in README.md:107. Either apply the same resolved limits on the serialize side (byte-measure the emitted head and whole document, and count nodes during the canonicalize walk that already visits every node), or narrow both claims to say the ceilings are enforced on read only.packages/artifact-format/src/schema.ts:56- checkTimestamp accepts calendar dates that do not exist. The guard is!RFC3339.test(value) || Number.isNaN(Date.parse(value)), but V8 does not reject out-of-range days: verified with node that Date.parse('2026-02-30T00:00:00Z') returns 1772409600000 (silently rolled over to 2026-03-02), so checkTimestamp('2026-02-30T00:00:00Z', 'at') returns null, i.e. 'this is a well-formed instant'. Same for '2026-04-31T...'. A seam that accepts the value and later re-derives it from Date gets a different day than the file says - precisely the silent-ambiguity class the RFC3339 comment on schema.ts:20 says is a bug in a timestamp the engine schedules on. Fix inside the existing check by round-tripping the parsed instant against the source fields (e.g. compare new Date(parsed).toISOString() day/month against the matched groups) rather than trusting Date.parse for range validation. No existing case in codec.surface.test.ts is affected: neither the valid nor the invalid list contains a rolled-over date.packages/artifact-format/src/serialize.ts:203- updateArtifactFrontMatter validates through canonicalFrontMatter(merged, resolveLimits(undefined)), so it always uses DEFAULT_LIMITS and takes no options of its own. A caller that parses with {limits:{maxFrontMatterDepth:64}} (the pattern codec.limits.test.ts:86 exercises) and then edits a legitimately 20-level-deep head gets a spurious FRONT_MATTER_TOO_DEEP from the update, even though both parseArtifact and serializeArtifact accept that same document under the caller's own limits. Accept an optional ParseOptions parameter and thread it into resolveLimits so the three entry points agree on the effective ceilings.packages/artifact-format/src/serialize.ts:193- updateArtifactFrontMatter builds its merged head on Object.assign(Object.create(null), ...) and returns that object directly as the public frontMatter (serialize.ts:205). The null prototype is load-bearing while merging (it is what makes a 'proto' patch key land as an own property), but leaking it to the caller means the returned ArtifactDocument behaves differently from every other one the library hands out: doc.frontMatter.hasOwnProperty(k) throws TypeError, and any implicit string coercion of the object throws 'Cannot convert object to primitive value'. Restoring the ordinary prototype after the merge (Object.setPrototypeOf(merged, Object.prototype)) keeps the already-created own 'proto' data property intact while making the update path's return value interchangeable with the parse path's.packages/artifact-format/tsconfig.json:18- The package tsconfig includes only src//* and excludes src//*.test.ts, and test/ (target.ts, assert.ts) is outside the include entirely, sopnpm typecheckcovers none of the package's test code - roughly half its lines. That exclusion was correct at stage 2, when target.ts deliberately described a surface src did not implement yet, but stage 3 makes the surface real, so the exclusion now only hides type drift in the very files that pin the public surface. Consider a separate tsconfig.test.json (or widening include and keeping rootDir/exclude only for the emitting build) so the suite and target.ts typecheck without shipping into dist.🔧 Fix: enforce codec ceilings on serialize, reject impossible dates
2 infos still open:
packages/artifact-format/src/serialize.ts:243- updateArtifactFrontMatter now accepts options and threads them into resolveLimits, but it only runs canonicalFrontMatter - which enforces representability and (inside canonicalize, serialize.ts:93) the depth ceiling. It never calls guardShape, so maxFrontMatterNodes is not applied on the edit path: updateArtifactFrontMatter(doc, {list: Array.from({length:6000},()=>1)}, {limits:{maxFrontMatterNodes:10}}) returns normally. The new limits case at codec.limits.test.ts:206 pins that depth IS honoured here, which makes the node gap the odd one out - a caller passing limits gets one of the two shape ceilings silently ignored. Not a correctness hole (the subsequent serializeArtifact still refuses to emit it, so no unreadable file can result), only an eagerness/option-fidelity gap in the function whose doc comment at serialize.ts:212 promises validation happens at the edit rather than later at the save. One line closes it: guardShape(canonicalFrontMatter(merged, limits), limits), reusing the tree canonicalFrontMatter already returns and currently discards.packages/artifact-format/src/serialize.ts:185- The two byte ceilings are checked in the opposite order from parse. parse.ts checks maxDocumentBytes first (parse.ts:44) and maxFrontMatterBytes second (parse.ts:123); serialize checks the front-matter ceiling at serialize.ts:186 and the document ceiling at serialize.ts:195. For a document that violates both - reachable with stock defaults, e.g. a single 5MB string value in the head - parse reports DOCUMENT_TOO_LARGE while serialize reports FRONT_MATTER_TOO_LARGE for the same bytes. Both are loud and both use the shared vocabulary, so this does not weaken the round-trip law; it just means the code a caller sees depends on the direction. Computing the document total from the already-known parts (4 + fmBytes + 4 + Buffer.byteLength(doc.body)) lets the document check run first, exactly mirroring parse, and as a side benefit avoids materializing and flattening the oversizedtextat serialize.ts:193 before deciding to reject it.✅ **Test** - passed
✅ No issues found.
pnpm --filter @loopany/artifact-format test- 173 tests / 5 files green (codec.surface, codec.structure, codec.roundtrip, codec.determinism, codec.limits)pnpm --filter @loopany/artifact-format typecheck-tsc --noEmitplustsc -p tsconfig.test.jsonpnpm --filter @loopany/artifact-format build- clean tsc emit; verified dist contains errors/index/parse/schema/serialize/shape/types only, no render modulenode consumer-smoke.mjs <pkgDir>- end-to-end consumer script importing the emitteddist/index.jsvia the packageexportsmap; 30 checks covering export surface, removed v1 exports, dependency budget, domain pass-through, format/mapping rejection, keyOrder D2/D3, determinism, D4 path-carrying issue accumulation, all five ceilings on parse AND serialize, strict YAML 1.2, checkTimestamp D1 incl. 2026-02-30, splitArtifact/updateArtifactFrontMatter/bodyFormatOf/safeParseArtifact - exit 0LC_ALL=tr_TR.UTF-8 node locale-check.mjsvsLC_ALL=en_US.UTF-8- byte-identical serialization where localeCompare orders differently (code-unit ordering proven)pnpm install --frozen-lockfile --ignore-scripts- the Dockerfile install-layer command; lockfile up to date across all 4 workspace projectsgrep -rn 'marked|sanitize-html|renderMarkdown|CORE_FIELD_ORDER|ArtifactCoreFields' packages/artifact-format- only surviving hits are the deliberate REMOVED_* assertion lists in test/target.ts✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.