Skip to content

💥 Namespace Markdown props under the props binding - #372

Merged
taras merged 7 commits into
mainfrom
agent/namespace-markdown-props
Aug 8, 2026
Merged

💥 Namespace Markdown props under the props binding#372
taras merged 7 commits into
mainfrom
agent/namespace-markdown-props

Conversation

@taras

@taras taras commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Why

Issue #305 changes Markdown props from implicit bare bindings to an explicit props namespace. Validated props are available consistently across root documents, Markdown components, text interpolation, eval blocks, executable blocks, and projected content.

Closes #305.

What changes

BREAKING:

Before:

  • A declared prop such as name created both {name} and {props.name} access.
  • Root and Markdown-component environments spread prop fields into ordinary bindings.

After:

  • Validated and defaulted props are installed as one env.values.props object.
  • Markdown uses props.name, including {props.name} interpolation.
  • Declaring name no longer creates a bare name binding.
  • Eval-, capture-, loop-, and component-created locals remain ordinary bare bindings and can coexist independently with props.name.
  • A literal binding named props follows normal shadowing and restoration rules in text, eval, and executable interpolation.

The exact validated object is installed without another clone or freeze. Projected content keeps the caller's lexical props object; component-authored content uses the component's validated props. Existing ordinary projected-binding lookup is preserved.

How it works

validated props → props environment → text/eval/executable interpolation and projection

The review and repository-analysis documents also normalize Oxlint data before it reaches the durable stream. Compatibility probes emit aggregate facts, diagnostic payloads retain only fields consumed by parseDiagnostics, and PR review diagnostics are limited to changed files.

Review workflow hardening

.github/workflows/review.yml and .github/workflows/repo-analysis.yml now execute the checked-out revision:

  1. Install the repository-pinned Deno version.
  2. Run deno task deps.
  3. Run deno task build.
  4. Invoke ./dist/xmd.

They keep secret detection enabled. Credentials are created inside non-serializable header factories, and diagnostic data is bounded before persistence. After XMD runs, each workflow fails closed unless the root journal close exists, is successful (including nested output status), and contains no <!-- ERROR: marker. Journal upload remains under if: always().

The review correctness prompt also bounds diffPreview to 40,000 characters so large reviews remain within the provider context limit.

Review guide

Start with: packages/core/src/eval-env.ts, packages/core/src/expand.ts, and packages/core/tests/props-binding.test.ts

Then review:

  1. Shared props environment construction and projection layering.
  2. Root and Markdown-component environment installation.
  3. Text/eval/executable interpolation using the same current binding.
  4. Caller-versus-authored projection frames.
  5. Review/Oxlint normalization and workflow postconditions.
  6. Portable integration and regression coverage.
  7. Mechanical component, fixture, and specification migrations.

What must stay true

  • Defaults and validation complete before root or Markdown-component body effects.
  • Invalid props expose no partial namespace and begin no body effect.
  • Function components still receive definition.fn(validatedProps) with unchanged generator arguments.
  • Existing missing-value, dotted-path, array-formatting, and escaping behavior remains intact.
  • Secret detection remains enabled for review and repository analysis.
  • WorkflowRun, Workspace, SQLite, replay, Git, Agent, and xmd workflow lifecycle code remain outside this PR.

How to verify it

  • Root and component tests cover namespaced text, direct eval, expression props, executable interpolation, defaults, invalid props, shadow/restoration, projection, and function-component identity.
  • The review workflow verifies a real root close and rejects missing, unsuccessful, or error-marked XMD output.
  • The latest PR Review comment is a real review and contains no <!-- ERROR: marker.

Scope

Included

  • Core props environment, interpolation, and projection behavior.
  • Bundled Markdown components, review components/policies, smoke fixtures, and affected tests.
  • Root/component prop specifications, interpolation language, workflow bootstrap and fail-closed documentation, conformance rows, and decision records.

Intentionally unchanged

  • Function-component generator arguments.
  • WorkflowRun, Workspace restoration, SQLite, replay, Git, Agent, and xmd workflow lifecycle code.
  • PR Document adversarial implementation workflow WIP #181 and its later mechanical migration.
  • Compatibility fallback syntax and secret-detection disabling.

