✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

✂️ Make xmd plan produce approved XMD source only (#724) - #749

Merged
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only
Sep 3, 2026
Merged

✂️ Make xmd plan produce approved XMD source only (#724)#749
taras merged 3 commits into
mainfrom
agent/issue-724-plan-source-only

Conversation

@taras

@tarastaras commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes#724.

Why

xmd plan did two jobs. It turned a request into a reviewed program, and it
could then run that program. A caller who wanted the artifact kept had to
remember which half of the grammar configured which job, and a caller who only
wanted source still paid for a command line describing an execution — a journal,
a permission mode, exec and fetch deadlines, and the generated root properties of
a document that did not exist yet.

Splitting them makes the relationship explicit and lets the caller decide when a
planned program runs.

What changes

Before:

$ xmd plan ""# source on stdout
$ xmd plan "" --output release.md # source in a file
$ xmd plan "" --run # writes a Plan, then runs it
$ xmd plan "" --run --journal t.jsonl --deny-all --props-name Ada

After:

$ xmd plan "Prepare the release program."
$ xmd plan "Prepare the release program."| xmd run -
$ xmd plan "Prepare the release program." --output release.md && xmd run release.md
Plan produces a program.
Run executes a program from the host/CLI.
Composition decides whether and when a planned program runs.

xmd plan executes exactly one root — the packaged plan command document — and
starts no program on any path. Without --output, stdout carries the approved
source and nothing else; with it, the path is created exclusively after approval,
teardown and validation, an existing path is left untouched, and stdout stays
empty.

The grammar keeps one request, --include, --agent-provider,
--default-agent, --session, --timeout, --output, and ordinary --help
and --version. Every option that configured the former execution is refused by
name before anything is read:

$ xmd plan "" --runxmd plan --run was removed because xmd plan only produces approved source.Run the program explicitly: xmd plan "..." | xmd run - xmd plan "..." --output release.md && xmd run release.md
$ xmd plan "" --journal trace.jsonlunrecognized option for xmd plan: --journal — configure the program when you run the approved source with xmd run

The same options are unchanged under xmd run.

How it works

fixed preflight → syntax catalog → the packaged plan command document
→ <Plan>: authorship frame, one Session, draft/repair/review/approve
→ complete teardown → the host's own structural check → stdout or --output

One invocation settles one structural check —
validateDocumentStructure under the ordinary run-profile registry, the
<plan> identity, the caller's ordered includes and the run profile's
declarations — and hands that same function to three places: <CheckDraft>
answers a draft with it, <AdmitPlan> admits the approved bytes with it, and
the host asks it once more after the command document has settled. What the
three can disagree about is when it was asked, not what: a component the
approved Plan names, removed after a successful admission, is caught by the last
one and nowhere else.

Validation is structural on purpose. A Plan whose root declares required
properties is a Plan; the values belong to whoever runs it later, and xmd plan
has no property source to resolve them from.

Review guide

Start with:specs/plan-command-spec.md — rewritten around the single
source artifact, in comprehension order.

Then review:

  1. packages/cli/src/plan-args.ts — the complete fixed grammar, both approved
    refusals, and removedPlanOption().
  2. packages/cli/src/plan-component.tsStructuralValidation,
    structuralValidation(), and the two privates that now share it.
  3. packages/cli/src/plan.ts — one root, one gate, two sinks.
  4. packages/cli/src/cli.tsplanConfig, the help epilogue, the preflight
    ahead of the help short-circuit, and the removal of planExecutor.

Look carefully at:

  • The ordering in plan.test.ts C10: draft check → review → authorship teardown
    → admission → host gate. That order is the reason the last check exists.
  • packages/cli/src/agent-stack.tsAgentStack split into AuthorshipStack
    plus the permission mode a run installs.

What must stay true

  • Planning never runs the approved program. Enforced by there being no
    execution capability to reach — no PlanExecution, PlanExecutionConfig,
    planExecutor, PlanDependencies.execute or second root — and checked by
    PS6/PS9/PS10, whose approved program writes a file and then fails.
  • The approved bytes are exact. Enforced by exact: true on the declaration
    and a single process.stdout.write, checked by PS6 (one chunk, byte for byte)
    and PS7 (the same bytes in the file).
  • A required root property is not a defect. Enforced by
    validateDocumentStructure, checked by PS8 and plan.test.ts C7.
  • The host's gate is a separate moment, not a separate contract. Enforced by
    the one injected StructuralValidation, checked by C10 and PS9 — deleting the
    gate fails both.
  • xmd run is untouched. Checked by PS5 and by the existing run suites.
  • The authorship ceiling is the host's. No permission mode reaches planning;
    AuthorshipStack has no field for one, checked by plan.test.ts C5.

How to verify it

deno task test \
packages/cli/tests/plan-args.test.ts packages/cli/tests/plan-cli.test.ts \
packages/cli/tests/plan.test.ts packages/cli/tests/plan-command-document.test.ts \
packages/cli/tests/plan-component.test.ts packages/cli/tests/syntax-cli.test.ts \
packages/cli/tests/packaged-document.test.ts
  • PS2/PS3 prove every removed spelling returns its exact approved refusal
    from a real subprocess whose agent name resolves to nothing and whose HOME is
    empty, and fail if a refusal arrives after a catalog, a provider or a session
    directory — or if it never arrives because --help short-circuited first.
  • PS4/PS5 pin the complete xmd plan --help output and fail if a removed
    option returns to it, or if splitting the grammar narrowed xmd run.
  • PS6 approves a program that writes ran.txt and then exits 3; it proves
    non-execution and fails if anything interprets the source — success plus the
    exact bytes plus neither observation is the whole claim.
  • PS7 observes the --output path from inside the authorship frame's own
    teardown and fails if the file is created early or truncated.
  • PS8 produces a Plan declaring a required root prop with no value supplied,
    and fails if full root-prop validation replaces the structural check.
  • PS9 / plan.test.ts C10 let the draft check and <AdmitPlan> really
    validate and succeed, move the tree immediately after that admission, and watch
    the host refuse. Both fail if the host's final validation is deleted —
    independently confirmed.
  • PS12 fails if any shipped page names --run other than to say it is gone.

Also run locally, all green: deno task check, deno task lint,
git diff --check, and deno task test --changed=origin/main
(43 passed, 284 steps, 0 failed) — which includes
scripts/tests/cli-npm-bin.test.ts, where the emitted npm package answers
xmd plan --run with the migration before an Agent is needed, and
scripts/tests/plan-component-compiled.test.ts after deno task build, where
the compiled binary ships the same <Plan> bytes and the source-only help.

Scope

Included

  • xmd plan produces approved source only; the execution capability is removed
    structurally.
  • The planning-only grammar, both approved refusals, and the revised help.
  • One shared structural check across the draft check, the admission and the host
    gate.
  • specs/plan-command-spec.md, architecture.md,
    specs/executable-mdx-spec.md, specs/root-document-props-spec.md,
    specs/acp-client-spec.md, README.md and the homepage.

Intentionally unchanged

New abstractions

  • StructuralValidation and structuralValidation()
    (packages/cli/src/plan-component.ts) exist because three call sites had to
    ask one question of one environment: <CheckDraft>, <AdmitPlan> and the
    command's own gate. Three consumers in production, and it is the seam the
    final-gate evidence needs.

  • AuthorshipStack (packages/cli/src/agent-stack.ts) exists because planning
    settles a provider, a default agent and adapters and no permission mode;
    AgentStack extends it for xmd run. Two consumers.

  • AuthorshipFlags (packages/cli/src/agent-config.ts) is the same split at the
    flag layer.

  • removedPlanOption() (packages/cli/src/plan-args.ts) exists because the
    removal has to be decidable before --help short-circuits the dispatch, and
    it shares its classification with scanPlanArgs so a spelling cannot be
    removed to one and unknown to the other.

  • Each new abstraction has multiple concrete uses or a clear justification.

  • No speculative functionality is included.

Generated or mechanical changes

  • packages/cli/tests/plan-cli.test.ts and packages/cli/tests/plan-args.test.ts
    are largely rewritten around the PS rows rather than edited; read them as new
    files.
  • The deletions in packages/cli/src/plan.ts and plan-args.ts are the removed
    execution and property-binding plumbing and carry no behaviour of their own.

Risks and limitations

  • --timeout expiry still reports "the run exceeded its --timeout of Nms and
    was cancelled". The wording is deliberately unchanged, but "the run" now names
    something this command does not have.
  • xmd test and xmd workflow refuse agent and timeout options with a sentence
    naming where each belongs. --agent-provider, --default-agent and
    --timeout still belong to both commands; the permission flags and the
    exec/fetch deadlines are now xmd run's alone, so a small belongsTo() helper
    answers per flag rather than one sentence that had become half untrue.
  • Rollback: revert the two commits. Nothing persists across an invocation, and
    no artifact format changed.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

`xmd plan` mapped one request to a reviewed program and could then run it.
Two responsibilities in one command meant a caller who wanted the program
kept had to remember which flags configured which half, and a caller who
only wanted source still paid for a grammar describing an execution.
It now produces the artifact and stops. One invocation executes exactly one
root — the packaged plan command document — awaits its complete teardown,
structurally revalidates the returned bytes, and delivers them to stdout or
to an exclusively created `--output` path. Whether that program runs is the
caller's composition: `xmd plan … | xmd run -`, or a saved artifact a later
`xmd run` names.
The execution capability is gone structurally rather than left unselected:
`PlanCommand.run`, `PlanExecution`, `PlanExecutionConfig`, the
`PlanDependencies.execute` callback, `planExecutor`, the final
`retainedSource("<plan>")` run, and the command-side property binding,
signature freezing and reserved-token plumbing all had no consumer left.
The final host gate stays, and is now structural — the same contract
`<AdmitPlan>` applies — so a Plan declaring required root props is admitted
rather than refused for arguments nobody has offered it yet.
The grammar keeps one request, `--include`, `--agent-provider`,
`--default-agent`, `--session`, `--timeout`, `--output`, and ordinary help
and version. Every option that configured the former execution is refused by
name in fixed preflight, before the general parser can drop or coerce a
token: `--run` reports the migration naming both compositions, and each of
the rest reports that `xmd run` is where a program is configured. Authorship
now settles a provider and a default agent and no permission mode at all.
The same options are unchanged under `xmd run`.
Four blockers from the #724 feedback review.
The command validated approved bytes twice, but nothing could tell the two
apart: `<AdmitPlan>` and the host's gate reached the same conclusion from
separately built inputs, so the C10 that removed a component during teardown
was answered by the first of them and deleting the second left every suite
green. The invocation now settles one structural check — `validateDocumentStructure`
under the ordinary run-profile registry, the `<plan>` identity, the caller's
includes and the run profile's declarations — and hands that one to
`<CheckDraft>`, to `<AdmitPlan>` and to itself. What the three can disagree
about is when it was asked, which is the whole point of keeping the last one:
C10 and PS9 now let both Component gates really validate and succeed, move the
tree immediately after that admission, and watch the host refuse. Deleting the
host's gate fails both.
`AuthorshipProfile.installElicitation` and `.assess` had no consumer and are
gone; the dependencies `planComponentDeclaration` captures are unchanged.
`--help` is lifted out of argv before any command's own grammar runs, so
`xmd plan --help --run` printed a page describing a command that would have
refused the caller. A shared pure `removedPlanOption()` — the same
classification the scan uses — answers first, in either order. Help that names
no removed option is still the exact approved help.
Generated-property recognition was a prefix test, so `--propspective`,
`--no-propspective` and a bare `--no-props` were told to configure their
program with `xmd run`. It is now exactly `--props`, `--props-*` and
`--no-props-*`; everything else keeps the generic unknown-option refusal.
The three session-directory diagnostics are restored to their base wording —
they were never part of the approved source-only copy. Stale prose that still
had an approved Plan running or receiving the ordinary run stack is corrected
in `agent-stack.ts`, the repository-provider comment in `cli.ts`, and
`plan.test.ts`.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 13 redundant comments. Inline suggestions to remove them below.

// leases on the checkouts this document selects, and the evidence of what it
// published. `xmd run` and an approved `xmd plan --run` supply the live one;
// published. `xmd run` supplies the live one; `xmd plan` executes no document
// of a caller's and reaches this line for none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// of a caller's and reaches this line for none.

// is lifted out of argv early enough that a command's own grammar never sees
// the invocation it was written on, so a Plan command line naming a removed
// option would be answered with a page describing a command that would refuse
// it. It is refused here instead, in either order, having read nothing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// it. It is refused here instead, in either order, having read nothing.

// The rest of what `xmd plan` decides on its own — cardinality, an unknown
// option, an empty session — answered before the shared checks below could
// report one of them as something else. The removed options were answered
// above, ahead of help.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// above, ahead of help.

},
// Who writes, and nothing else. There is no permission mode to settle:
// this command starts no program, and the ceiling authorship runs under
// is the host's rather than the command line's.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// is the host's rather than the command line's.

// The two commands a `--timeout` bounds. `xmd plan`'s deadline encloses
// something different from a run's — the catalog, the assistant session,
// every repair, the human review, provider teardown, final validation and the
// artifact — and covers no later program, because it starts none.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// artifact — and covers no later program, because it starts none.

// Built before the declaration exists, and handed to it: the packaged `<Plan>`
// description is the declaration an ordinary run resolves, so what the draft
// check, the admission and the gate below all ask about is the profile the
// approved program would actually run in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// approved program would actually run in.

// about a candidate — and however recently `<AdmitPlan>` concluded it — these
// are the bytes a later `xmd run` would execute, and the tree they resolve
// against has had a whole teardown to move since. So they are checked once
// more, as though nothing had ever validated them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// more, as though nothing had ever validated them.

// wrote it — no fence, no heading, no trailing newline of this command's —
// so a caller can pipe it into `xmd run -`, a file, a diff or another
// program. A caller who named `--output` already has it, and gets a quiet
// command instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// command instead.

export const AGENT = "scripted-agent";

/** The answer a case that is not about validation wants: this is a program. */
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

// The deterministic seam standing where production's answer goes, recording
// every candidate it was asked about — the draft check's and the
// admission's alike, which is every time these bytes are decided on.
// deno-lint-ignore require-yield

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// deno-lint-ignore require-yield

@github-actions

github-actionsBot commented Sep 3, 2026

Copy link
Copy Markdown

PR #749: ✂️ Make xmd plan produce approved XMD source only (#724)

29 files, +1966 / -2562

Scope

🔴 PR has 4528 lines changed. Split into focused PRs.

🟡 4528 lines changed. PRs under 400 receive more thorough review.

🟡 29 files changed. Are all changes related?

🟡 Changes span 8 directories.

Structural

Oxlint structural signals:

  • no-unnecessary-type-arguments ×10: packages/cli/src/props.ts
  • no-unused-vars ×8: packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
  • no-empty-function ×1: packages/cli/src/cli.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 42 diagnostics across 6 files (12 rules)
Density: 0.021 violations/added-line

no-unnecessary-type-arguments (10): packages/cli/src/props.ts
no-unused-vars (8): packages/cli/src/plan-component.ts, packages/cli/src/authorship-profile.ts, packages/cli/src/plan.ts (+2)
unbound-method (7): packages/cli/src/plan.ts, packages/cli/tests/support/plan-harness.ts, packages/cli/src/plan-component.ts (+1)
no-base-to-string (5): packages/cli/src/plan-component.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/src/plan-component.ts, packages/cli/src/cli.ts
consistent-function-scoping (1): packages/cli/src/authorship-profile.ts
no-empty-function (1): packages/cli/src/cli.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-array-sort (1): packages/cli/src/props.ts
no-floating-promises (1): packages/cli/src/cli.ts
consistent-return (1): packages/cli/src/cli.ts

Correctness

FILE: packages/cli/src/plan-component.ts, PATTERN: no-unused-vars, CONCERN: Unused variables or functions may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/agent-stack.ts, PATTERN: no-unused-vars, CONCERN: Unused variables may indicate unreviewed generated code, QUESTION: Are these unused symbols intentional or remnants of previous implementations?
FILE: packages/cli/src/plan-component.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan.ts, PATTERN: unbound-method, CONCERN: Unbound methods may indicate unnecessary indirection, QUESTION: Is the method binding necessary, or could it be simplified?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan.ts, PATTERN: no-unnecessary-type-arguments, CONCERN: Redundant type arguments may indicate overly explicit typing, QUESTION: Can type inference simplify these declarations?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?
FILE: packages/cli/src/plan-component.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Are these assertions necessary, or can the type system be adjusted?

No extraneous code patterns detected.

Two CI-only failures, both from checks that do not run in `deno task check`.
`tsconfig.node.json` targets ES2022 and Deno's own lib is newer, so
`Object.keys(scan).toSorted()` passed locally and failed all seven `test-node`
shards with TS2550. The assertion is about which fields the scan established,
not about their order, so it compares the two directions of set membership
instead of sorting — `unicorn(no-array-sort)` would only have suggested
`toSorted` back.
`site/` runs its own `deno task check`, which begins with `deno fmt --check`;
`oxfmt` formats `packages/` and never saw the homepage paragraph. Reflowed by
`deno fmt`, leaving the sentence PS12 pins on one line.
@taras
taras marked this pull request as ready for review September 3, 2026 11:05
@taras
taras merged commit d820e8f into mainSep 3, 2026
38 checks passed
@taras
taras deleted the agent/issue-724-plan-source-only branch September 3, 2026 11:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xmd plan produce approved XMD source only

1 participant

@taras