✨ Let a workflow definition name one section of its document - #431
Draft
taras wants to merge 2 commits into
Draft
Conversation
`site:check` and `site:build` run concurrently under the verifier, and Vite writes a transient `vite.config.ts.timestamp-*.mjs` beside the config while it loads it. A `deno lint .` that walked one reported `no-var` on generated code nobody wrote, failing the battery for a file that no longer existed by the time anyone looked. The shim is now excluded from the site's own checks and ignored by Git. Either alone silences it; both are kept because the exclusion states the checker's scope and the ignore keeps the file from being committed.
A workflow definition identified a whole root document. It now optionally carries the exact canonical document target the run is a run of, so a workflow can be a run of one section. `targetPath` is the resolved exact target, never the selector a caller wrote: a glob describes what somebody asked for, and re-resolving one against a different checkout can name a different section. Absent, the definition means the complete document — which is what a whole-document workflow is, not a legacy spelling. What counts as canonical is not restated in the workflow package. Core's predicate is exported as `isCanonicalDocumentTarget` and used directly, because identity two packages define separately is identity they can disagree about. The member is closed like every other: writing it at all makes it present, so an explicit `undefined` or `null` is a descriptor that asked for a target and failed to name one. A refusal reports `$.targetPath` in fixed wording that never echoes what it read, since a canonical target encodes heading text. Compatible reuse compares it. A run of one section, a run of another, and a run of the whole document are three different runs, so reusing one run id for another reports a definition conflict; the same exact target is the same run and is found. `version` stays 1 and there is no second version, union, or migration.
| throw fail(`expected a string, found ${describe(value)}`, path); | ||
| } | ||
| // Deliberately says nothing about the target it read: a canonical target | ||
| // encodes heading text, and heading text is document content. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // encodes heading text, and heading text is document content. |
| rootDocumentPath: definition.rootDocumentPath, | ||
| // Written only when there is one. An untargeted definition that stored an | ||
| // explicit absence would parse back as a descriptor that asked for a target | ||
| // and failed to name it. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // and failed to name it. |
PR #431: ✨ Let a workflow definition name one section of its document11 files, +401 / -6 Scope🟡 407 lines changed. PRs under 400 receive more thorough review. 🟡 Changes span 7 directories. 🟡 PR mixes config and source changes. Structural✅ No structural bloat detected. Slop
Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
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.
Why
Issue #412 makes a root document's sections individually addressable. #421 built the core model, and #427 taught
xmd targetsandxmd runto use it. Neither could be persisted: a workflow definition identified a whole root document, so a workflow could not be a run of one section.This is the last layer. It amends the existing
GitWorkflowDefinitionV1in place — no version 2, no version union, no migration, no compatibility-only machinery.What changes
Before:
After — one optional member:
#.versionremains1as the schema tag.The five-member untargeted shape stays valid because it is the representation of a whole-document workflow, not a legacy format being preserved.
How it works
Canonical-target authority is not duplicated. Core's
isCanonicalTarget()— the predicate document references already use — is exported asisCanonicalDocumentTargetand called by the workflow parser. Identity that two packages define separately is identity they can disagree about, and this member is compared against targets the document layer produced.Presence is the member being written, not its value. A descriptor that wrote
targetPathand gave itundefinedornullasked for a target and failed to say which — refused, rather than read as the whole document.definitionToJson()writes the member only when there is one, so an untargeted definition round-trips to five members.Diagnostics say nothing about the target. A canonical target encodes heading text, and heading text is document content, so a refusal is fixed wording at
$.targetPath.Storage is unchanged. The definition already lives in a JSON column; there is no schema migration and no new table or column.
Review guide
Start with:
packages/workflow/tests/workflow-definition.test.ts(Tier WD, WD18–WD24)Then review:
packages/workflow/src/storage/definition.ts— the interface,parseTargetPath(), and serializationpackages/workflow/src/storage/compatibility.ts— one added comparisonpackages/core/mod.ts— the renamed public exportLook carefully at: presence-vs-value in
parseTargetPath().members.has()rather thanget() !== undefinedis what separates "no target" from "a target I failed to name".What must stay true
*,**, and embedded wildcards. Checked by WD21 and WD23.sameDefinition()comparingtargetPath, where absent equals only absent. Checked by WD24, WS30, WS32.How to verify it
targetPathkey.%2F,%2A,%23,%25,%20, and non-ASCII.$.targetPath— empty, leading#,*,**, embedded wildcard, malformed and lowercase escapes, leading/trailing/uncollapsed whitespace, empty levels, and an NFD spelling — with no echo.Mutation checks
targetPathremoved fromsameDefinition()targetPathomitted fromdefinitionToJson()Scope
Included
targetPathonGitWorkflowDefinitionV1: parsing, serialization, comparison.isCanonicalDocumentTargetexported from@executablemd/core.9940870): Vite's transientvite.config.ts.timestamp-*.mjsis excluded from the site's checks and ignored by Git.Intentionally unchanged
GitWorkflowDefinitionV2, version union, version dispatch, migration, or legacy conversion.xmd workflow start README.md#Targetis not shipped here. 🚀 Start and resume a workflow run from the CLI (#366 PR 2) #428 is the consumer.Risks and limitations
targetPathyet — this is the persistence capability, not its consumer.bdcb258) and 🚀 Start and resume a workflow run from the CLI (#366 PR 2) #428 (856fa25) both touch workflow architecture and specifications. Their branches are untouched by this PR and will need a rebase and a re-read ofspecs/workflow-spec.md§9.1–9.2,specs/workflow-workspace-spec.md§3, and thearchitecture.mdconstruct inventory.Scope confirmation
Base and verification
f57ae74, ✨ Address a document's sections from the command line #427 /b324b97.b324b974f821f742132c8ecf9785308fa816e403(origin/main).5d4a9db. Commits:9940870(site shim fix),5d4a9db(definition target).composabilityfailure was an intermittent instance of the very race9940870fixes; a re-run at the identical commit passed andci-main-redmainis red: CI failure at b324b97 #430 self-closed.deno task lintdeno task checkdeno task check:jsrSuccess Dry run completegit diff --checkdeno task test packages/workflow/tests/workflow-definition.test.ts packages/workflow/tests/workflow-run-storage.test.tsdeno task test --changed=origin/mainpnpm exec tsx --tsconfig tsconfig.node.json --test packages/workflow/tests/workflow-definition.test.tsbun test packages/workflow/tests/workflow-definition.test.ts(cd site && deno task check)with a shim presentworkflow-run-storage.test.tsstays excluded from Node and Bun by the existingnode:sqliteentry inscripts/runtime-test-exclusions.ts; no exclusion was added.The site fix carries a fail-first proof. With neither the ignore nor the exclusion,
deno lint .insite/reproduces the CI failure exactly (error[no-var],Found 1 problem); either mechanism alone silences it.