New dependencies

None.

Generated or mechanical changes

Review components/policies, smoke fixtures, bundled providers, and embedded examples contain mechanical prop.nameprops.name migrations. No changeset or version bump is required.

Risks and limitations

This is an intentional breaking language change; consumers using bare declared props must migrate to props.name.

Verification

Latest pushed head: 5d6da5c.

GitHub checks all pass, including review, lint, jsr, test-deno, test-node, test-bun, smoke, site, and deploy/taras/executablemd. composability is intentionally skipped for this pull request.

In a fresh prepared clone:

  • deno task lint — pass, 0 errors (1,227 existing warnings).
  • deno task check — pass.
  • deno task test — pass, 383 passed and 0 failed.
  • deno task check:jsr — pass, Success Dry run complete.
  • deno task test:bun — pass, 2,487 passed and 0 failed.
  • deno task test:node — one failure in packages/core/tests/config-api.test.ts, the known asynchronous-teardown issue tracked by 💥 Stabilize Node runtime test flake in config-api CF7 #371; the GitHub test-node check passes.
  • git diff --check — pass.

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.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

PR #372: 💥 Namespace Markdown props under the props binding

71 files, +1542 / -562

Scope

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

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

🟡 71 files changed. Are all changes related?

Structural

✅ No structural bloat detected.

Slop

  • packages/core/src/expand.ts:460// the caller frame; only the resource scope moves.
  • packages/core/src/expand.ts:1915// layering helper keeps a projected caller's props object lexical.
  • packages/core/src/expand.ts:2411// expansion while retaining the projected caller's props namespace.

Static Analysis

✅ Oxlint found no issues.

Correctness

No extraneous code patterns detected.

@taras
taras marked this pull request as ready for review August 7, 2026 13:22

@github-actions github-actions Bot 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 3 redundant comments. Inline suggestions to remove them below.

// context env so that ancestor bindings propagate through all levels.
// The current context env's bindings take precedence (innermost-wins).
// The current context env's ordinary bindings take precedence; the shared
// layering helper keeps a projected caller's props object lexical.

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
// layering helper keeps a projected caller's props object lexical.

// the lexical caller's bindings under the current component's, so what
// it reads is what its author wrote beside it.
// Projection follows the same ordinary-binding layering as Markdown
// expansion while retaining the projected caller's props namespace.

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
// expansion while retaining the projected caller's props namespace.

Comment thread specs/executable-mdx-spec.md Outdated
```ts eval
// Install Sample Api middleware on the current component scope.
// baseUrl and model are closed over here — no context lookup at call time.
// baseUrl and props.model are closed over here — no context lookup at call time.

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
// baseUrl and props.model are closed over here — no context lookup at call time.

@taras
taras force-pushed the agent/namespace-markdown-props branch from 5d6da5c to 52c714f Compare August 7, 2026 18:25
@taras
taras force-pushed the agent/namespace-markdown-props branch from 52c714f to 5c927b8 Compare August 8, 2026 23:32

@github-actions github-actions Bot 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 3 redundant comments. Inline suggestions to remove them below.

// Slots were resolved during substitution, so the environment, meta,
// props and hide set are the body's own — only the resource scope moves.
// Slots were resolved during substitution. The projected content keeps
// the caller frame; only the resource scope moves.

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
// the caller frame; only the resource scope moves.

// context env so that ancestor bindings propagate through all levels.
// The current context env's bindings take precedence (innermost-wins).
// The current context env's ordinary bindings take precedence; the shared
// layering helper keeps a projected caller's props object lexical.

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
// layering helper keeps a projected caller's props object lexical.

// the lexical caller's bindings under the current component's, so what
// it reads is what its author wrote beside it.
// Projection follows the same ordinary-binding layering as Markdown
// expansion while retaining the projected caller's props namespace.

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
// expansion while retaining the projected caller's props namespace.

@taras
taras merged commit 91a8b57 into main Aug 8, 2026
11 checks passed
@taras
taras deleted the agent/namespace-markdown-props branch August 8, 2026 23:40
@taras taras mentioned this pull request Aug 9, 2026
4 tasks
Sign up for free to 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.

Namespace Markdown props under the props binding

1 participant