diff --git a/.github/workflows/repo-analysis.yml b/.github/workflows/repo-analysis.yml index ed3fc135..8520c9e5 100644 --- a/.github/workflows/repo-analysis.yml +++ b/.github/workflows/repo-analysis.yml @@ -29,13 +29,15 @@ jobs: ref: ${{ inputs.ref }} fetch-depth: 0 - # install.sh resolves the latest published release, for the same reason - # PR Review does — a pinned version names an unpublished release for the - # whole of a release-bump PR. See .github/workflows/review.yml. - - name: Install xmd release binary - run: | - curl -fsSL https://executable.md/install.sh | sh - echo "$HOME/.local/bin" >> "$GITHUB_PATH" + - uses: denoland/setup-deno@e95548e56dfa95d4e1a28d6f422fafe75c4c26fb # v2.0.3 + with: + deno-version: v2.9.5 + + - name: Install dependencies + run: deno task deps + + - name: Build the checked-out xmd binary + run: deno task build - name: Run repo analysis env: @@ -43,19 +45,12 @@ jobs: GITHUB_TOKEN: ${{ github.token }} GITHUB_REPOSITORY: ${{ github.repository }} run: | - xmd run .reviews/AnalyzeRepoCI.md \ + ./dist/xmd run .reviews/AnalyzeRepoCI.md \ --component-dir .reviews/components \ --component-dir .reviews/policies \ --component-dir packages/core/components \ -j .reviews/journal.analyze.ci.jsonl > .reviews/analyze-report.md - # xmd embeds a failed EnsureOxlint as a comment and still exits 0, so - # assert the lint binaries were provisioned — fail the job if not. - - name: Verify oxlint provisioning - run: | - test -x .reviews/.oxlint/oxlint && test -x .reviews/.oxlint/tsgolint \ - || { echo "::error::oxlint/tsgolint could not be provisioned by EnsureOxlint"; exit 1; } - - name: Write run metadata if: always() run: | diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index 57ab97cb..3abbda17 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -15,14 +15,15 @@ jobs: with: fetch-depth: 0 - # install.sh resolves the latest published release. Pinning a version here - # broke every release-bump PR, which pinned the release it was preparing - # and downloaded a binary that did not exist yet; the tradeoff is that a - # new release changes PR-review behavior as soon as it is the latest. - - name: Install xmd release binary - run: | - curl -fsSL https://executable.md/install.sh | sh - echo "$HOME/.local/bin" >> "$GITHUB_PATH" + - uses: denoland/setup-deno@e95548e56dfa95d4e1a28d6f422fafe75c4c26fb # v2.0.3 + with: + deno-version: v2.9.5 + + - name: Install dependencies + run: deno task deps + + - name: Build the checked-out xmd binary + run: deno task build - name: Run review env: @@ -34,20 +35,13 @@ jobs: GITHUB_REPOSITORY: ${{ github.repository }} DEEPINFRA_TOKEN: ${{ secrets.DEEPINFRA_TOKEN }} run: | - xmd run .reviews/ReviewPR.md \ + ./dist/xmd run .reviews/ReviewPR.md \ --component-dir .reviews/components \ --component-dir .reviews/policies \ --component-dir packages/core/components \ -j .reviews/journal.jsonl \ --verbose - # xmd embeds a failed EnsureOxlint as a comment and still exits 0, so - # assert the lint binaries were provisioned — fail the job if not. - - name: Verify oxlint provisioning - run: | - test -x .reviews/.oxlint/oxlint && test -x .reviews/.oxlint/tsgolint \ - || { echo "::error::oxlint/tsgolint could not be provisioned by EnsureOxlint"; exit 1; } - - name: Upload journal if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 diff --git a/.reviews/AnalyzeRepo.md b/.reviews/AnalyzeRepo.md index 0de780be..ed8e7ced 100644 --- a/.reviews/AnalyzeRepo.md +++ b/.reviews/AnalyzeRepo.md @@ -83,10 +83,33 @@ const doctor = parseDoctorResult(doctorJson); ```bash exec OUT=$(OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" -else - echo "[]" +if [ -z "$OUT" ] || ! printf '%s' "$OUT" | jq -c ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) +'; then + printf '[]' fi ``` @@ -97,10 +120,33 @@ fi ```bash exec OUT=$(.reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" -else - echo "[]" +if [ -z "$OUT" ] || ! printf '%s' "$OUT" | jq -c ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) +'; then + printf '[]' fi ``` diff --git a/.reviews/AnalyzeRepoCI.md b/.reviews/AnalyzeRepoCI.md index be8f9730..90539595 100644 --- a/.reviews/AnalyzeRepoCI.md +++ b/.reviews/AnalyzeRepoCI.md @@ -2,6 +2,8 @@ title: Repository Analysis (CI) --- + + ```bash silent exec mkdir -p .reviews cat > .reviews/tsconfig.oxlint.json << 'TSCONFIG' @@ -79,10 +81,33 @@ const doctor = parseDoctorResult(doctorJson); ```bash exec OUT=$(OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" -else - echo "[]" +if [ -z "$OUT" ] || ! printf '%s' "$OUT" | jq -c ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) +'; then + printf '[]' fi ``` @@ -93,10 +118,33 @@ fi ```bash exec OUT=$(.reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" -else - echo "[]" +if [ -z "$OUT" ] || ! printf '%s' "$OUT" | jq -c ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) +'; then + printf '[]' fi ``` @@ -127,3 +175,5 @@ const cleanupAnalysis = buildCleanupAnalysis(diagnostics); + + diff --git a/.reviews/ReviewPR.local.md b/.reviews/ReviewPR.local.md index 843c0fcc..c852eab9 100644 --- a/.reviews/ReviewPR.local.md +++ b/.reviews/ReviewPR.local.md @@ -94,10 +94,43 @@ git diff --name-only {BASE_SHA}...{HEAD_SHA} -- '*.ts' '*.tsx' | grep -v '\.test || doctor.recommendation === "type-aware-filtered"}> ```bash exec -if [ -n "{changedTsFiles}" ]; then - echo "{changedTsFiles}" | tr '\n' ' ' | OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true +changed_files=$(cat <<'FILES' +{changedTsFiles} +FILES +) +if [ -z "$changed_files" ]; then + printf '[]' else - echo "[]" + raw=$(printf '%s\n' "$changed_files" | OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) + if [ -z "$raw" ] || ! printf '%s' "$raw" | jq -c --arg changed "$changed_files" ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) + | map(select(.file as $file | ($changed | split("\n") | index($file)) != null)) + '; then + printf '[]' + fi fi ``` @@ -107,10 +140,43 @@ fi && doctor.oxlintInstalled}> ```bash exec -if [ -n "{changedTsFiles}" ]; then - echo "{changedTsFiles}" | tr '\n' ' ' | xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true +changed_files=$(cat <<'FILES' +{changedTsFiles} +FILES +) +if [ -z "$changed_files" ]; then + printf '[]' else - echo "[]" + raw=$(printf '%s\n' "$changed_files" | xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) + if [ -z "$raw" ] || ! printf '%s' "$raw" | jq -c --arg changed "$changed_files" ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) + | map(select(.file as $file | ($changed | split("\n") | index($file)) != null)) + '; then + printf '[]' + fi fi ``` diff --git a/.reviews/ReviewPR.md b/.reviews/ReviewPR.md index 09741a0b..ecea60f1 100644 --- a/.reviews/ReviewPR.md +++ b/.reviews/ReviewPR.md @@ -2,6 +2,8 @@ title: PR Review --- + + ```ts eval const BASE_SHA = process.env.BASE_SHA ?? "HEAD~1"; const HEAD_SHA = process.env.HEAD_SHA ?? "HEAD"; @@ -98,10 +100,43 @@ git diff --name-only {BASE_SHA}...{HEAD_SHA} -- '*.ts' '*.tsx' | grep -v '\.test || doctor.recommendation === "type-aware-filtered"}> ```bash exec -if [ -n "{changedTsFiles}" ]; then - echo "{changedTsFiles}" | tr '\n' ' ' | OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true +changed_files=$(cat <<'FILES' +{changedTsFiles} +FILES +) +if [ -z "$changed_files" ]; then + printf '[]' else - echo "[]" + raw=$(printf '%s\n' "$changed_files" | OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) + if [ -z "$raw" ] || ! printf '%s' "$raw" | jq -c --arg changed "$changed_files" ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) + | map(select(.file as $file | ($changed | split("\n") | index($file)) != null)) + '; then + printf '[]' + fi fi ``` @@ -111,10 +146,43 @@ fi && doctor.oxlintInstalled}> ```bash exec -if [ -n "{changedTsFiles}" ]; then - echo "{changedTsFiles}" | tr '\n' ' ' | xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true +changed_files=$(cat <<'FILES' +{changedTsFiles} +FILES +) +if [ -z "$changed_files" ]; then + printf '[]' else - echo "[]" + raw=$(printf '%s\n' "$changed_files" | xargs .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) + if [ -z "$raw" ] || ! printf '%s' "$raw" | jq -c --arg changed "$changed_files" ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def span_line: + if (.line? | type) == "number" then .line + elif (.labels?[0].span.line? | type) == "number" then .labels[0].span.line + else 0 + end; + def span_column: + if (.column? | type) == "number" then .column + elif (.labels?[0].span.column? | type) == "number" then .labels[0].span.column + else 0 + end; + entries + | map({ + message: (if (.message? | type) == "string" then .message else "" end), + ruleId: (if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end), + severity: (if .severity == "error" then "error" else "warning" end), + file: (if (.file? | type) == "string" then .file elif (.filename? | type) == "string" then .filename else "" end), + line: span_line, + column: span_column + }) + | map(select(.file as $file | ($changed | split("\n") | index($file)) != null)) + '; then + printf '[]' + fi fi ``` @@ -146,3 +214,5 @@ const diagnostics = parseDiagnostics(rawDiagnostics, pr, doctor); + + diff --git a/.reviews/components/AbstractionNames.md b/.reviews/components/AbstractionNames.md index 338ef8e2..3572a83c 100644 --- a/.reviews/components/AbstractionNames.md +++ b/.reviews/components/AbstractionNames.md @@ -18,14 +18,14 @@ props: --- ```ts eval -const re = new RegExp(pattern, "i"); -const suspicious = pr.created +const re = new RegExp(props.pattern, "i"); +const suspicious = props.pr.created .filter(f => f.path.endsWith(".ts") && !f.isTest && !f.isTypeDeclaration) .filter(f => re.test(f.path)); const triggered = suspicious.length > 0; -const resolvedMessage = message.replace( +const resolvedMessage = props.message.replace( "{names}", suspicious.map(f => f.path).join(", ") ); ``` - + diff --git a/.reviews/components/CleanupIssues.md b/.reviews/components/CleanupIssues.md index 7f711724..110458b9 100644 --- a/.reviews/components/CleanupIssues.md +++ b/.reviews/components/CleanupIssues.md @@ -11,30 +11,31 @@ props: --- ```ts persist eval -const token = process.env.GITHUB_TOKEN; const repo = process.env.GITHUB_REPOSITORY; -if (!token || !repo) { +function githubHeaders() { + return { + "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, + "Accept": "application/vnd.github+json", + "Content-Type": "application/json", + }; +} + +if (!process.env.GITHUB_TOKEN || !repo) { return ""; } const [owner, repoName] = repo.split("/"); const api = `https://api.github.com/repos/${owner}/${repoName}`; -const headers = { - "Authorization": `Bearer ${token}`, - "Accept": "application/vnd.github+json", - "Content-Type": "application/json", -}; - const LABEL = "cleanup"; const TOP_N = 5; // 1. Ensure label exists -const labelResponse = yield* fetch(`${api}/labels/${LABEL}`, { headers }); +const labelResponse = yield* fetch(`${api}/labels/${LABEL}`, { headers: githubHeaders() }); if (labelResponse.status === 404) { yield* fetch(`${api}/labels`, { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ name: LABEL, description: "Auto-generated cleanup finding from repo analysis", @@ -49,7 +50,7 @@ let page = 1; while (true) { const batch = yield* fetch( `${api}/issues?labels=${LABEL}&state=open&per_page=100&page=${page}`, - { headers }, + { headers: githubHeaders() }, ).expect().json(); if (!Array.isArray(batch) || batch.length === 0) break; @@ -69,7 +70,7 @@ for (const issue of existingIssues) { } // 4. Process top 5 clusters -const topClusters = cleanupAnalysis.fileClusters.slice(0, TOP_N); +const topClusters = props.cleanupAnalysis.fileClusters.slice(0, TOP_N); const topFiles = new Set(topClusters.map(c => c.file)); let created = 0; @@ -108,14 +109,14 @@ for (const cluster of topClusters) { if (existing) { yield* fetch(api + "/issues/" + existing.number, { method: "PATCH", - headers, + headers: githubHeaders(), body: JSON.stringify({ title, body }), }).expect(); updated++; } else { yield* fetch(api + "/issues", { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ title, body, @@ -131,7 +132,7 @@ for (const [file, issue] of issuesByFile.entries()) { if (!topFiles.has(file)) { yield* fetch(api + "/issues/" + issue.number + "/comments", { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ body: "Resolved — file no longer in top-5 cleanup targets. Closing automatically.", }), @@ -139,7 +140,7 @@ for (const [file, issue] of issuesByFile.entries()) { yield* fetch(api + "/issues/" + issue.number, { method: "PATCH", - headers, + headers: githubHeaders(), body: JSON.stringify({ state: "closed" }), }).expect(); closed++; diff --git a/.reviews/components/CommentReview.md b/.reviews/components/CommentReview.md index e3b95ef2..944b2eaf 100644 --- a/.reviews/components/CommentReview.md +++ b/.reviews/components/CommentReview.md @@ -12,7 +12,7 @@ props: // --------------------------------------------------------------------------- // 1. Build comment/code pairs with file/line metadata const pairs = []; -const lines = pr.added.filter(l => !l.isTest); +const lines = props.pr.added.filter(l => !l.isTest); for (let i = 0; i < lines.length - 1; i++) { const current = lines[i].content.trim(); @@ -39,24 +39,26 @@ let checklistMd = ""; // --------------------------------------------------------------------------- // 2. Fetch previous bot review comments and human replies -const token = process.env.GITHUB_TOKEN; const repo = process.env.GITHUB_REPOSITORY; const prNumber = process.env.PR_NUMBER; +function githubHeaders() { + return { + "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, + "Accept": "application/vnd.github+json", + }; +} + let previousFindings = []; let dismissedReplies = []; let repliesForClassification = []; -if (token && repo && prNumber) { +if (process.env.GITHUB_TOKEN && repo && prNumber) { const [owner, name] = repo.split("/"); const api = `https://api.github.com/repos/${owner}/${name}`; - const headers = { - "Authorization": `Bearer ${token}`, - "Accept": "application/vnd.github+json", - }; const allComments = yield* fetch( - `${api}/pulls/${prNumber}/comments?per_page=100`, { headers } + `${api}/pulls/${prNumber}/comments?per_page=100`, { headers: githubHeaders() } ).expect().json(); const botComments = allComments.filter(c => @@ -94,7 +96,7 @@ if (token && repo && prNumber) { }; try { const reactions = yield* fetch( - `${api}/pulls/comments/${reply.id}/reactions`, { headers } + `${api}/pulls/comments/${reply.id}/reactions`, { headers: githubHeaders() } ).expect().json(); const alreadyAcked = reactions.some(r => r.user.login === "github-actions[bot]" && r.content === "+1" @@ -168,7 +170,7 @@ const dismissedSet = new Set( ); const addedLineSet = new Set( - pr.added.map(l => `${l.file}:${l.lineNumber}`) + props.pr.added.map(l => `${l.file}:${l.lineNumber}`) ); const appliedFindings = previousFindings.filter(pf => pf.lineNumber && !addedLineSet.has(`${pf.file}:${pf.lineNumber}`) && diff --git a/.reviews/components/ConfigSourceMix.md b/.reviews/components/ConfigSourceMix.md index 42973ec0..2dd9f6cb 100644 --- a/.reviews/components/ConfigSourceMix.md +++ b/.reviews/components/ConfigSourceMix.md @@ -18,11 +18,11 @@ props: --- ```ts eval -const hasConfig = pr.files.some(f => f.isConfig); -const hasSource = pr.files.some(f => +const hasConfig = props.pr.files.some(f => f.isConfig); +const hasSource = props.pr.files.some(f => !f.isConfig && !f.isTest && !f.isTypeDeclaration ); -const triggered = hasConfig && hasSource && pr.stats.totalFiles > minFiles; +const triggered = hasConfig && hasSource && props.pr.stats.totalFiles > props.minFiles; ``` - + diff --git a/.reviews/components/DeepInfraProvider.md b/.reviews/components/DeepInfraProvider.md index 241bbeb2..b0b474a7 100644 --- a/.reviews/components/DeepInfraProvider.md +++ b/.reviews/components/DeepInfraProvider.md @@ -11,7 +11,7 @@ props: ```ts persist eval yield* Sample.around({ *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } @@ -27,7 +27,7 @@ yield* Sample.around({ "Content-Type": "application/json", "Authorization": `Bearer ${process.env.DEEPINFRA_TOKEN}`, }, - body: JSON.stringify({ model, messages, temperature: 0, max_tokens: 4096 }), + body: JSON.stringify({ model: props.model, messages, temperature: 0, max_tokens: 4096 }), }) .expect() .json(); diff --git a/.reviews/components/DescriptionCheck.md b/.reviews/components/DescriptionCheck.md index b650de45..4b86a54f 100644 --- a/.reviews/components/DescriptionCheck.md +++ b/.reviews/components/DescriptionCheck.md @@ -17,5 +17,5 @@ props: additionalProperties: false --- - + diff --git a/.reviews/components/Doctor.md b/.reviews/components/Doctor.md index d5701238..b9fb0605 100644 --- a/.reviews/components/Doctor.md +++ b/.reviews/components/Doctor.md @@ -42,7 +42,7 @@ test -x .reviews/.oxlint/oxlint && echo "EXISTS" || echo "MISSING" ```bash exec -test -f {tsconfigPath} && echo "EXISTS" || echo "MISSING" +test -f {props.tsconfigPath} && echo "EXISTS" || echo "MISSING" ``` @@ -87,13 +87,71 @@ Running type-aware probe to test Oxlint compatibility... + fallback='{"diagnosticCount":0,"importNoiseCount":0,"filesAnalyzed":0,"filesSkipped":0,"importErrors":0,"availableRuleIds":[],"tsgolintCrashed":false}'> ```bash exec -RESULT=$(OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig {tsconfigPath} --format json 2>.reviews/probe-stderr.tmp || true) +RESULT=$(OXLINT_TSGOLINT_PATH=.reviews/.oxlint/tsgolint .reviews/.oxlint/oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig {props.tsconfigPath} --format json 2>.reviews/probe-stderr.tmp || true) STDERR=$(cat .reviews/probe-stderr.tmp 2>/dev/null || echo "") rm -f .reviews/probe-stderr.tmp -echo "{\"diagnostics\":$RESULT,\"stderr\":\"$STDERR\"}" +if [ -n "$RESULT" ] && printf '%s' "$RESULT" | jq -c --arg stderr "$STDERR" ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def file: + if (.file? | type) == "string" then .file + elif (.filename? | type) == "string" then .filename + else "" + end; + def rule: + if (.ruleId? | type) == "string" then .ruleId + elif (.code? | type) == "string" then .code + else "unknown" + end; + def message: + if (.message? | type) == "string" then .message else "" end; + def import_noise: + ((.message | ascii_downcase | contains("cannot find module")) + or (.ruleId | ascii_downcase | contains("import"))); + def crashed($value): + ($value | ascii_downcase) as $lower + | ($lower | contains("tsgolint")) + and (($lower | contains("panic")) + or ($lower | contains("oom")) + or ($lower | contains("fatal"))); + entries + | map({file: file, ruleId: rule, message: message}) as $diagnostics + | { + diagnosticCount: ($diagnostics | length), + importNoiseCount: ([$diagnostics[] | select(import_noise)] | length), + filesAnalyzed: ([$diagnostics[].file | select(length > 0)] | unique | length), + filesSkipped: ([$diagnostics[] | select(import_noise) | .file | select(length > 0)] | unique | length), + importErrors: ([$diagnostics[] | select(import_noise)] | length), + availableRuleIds: ([$diagnostics[].ruleId | select(length > 0)] | unique), + tsgolintCrashed: crashed($stderr) + } +'; then + : +else + jq -cn --arg stderr "$STDERR" ' + def crashed($value): + ($value | ascii_downcase) as $lower + | ($lower | contains("tsgolint")) + and (($lower | contains("panic")) + or ($lower | contains("oom")) + or ($lower | contains("fatal"))); + { + diagnosticCount: 0, + importNoiseCount: 0, + filesAnalyzed: 0, + filesSkipped: 0, + importErrors: 0, + availableRuleIds: [], + tsgolintCrashed: crashed($stderr) + } + ' +fi ``` @@ -117,31 +175,25 @@ const TYPE_AWARE_RULES = [ "no-unnecessary-boolean-literal-compare", ]; -let probe = { diagnostics: [], stderr: "" }; -try { probe = JSON.parse(probeResult); } catch { /* malformed */ } - -const diagnostics = Array.isArray(probe.diagnostics) - ? probe.diagnostics - : (probe.diagnostics && typeof probe.diagnostics === "object" - && Array.isArray(probe.diagnostics.diagnostics)) - ? probe.diagnostics.diagnostics - : []; - -const importNoise = diagnostics.filter(d => - d.message?.includes("Cannot find module") - || d.message?.includes("cannot find") - || d.ruleId?.includes("import") -); +let probe = { + diagnosticCount: 0, + importNoiseCount: 0, + filesAnalyzed: 0, + filesSkipped: 0, + importErrors: 0, + availableRuleIds: [], + tsgolintCrashed: false, +}; +try { probe = { ...probe, ...JSON.parse(probeResult) }; } catch { } -const fileSet = new Set(diagnostics.map(d => d.file).filter(Boolean)); -const noiseRatio = diagnostics.length > 0 - ? importNoise.length / diagnostics.length : 0; +const diagnosticCount = typeof probe.diagnosticCount === "number" + ? probe.diagnosticCount : 0; +const importNoiseCount = typeof probe.importNoiseCount === "number" + ? probe.importNoiseCount : 0; +const noiseRatio = diagnosticCount > 0 + ? importNoiseCount / diagnosticCount : 0; -const tsgolintCrashed = typeof probe.stderr === "string" - && probe.stderr.includes("tsgolint") - && (probe.stderr.includes("panic") - || probe.stderr.includes("OOM") - || probe.stderr.includes("fatal")); +const tsgolintCrashed = probe.tsgolintCrashed === true; const typeAwareAvailable = canProbeTypeAware && !tsgolintCrashed; @@ -167,9 +219,11 @@ const doctor = { tsconfigExists, nodeModulesExists, typeAwareAvailable, - filesAnalyzed: fileSet.size, - filesSkipped: new Set(importNoise.map(d => d.file).filter(Boolean)).size, - importErrors: importNoise.length, + filesAnalyzed: typeof probe.filesAnalyzed === "number" ? probe.filesAnalyzed : 0, + filesSkipped: typeof probe.filesSkipped === "number" ? probe.filesSkipped : 0, + importErrors: typeof probe.importErrors === "number" ? probe.importErrors : 0, + availableRuleIds: Array.isArray(probe.availableRuleIds) + ? probe.availableRuleIds : [], bloatRulesAvailable, bloatRulesMissing, recommendation, diff --git a/.reviews/components/EnsureOxlint.md b/.reviews/components/EnsureOxlint.md index 59d8f403..09b67771 100644 --- a/.reviews/components/EnsureOxlint.md +++ b/.reviews/components/EnsureOxlint.md @@ -15,7 +15,7 @@ props: # neither can change without the other. The workflow guards on the resulting # binaries (see review.yml). set -euo pipefail -DIR="{dir}" +DIR="{props.dir}" OXLINT_TAG="apps_v1.74.0" TSGOLINT_VERSION="0.25.0" mkdir -p "$DIR" diff --git a/.reviews/components/Finding.md b/.reviews/components/Finding.md index af480db9..4d8fde17 100644 --- a/.reviews/components/Finding.md +++ b/.reviews/components/Finding.md @@ -14,11 +14,11 @@ props: --- ```ts eval -const icon = severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; +const icon = props.severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; ``` - + -{icon} {message} +{icon} {props.message} diff --git a/.reviews/components/GitHubComment.md b/.reviews/components/GitHubComment.md index 0618b4a0..3d7ee32f 100644 --- a/.reviews/components/GitHubComment.md +++ b/.reviews/components/GitHubComment.md @@ -9,35 +9,36 @@ props: --- ```ts eval -// GITHUB_TOKEN is read inline at each call site, never assigned to a binding: -// eval bindings are journaled, and the journal is uploaded as a CI artifact. const content = yield* renderChildren(); -const body = marker + "\n" + content.trim(); +const body = props.marker + "\n" + content.trim(); const repo = process.env.GITHUB_REPOSITORY; const prNumber = process.env.PR_NUMBER; const [owner, name] = repo.split("/"); const api = `https://api.github.com/repos/${owner}/${name}`; -const commentsResult = yield* fetch(`${api}/issues/${prNumber}/comments`, { - headers: { +function githubHeaders() { + return { "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, "Accept": "application/vnd.github+json", - }, + }; +} + +const commentsResult = yield* fetch(`${api}/issues/${prNumber}/comments`, { + headers: githubHeaders(), }) .expect() .json(); const existing = commentsResult.find(c => - c.user.type === "Bot" && c.body.includes(marker) + c.user.type === "Bot" && c.body.includes(props.marker) ); if (existing) { yield* fetch(`${api}/issues/comments/${existing.id}`, { method: "PATCH", headers: { - "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, - "Accept": "application/vnd.github+json", + ...githubHeaders(), "Content-Type": "application/json", }, body: JSON.stringify({ body }), @@ -46,8 +47,7 @@ if (existing) { yield* fetch(`${api}/issues/${prNumber}/comments`, { method: "POST", headers: { - "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, - "Accept": "application/vnd.github+json", + ...githubHeaders(), "Content-Type": "application/json", }, body: JSON.stringify({ body }), diff --git a/.reviews/components/LinkedIssue.md b/.reviews/components/LinkedIssue.md index ae902f99..58424af5 100644 --- a/.reviews/components/LinkedIssue.md +++ b/.reviews/components/LinkedIssue.md @@ -18,8 +18,8 @@ props: --- ```ts eval -const hasIssue = /(?:#\d+|https:\/\/github\.com\/.*\/issues\/\d+)/.test(pr.meta.body); +const hasIssue = /(?:#\d+|https:\/\/github\.com\/.*\/issues\/\d+)/.test(props.pr.meta.body); ``` - whenLinesExceed} - severity={severity} message={message} /> + props.whenLinesExceed} + severity={props.severity} message={props.message} /> diff --git a/.reviews/components/NewDependencies.md b/.reviews/components/NewDependencies.md index fa949fd3..ef40827e 100644 --- a/.reviews/components/NewDependencies.md +++ b/.reviews/components/NewDependencies.md @@ -15,11 +15,11 @@ props: --- ```ts eval -const touchesPkg = pr.files.some(f => +const touchesPkg = props.pr.files.some(f => f.path === "package.json" || f.path.endsWith("/package.json") ); -const mentionsDeps = pr.meta.body.toLowerCase().includes("dependenc"); +const mentionsDeps = props.pr.meta.body.toLowerCase().includes("dependenc"); const triggered = touchesPkg && !mentionsDeps; ``` - + diff --git a/.reviews/components/OllamaProvider.md b/.reviews/components/OllamaProvider.md index 3c07a10f..69e24eab 100644 --- a/.reviews/components/OllamaProvider.md +++ b/.reviews/components/OllamaProvider.md @@ -14,7 +14,7 @@ props: ```ts persist eval yield* Sample.around({ *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } @@ -24,10 +24,10 @@ yield* Sample.around({ } messages.push({ role: "user", content: context.content }); - const result = yield* fetch(`${baseUrl}/v1/chat/completions`, { + const result = yield* fetch(`${props.baseUrl}/v1/chat/completions`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ model, messages, temperature: 0 }), + body: JSON.stringify({ model: props.model, messages, temperature: 0 }), }) .expect() .json(); diff --git a/.reviews/components/OxlintSignals.md b/.reviews/components/OxlintSignals.md index 186222c9..175d23a2 100644 --- a/.reviews/components/OxlintSignals.md +++ b/.reviews/components/OxlintSignals.md @@ -11,11 +11,11 @@ props: --- ```ts eval -if (groups.length === 0) return; +if (props.groups.length === 0) return; -const lines = groups.map(g => +const lines = props.groups.map(g => `- \`${g.ruleId}\` ×${g.count}: ${g.files.slice(0, 3).join(", ")}${g.files.length > 3 ? ` (+${g.files.length - 3})` : ""}` ); -return `**Oxlint ${label}:**\n${lines.join("\n")}`; +return `**Oxlint ${props.label}:**\n${lines.join("\n")}`; ``` diff --git a/.reviews/components/OxlintSummary.md b/.reviews/components/OxlintSummary.md index 937c4009..53f15491 100644 --- a/.reviews/components/OxlintSummary.md +++ b/.reviews/components/OxlintSummary.md @@ -13,22 +13,22 @@ props: - + 🟡 Oxlint not installed. Static analysis skipped. - 0}> + 0}> -{diagnostics.summary} +{props.diagnostics.summary} - 0 - && doctor.oxlintInstalled}> + 0 + && props.doctor.oxlintInstalled}> -*{doctor.bloatRulesMissing.length} type-aware rules unavailable +*{props.doctor.bloatRulesMissing.length} type-aware rules unavailable — install `oxlint-tsgolint` for full coverage.* diff --git a/.reviews/components/Pattern.md b/.reviews/components/Pattern.md index 8cd51dfb..9a57645a 100644 --- a/.reviews/components/Pattern.md +++ b/.reviews/components/Pattern.md @@ -22,16 +22,16 @@ props: --- ```ts eval -const re = new RegExp(pattern, "g"); -const lines = excludeTests - ? pr.added.filter(l => !l.isTest) - : pr.added; +const re = new RegExp(props.pattern, "g"); +const lines = props.excludeTests + ? props.pr.added.filter(l => !l.isTest) + : props.pr.added; const matches = lines.filter(l => re.test(l.content)); re.lastIndex = 0; -if (matches.length >= min) { - const icon = severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; - return icon + " " + message +if (matches.length >= props.min) { + const icon = props.severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; + return icon + " " + props.message .replace("{count}", String(matches.length)); } ``` diff --git a/.reviews/components/PrPolicyReport.md b/.reviews/components/PrPolicyReport.md index 7ca0e588..d5ab9808 100644 --- a/.reviews/components/PrPolicyReport.md +++ b/.reviews/components/PrPolicyReport.md @@ -12,16 +12,16 @@ props: additionalProperties: false --- -## PR #{pr.meta.number}: {pr.meta.title} +## PR #{props.pr.meta.number}: {props.pr.meta.title} -**{pr.stats.totalFiles}** files, **+{pr.stats.additions}** / **-{pr.stats.deletions}** +**{props.pr.stats.totalFiles}** files, **+{props.pr.stats.additions}** / **-{props.pr.stats.deletions}** - + - + - + - + - + diff --git a/.reviews/components/Ratio.md b/.reviews/components/Ratio.md index 7d97b0af..b9a2b037 100644 --- a/.reviews/components/Ratio.md +++ b/.reviews/components/Ratio.md @@ -26,20 +26,20 @@ props: --- ```ts eval -const numRe = new RegExp(numerator, "g"); -const denRe = new RegExp(denominator, "g"); -const lines = excludeTests - ? pr.added.filter(l => !l.isTest) - : pr.added; +const numRe = new RegExp(props.numerator, "g"); +const denRe = new RegExp(props.denominator, "g"); +const lines = props.excludeTests + ? props.pr.added.filter(l => !l.isTest) + : props.pr.added; const source = lines.map(l => l.content).join("\n"); const numCount = (source.match(numRe) ?? []).length; const denCount = (source.match(denRe) ?? []).length; -if (denCount >= minDenominator && numCount / denCount > threshold) { +if (denCount >= props.minDenominator && numCount / denCount > props.threshold) { const ratio = (numCount / denCount * 100).toFixed(1); - const icon = severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; - return icon + " " + message + const icon = props.severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; + return icon + " " + props.message .replace("{ratio}", ratio) .replace("{numeratorCount}", String(numCount)) .replace("{denominatorCount}", String(denCount)); diff --git a/.reviews/components/ReleaseSpecWarning.md b/.reviews/components/ReleaseSpecWarning.md index 4773e8ba..3f2257c1 100644 --- a/.reviews/components/ReleaseSpecWarning.md +++ b/.reviews/components/ReleaseSpecWarning.md @@ -29,7 +29,7 @@ list when a file is added or removed. ```ts eval -const releaseChanged = files.filter((path) => releaseConfigFiles.includes(`- ${path}`)); +const releaseChanged = props.files.filter((path) => releaseConfigFiles.includes(`- ${path}`)); const changedList = releaseChanged.join(", "); // TODO: evaluate whether the diff's changes are actually reflected in the @@ -38,7 +38,7 @@ const changedList = releaseChanged.join(", "); - 0 && !files.includes("specs/release-process-spec.md")}> + 0 && !props.files.includes("specs/release-process-spec.md")}> > [!WARNING] > This PR changes release configuration ({changedList}) without touching diff --git a/.reviews/components/RepoPolicyReport.md b/.reviews/components/RepoPolicyReport.md index 14309d1e..1dcc5ac5 100644 --- a/.reviews/components/RepoPolicyReport.md +++ b/.reviews/components/RepoPolicyReport.md @@ -20,10 +20,10 @@ props: ## Repository Analysis -**{fileCount}** TypeScript files, **{lineCount}** total lines +**{props.fileCount}** TypeScript files, **{props.lineCount}** total lines - + - + - + diff --git a/.reviews/components/ReviewSection.md b/.reviews/components/ReviewSection.md index 9381a5f2..2c3026c1 100644 --- a/.reviews/components/ReviewSection.md +++ b/.reviews/components/ReviewSection.md @@ -14,6 +14,6 @@ props: ```ts eval const content = yield* renderChildren(); return content.trim().length > 0 - ? `### ${heading}\n\n${content}` - : `### ${heading}\n\n${clean}`; + ? `### ${props.heading}\n\n${content}` + : `### ${props.heading}\n\n${props.clean}`; ``` diff --git a/.reviews/components/Show.md b/.reviews/components/Show.md index e4997e92..2e4d03a1 100644 --- a/.reviews/components/Show.md +++ b/.reviews/components/Show.md @@ -12,10 +12,10 @@ props: --- ```ts eval -if (when) { +if (props.when) { return yield* renderChildren(); } -if (fallback) { - return fallback; +if (props.fallback) { + return props.fallback; } ``` diff --git a/.reviews/components/SuggestRemoval.md b/.reviews/components/SuggestRemoval.md index a20c6304..82b31ef9 100644 --- a/.reviews/components/SuggestRemoval.md +++ b/.reviews/components/SuggestRemoval.md @@ -12,12 +12,19 @@ props: --- ```ts eval -const token = process.env.GITHUB_TOKEN; const repo = process.env.GITHUB_REPOSITORY; const prNumber = process.env.PR_NUMBER; const headSha = process.env.HEAD_SHA; -if (!token || !repo || !prNumber || !headSha) { +function githubHeaders() { + return { + "Authorization": `Bearer ${process.env.GITHUB_TOKEN}`, + "Accept": "application/vnd.github+json", + "Content-Type": "application/json", + }; +} + +if (!process.env.GITHUB_TOKEN || !repo || !prNumber || !headSha) { return ""; } @@ -25,14 +32,8 @@ const [owner, name] = repo.split("/"); const api = `https://api.github.com/repos/${owner}/${name}`; const graphql = "https://api.github.com/graphql"; -const headers = { - "Authorization": `Bearer ${token}`, - "Accept": "application/vnd.github+json", - "Content-Type": "application/json", -}; - const existingReviews = yield* fetch( - `${api}/pulls/${prNumber}/reviews`, { headers } + `${api}/pulls/${prNumber}/reviews`, { headers: githubHeaders() } ).expect().json(); const botReviews = existingReviews.filter(r => @@ -44,7 +45,7 @@ for (const review of botReviews) { try { yield* fetch(`${api}/pulls/${prNumber}/reviews/${review.id}`, { method: "DELETE", - headers, + headers: githubHeaders(), }).expect(); } catch { // Review may already be submitted (can't delete submitted reviews). @@ -52,7 +53,7 @@ for (const review of botReviews) { } // 2. React 👍 on dismiss replies and resolve their threads -if (dismissedReplies.length > 0) { +if (props.dismissedReplies.length > 0) { // Fetch review threads via GraphQL to get thread node IDs const threadsQuery = `query($owner: String!, $name: String!, $pr: Int!) { repository(owner: $owner, name: $name) { @@ -74,7 +75,7 @@ if (dismissedReplies.length > 0) { try { const threadsResult = yield* fetch(graphql, { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ query: threadsQuery, variables: { owner, name, pr: parseInt(prNumber, 10) }, @@ -92,13 +93,13 @@ if (dismissedReplies.length > 0) { // If GraphQL fails, skip thread resolution — 👍 reaction still works } - for (const reply of dismissedReplies) { + for (const reply of props.dismissedReplies) { // React 👍 if (reply.replyId) { try { yield* fetch(`${api}/pulls/comments/${reply.replyId}/reactions`, { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ content: "+1" }), }).expect(); } catch {} @@ -111,7 +112,7 @@ if (dismissedReplies.length > 0) { try { yield* fetch(graphql, { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ query: `mutation($threadId: ID!) { resolveReviewThread(input: { threadId: $threadId }) { @@ -128,8 +129,8 @@ if (dismissedReplies.length > 0) { } // 3. Post new review with pending findings -if (findings.length > 0) { - const comments = findings.map(f => ({ +if (props.findings.length > 0) { + const comments = props.findings.map(f => ({ path: f.file, line: f.lineNumber, body: `Redundant comment — restates what the code does.\n\`\`\`suggestion\n\`\`\``, @@ -137,11 +138,11 @@ if (findings.length > 0) { yield* fetch(`${api}/pulls/${prNumber}/reviews`, { method: "POST", - headers, + headers: githubHeaders(), body: JSON.stringify({ commit_id: headSha, event: "COMMENT", - body: `Found ${findings.length} redundant comment${findings.length === 1 ? "" : "s"}. Inline suggestions to remove them below.`, + body: `Found ${props.findings.length} redundant comment${props.findings.length === 1 ? "" : "s"}. Inline suggestions to remove them below.`, comments, }), }).expect(); diff --git a/.reviews/components/Threshold.md b/.reviews/components/Threshold.md index 7f2c6a13..14738b9d 100644 --- a/.reviews/components/Threshold.md +++ b/.reviews/components/Threshold.md @@ -21,14 +21,14 @@ props: ```ts eval const metrics = { - totalChanges: pr.stats.totalChanges, - totalFiles: pr.stats.totalFiles, - additions: pr.stats.additions, - deletions: pr.stats.deletions, - directories: pr.directories.size, + totalChanges: props.pr.stats.totalChanges, + totalFiles: props.pr.stats.totalFiles, + additions: props.pr.stats.additions, + deletions: props.pr.stats.deletions, + directories: props.pr.directories.size, }; -const actual = metrics[metric]; +const actual = metrics[props.metric]; const ops = { ">": (a, b) => a > b, ">=": (a, b) => a >= b, @@ -37,10 +37,10 @@ const ops = { "==": (a, b) => a == b, }; -if (ops[op](actual, value)) { - const icon = severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; - return icon + " " + message +if (ops[props.op](actual, props.value)) { + const icon = props.severity === "error" ? "\ud83d\udd34" : "\ud83d\udfe1"; + return icon + " " + props.message .replace("{actual}", String(actual)) - .replace("{value}", String(value)); + .replace("{value}", String(props.value)); } ``` diff --git a/.reviews/components/UnusedInDiff.md b/.reviews/components/UnusedInDiff.md index b4c5ca17..6ee2b71c 100644 --- a/.reviews/components/UnusedInDiff.md +++ b/.reviews/components/UnusedInDiff.md @@ -16,7 +16,7 @@ props: --- ```ts eval -const lines = pr.added.filter(l => +const lines = props.pr.added.filter(l => l.file.endsWith(".ts") || l.file.endsWith(".tsx") ); const source = lines.map(l => l.content).join("\n"); @@ -26,7 +26,7 @@ const source = lines.map(l => l.content).join("\n"); // whose `type` keyword sits inside braces rather than at the start of a // declaration. const declPattern = new RegExp( - `^\\s*(?:export\\s+)?(?:default\\s+|declare\\s+)?${construct}\\s+(\\w+)` + `^\\s*(?:export\\s+)?(?:default\\s+|declare\\s+)?${props.construct}\\s+(\\w+)` ); const decls = []; @@ -45,8 +45,8 @@ const unused = decls .filter(d => d.refs <= 1); const hasUnused = unused.length > 0; -const icon = severity === "error" ? "🔴" : "🟡"; -const summary = icon + " " + message +const icon = props.severity === "error" ? "🔴" : "🟡"; +const summary = icon + " " + props.message .replace("{names}", unused.map(u => u.name).join(", ")) .replace("{count}", String(unused.length)); ``` diff --git a/.reviews/policies/BloatPolicy.md b/.reviews/policies/BloatPolicy.md index 3b40c3bd..ae8fb64a 100644 --- a/.reviews/policies/BloatPolicy.md +++ b/.reviews/policies/BloatPolicy.md @@ -12,15 +12,15 @@ props: - - - - - - diff --git a/.reviews/policies/ExtraneousCodePolicy.md b/.reviews/policies/ExtraneousCodePolicy.md index b43586cb..9b7f9b6b 100644 --- a/.reviews/policies/ExtraneousCodePolicy.md +++ b/.reviews/policies/ExtraneousCodePolicy.md @@ -15,18 +15,18 @@ props: - 20}> + 20}> You are reviewing a TypeScript PR for EXTRANEOUS code only. -PR: {pr.meta.title} -Description: {pr.meta.body} +PR: {props.pr.meta.title} +Description: {props.pr.meta.body} STATIC ANALYSIS SIGNALS: -{diagnostics.summary} -Violation density: {diagnostics.density} per added line. +{props.diagnostics.summary} +Violation density: {props.diagnostics.density} per added line. DENSITY CALIBRATION: - Below 0.020: clean — experienced contributor, reviewed code @@ -57,7 +57,7 @@ For each finding: FILE, PATTERN, CONCERN, QUESTION for author. If clean: "No extraneous code patterns detected." DIFF: -{pr.diffPreview} +{props.pr.diffPreview} diff --git a/.reviews/policies/RepoCleanupPolicy.md b/.reviews/policies/RepoCleanupPolicy.md index a59695bb..149adafb 100644 --- a/.reviews/policies/RepoCleanupPolicy.md +++ b/.reviews/policies/RepoCleanupPolicy.md @@ -17,7 +17,7 @@ props: - 0 && !!cleanupAnalysis}> + 0 && !!props.cleanupAnalysis}> @@ -34,7 +34,7 @@ PRINCIPLES: Single-consumer abstractions should be inlined. - YAGNI: flag code that exists "just in case" with no current caller. -{cleanupAnalysis.promptContext} +{props.cleanupAnalysis.promptContext} For each of the top 5 clusters above, produce exactly this format: @@ -63,9 +63,9 @@ Example of a good item: - 0 && !cleanupAnalysis}> + 0 && !props.cleanupAnalysis}> -{diagnostics.summary} +{props.diagnostics.summary} diff --git a/.reviews/policies/ScopePolicy.md b/.reviews/policies/ScopePolicy.md index afbef961..7f414f2a 100644 --- a/.reviews/policies/ScopePolicy.md +++ b/.reviews/policies/ScopePolicy.md @@ -10,39 +10,39 @@ props: - - - - - - - - - diff --git a/.reviews/policies/SlopPolicy.md b/.reviews/policies/SlopPolicy.md index c030a3e1..8d601440 100644 --- a/.reviews/policies/SlopPolicy.md +++ b/.reviews/policies/SlopPolicy.md @@ -12,7 +12,7 @@ props: - - + - diff --git a/packages/code-review-agent/src/parse-diff.ts b/packages/code-review-agent/src/parse-diff.ts index 8b7209f9..5db30890 100644 --- a/packages/code-review-agent/src/parse-diff.ts +++ b/packages/code-review-agent/src/parse-diff.ts @@ -8,7 +8,7 @@ import type { PR, DiffFile, DiffHunk, DiffLine } from "./types.ts"; -const DIFF_PREVIEW_MAX = 80_000; +const DIFF_PREVIEW_MAX = 40_000; const EXTENSION_LANGUAGE: Record = { ".ts": "typescript", diff --git a/packages/code-review-agent/src/parse-doctor.ts b/packages/code-review-agent/src/parse-doctor.ts index b83a1e78..69cf4482 100644 --- a/packages/code-review-agent/src/parse-doctor.ts +++ b/packages/code-review-agent/src/parse-doctor.ts @@ -16,6 +16,7 @@ const DEFAULTS: DoctorResult = { filesAnalyzed: 0, filesSkipped: 0, importErrors: 0, + availableRuleIds: [], bloatRulesAvailable: [], bloatRulesMissing: [], recommendation: "syntax-only", diff --git a/packages/code-review-agent/src/types.ts b/packages/code-review-agent/src/types.ts index 31aa3eea..c1a777b2 100644 --- a/packages/code-review-agent/src/types.ts +++ b/packages/code-review-agent/src/types.ts @@ -95,6 +95,7 @@ export interface DoctorResult { filesAnalyzed: number; filesSkipped: number; importErrors: number; + availableRuleIds: string[]; bloatRulesAvailable: string[]; bloatRulesMissing: string[]; recommendation: "type-aware" | "type-aware-filtered" | "syntax-only"; diff --git a/packages/code-review-agent/tests/parse-diagnostics.test.ts b/packages/code-review-agent/tests/parse-diagnostics.test.ts index 1c091ef8..cb02e139 100644 --- a/packages/code-review-agent/tests/parse-diagnostics.test.ts +++ b/packages/code-review-agent/tests/parse-diagnostics.test.ts @@ -35,6 +35,7 @@ function makeDoctor(overrides: Partial = {}): DoctorResult { filesAnalyzed: 10, filesSkipped: 0, importErrors: 0, + availableRuleIds: [], bloatRulesAvailable: [], bloatRulesMissing: [], recommendation: "type-aware", diff --git a/packages/code-review-agent/tests/parse-diff.test.ts b/packages/code-review-agent/tests/parse-diff.test.ts index 634a0a6b..598270fc 100644 --- a/packages/code-review-agent/tests/parse-diff.test.ts +++ b/packages/code-review-agent/tests/parse-diff.test.ts @@ -145,7 +145,7 @@ describe("parseDiff", () => { expect(pr.files[0].isTypeDeclaration).toBe(true); }); - it("truncates diffPreview at 80K chars", function* () { + it("truncates diffPreview at 40K chars", function* () { const longLine = "x".repeat(100_000); const rawDiff = [ "diff --git a/big.ts b/big.ts", @@ -160,7 +160,7 @@ describe("parseDiff", () => { const pr = parseDiff(rawDiff, rawFiles, META); expect(pr.addedSource.length).toBe(100_000); - expect(pr.diffPreview.length).toBe(80_000); + expect(pr.diffPreview.length).toBe(40_000); }); it("computes directories at depth 2", function* () { diff --git a/packages/core/components/AnthropicProvider.md b/packages/core/components/AnthropicProvider.md index bb80b0e6..701ad721 100644 --- a/packages/core/components/AnthropicProvider.md +++ b/packages/core/components/AnthropicProvider.md @@ -20,7 +20,7 @@ yield * Sample.around( { *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } @@ -38,7 +38,7 @@ yield * "anthropic-version": "2023-06-01", }, body: JSON.stringify({ - model, + model: props.model, max_tokens: 4096, system: context.system || undefined, messages: [{ role: "user", content: context.content }], diff --git a/packages/core/components/Instruction.md b/packages/core/components/Instruction.md index 76ae2856..24195e0e 100644 --- a/packages/core/components/Instruction.md +++ b/packages/core/components/Instruction.md @@ -25,7 +25,7 @@ yield * const existing = context.system || ""; return yield* next({ ...context, - system: existing ? existing + "\n" + system : system, + system: existing ? existing + "\n" + props.system : props.system, }); }, }, diff --git a/packages/core/components/OllamaProvider.md b/packages/core/components/OllamaProvider.md index e5739026..cc63c5bd 100644 --- a/packages/core/components/OllamaProvider.md +++ b/packages/core/components/OllamaProvider.md @@ -26,7 +26,7 @@ yield * Sample.around( { *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } @@ -36,10 +36,10 @@ yield * } messages.push({ role: "user", content: context.content }); - const result = yield* fetch(`${baseUrl}/v1/chat/completions`, { + const result = yield* fetch(`${props.baseUrl}/v1/chat/completions`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ model, messages, temperature: 0 }), + body: JSON.stringify({ model: props.model, messages, temperature: 0 }), }) .expect() .json(); diff --git a/packages/core/components/Sample.md b/packages/core/components/Sample.md index 500a37c6..3aee18fa 100644 --- a/packages/core/components/Sample.md +++ b/packages/core/components/Sample.md @@ -31,15 +31,15 @@ props: ```js persist eval const childrenOutput = yield * renderChildren(); -const content = childrenOutput || prompt || ""; +const content = childrenOutput || props.prompt || ""; const sampleResult = yield * Sample.operations.sample({ content, - params: params || undefined, + params: props.params || undefined, componentName: "Sample", - model: model || undefined, + model: props.model || undefined, }); return sampleResult; diff --git a/packages/core/src/eval-env.ts b/packages/core/src/eval-env.ts index 4778b68f..279daeba 100644 --- a/packages/core/src/eval-env.ts +++ b/packages/core/src/eval-env.ts @@ -14,6 +14,50 @@ */ import type { ErrorMode } from "./errors.ts"; +import { derivedEnvironment } from "./live-env.ts"; +import type { EvalEnv, Json } from "./types.ts"; + +export function propsEnvironment(validatedProps: Record): EvalEnv { + return { values: { props: validatedProps } }; +} + +/** + * Layer an inner environment over an outer one while keeping the lexical + * props namespace attached to projected content. + * + * The layered environment is derived from the outer one so it keeps sharing + * that invocation's live overlay: live bindings live beside the durable values, + * not inside them, and a layered copy that dropped the overlay would hide them. + */ +export function layerEnvironments( + outer: EvalEnv | undefined, + inner: EvalEnv | undefined, + preserveOuterProps = true, +): EvalEnv | undefined { + if (outer === undefined) { + return inner; + } + if (inner === undefined) { + return outer; + } + + const values = { ...outer.values, ...inner.values }; + if (preserveOuterProps && "props" in outer.values) { + values.props = outer.values.props; + } + return derivedEnvironment(outer, values); +} + +export function layerProjectedContentEnvironment( + caller: EvalEnv | undefined, + authored: EvalEnv | undefined, +): EvalEnv | undefined { + const callerProps = + caller !== undefined && "props" in caller.values + ? derivedEnvironment(caller, { props: caller.values.props }) + : undefined; + return layerEnvironments(callerProps, authored); +} /** Bindings the snapshot rebinds; the shared record holds the unbound originals. */ const PROJECTING = ["renderChildren", "render", "useContent"]; diff --git a/packages/core/src/eval-interpolate.ts b/packages/core/src/eval-interpolate.ts index cb85cc96..f24aec82 100644 --- a/packages/core/src/eval-interpolate.ts +++ b/packages/core/src/eval-interpolate.ts @@ -3,8 +3,9 @@ * * Substitutes `{name}` and `{name.path.chain}` references in content * with values from the eval binding environment (`env.values`). This - * runs in the expansion engine for both code block content and text - * segments. + * runs in the expansion engine for both executable block content and text + * segments. When the environment contains the `props` root, dotted paths + * such as `{props.release.version}` are handled by the same traversal. * * References use JavaScript identifier syntax with optional dot paths: * /\{([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\}/g @@ -13,9 +14,10 @@ * Subsequent segments traverse nested properties. If any intermediate * value is null/undefined, the reference is left verbatim. * - * No collision with `{meta.key}` / `{props.key}`: those are consumed - * by the `interpolate()` pass which runs first. By the time this - * function runs, namespaced references are already resolved. + * Text `{meta.key}` references are consumed by the `interpolate()` pass + * before this function runs. Text `{props.key}` uses the same current + * `env.values.props` binding as this function. Executable block content has + * no text pass, so `{props.key}` reaches this function directly. * * If `env.values` has no key matching the root reference, it is left * verbatim. Non-string values are converted via `String()`. diff --git a/packages/core/src/execute.ts b/packages/core/src/execute.ts index 96037b22..40a54daf 100644 --- a/packages/core/src/execute.ts +++ b/packages/core/src/execute.ts @@ -90,6 +90,7 @@ import type { RootDocumentSource } from "./root-source.ts"; import { useEvalScope } from "@effectionx/scope-eval"; import { Stdio } from "@effectionx/process"; import { useSecretDetection } from "./secrets/policy.ts"; +import { propsEnvironment } from "./eval-env.ts"; import { liveEnvironment } from "./live-env.ts"; export interface ExecuteSettings { @@ -566,7 +567,7 @@ function* documentWorkflow(props: Record): Workflow { return scoped(function* () { + const overrideEnv = override === undefined ? undefined : { values: override }; yield* provideEnv( - derivedEnvironment(callerEnv, { ...(callerEnv?.values ?? {}), ...(override ?? {}) }), + layerEnvironments(callerEnv, overrideEnv, false) ?? { + values: {}, + }, ); if (scope) { yield* provideEvalScope(scope); @@ -182,15 +190,8 @@ interface ProjectionState { invocation: Invocation; enclosing: ProjectionHandle | undefined; children: Segment[]; - /** - * The environment projected content expands in. Undefined leaves the ambient - * one in place, which is how a function component publishes bindings to its - * own content: it installs an env and `content()` inherits it. - */ - callerEnv: EvalEnv | undefined; - meta: Record; - props: Record; - hideSet: Set; + caller: ProjectionFrame; + authored: ProjectionFrame; counter: BlockCounter; /** * The loop active where the caller wrote the content it projects, read at @@ -214,17 +215,23 @@ interface ProjectionState { printedErrors?: Segment[]; } +interface ProjectionFrame { + env: EvalEnv | undefined; + meta: Record; + props: Record; + hideSet: Set; +} + /** * Build the handle one invocation publishes (spec §6.3). * * Every projection expands in a task the invocation's content scope owns, so * nested invocations and persistent work created by projected content descend - * from it and stop with the invocation. The environment is the caller's, the - * resource scope is the callee's. + * from it and stop with the invocation. Projected and authored requests carry + * their own lexical frame; the resource scope is the callee's. */ function createProjectionHandle(state: ProjectionState): ProjectionHandle { const slots = partitionBySlot(state.children); - const project = makeProjectFn(state.callerEnv); let slotErrorsEmitted = false; function select(request: ProjectionRequest): Segment[] { @@ -241,13 +248,15 @@ function createProjectionHandle(state: ProjectionState): ProjectionHandle { } function environmentFor(request: ProjectionRequest): EvalEnv | undefined { - if (request.kind === "children" && request.override) { - return derivedEnvironment(state.callerEnv, { - ...(state.callerEnv?.values ?? {}), - ...request.override, - }); + const frame = request.kind === "markdown" ? state.authored : state.caller; + if (request.kind === "children" && request.override !== undefined) { + return layerEnvironments(frame.env, { values: request.override }, false); } - return state.callerEnv; + return frame.env; + } + + function frameFor(request: ProjectionRequest): ProjectionFrame { + return request.kind === "markdown" ? state.authored : state.caller; } const claimed = new WeakSet(); @@ -373,6 +382,8 @@ function createProjectionHandle(state: ProjectionState): ProjectionHandle { const segments = select(request); const mode = request.mode ?? (yield* ErrorMode.get()) ?? "print"; const contentScope = yield* state.invocation.useContentScope(); + const frame = frameFor(request); + const project = makeProjectFn(frame.env); // The enclosing handle answers written inside projected // content: it belongs to the caller's invocation, not to this one. // Dynamic markdown is the component's own, so it keeps this handle. @@ -415,9 +426,9 @@ function createProjectionHandle(state: ProjectionState): ProjectionHandle { yield* ActiveLoop.set(request.kind === "markdown" ? undefined : state.callerLoop); yield* expandSegments( project(segments), - state.meta, - state.props, - state.hideSet, + frame.meta, + frame.props, + frame.hideSet, state.counter, rendered, path, @@ -442,24 +453,21 @@ function createProjectionHandle(state: ProjectionState): ProjectionHandle { }, *expandClaimed( element: ComponentElement, - meta: Record, - props: Record, - hideSet: Set, owner: Segment[], elementPath: string, ): Operation { - // 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. // The error mode has to travel with them: the content task does not inherit // the documentation or frame this `` sits in. const mode = (yield* ErrorMode.get()) ?? "print"; return yield* runInContentScope({ segments: element.children, mode, - env: undefined, - meta, - props, - hideSet, + env: layerProjectedContentEnvironment(state.caller.env, state.authored.env), + meta: state.caller.meta, + props: state.caller.props, + hideSet: state.caller.hideSet, inner: state.enclosing, loop: state.callerLoop, errors: [], @@ -511,6 +519,7 @@ function validateRenderOverride(override: unknown): Record | un } const MAX_EXPANSION_DEPTH = 64; +const ESCAPED_BRACE_PLACEHOLDER = "\uE000"; /** * Expand an array of segments, resolving components and executing code blocks. @@ -586,16 +595,24 @@ export function* expandSegments( // Heal incomplete markdown constructs at segment boundaries (spec §2.3) // Runs synchronously — no yield, no journal entry const healed = healSegment(segment.content); + const protectedEscapes = healed.replaceAll("\\{", ESCAPED_BRACE_PLACEHOLDER); + const textEvalEnv = yield* env; + const textProps = + textEvalEnv !== undefined && "props" in textEvalEnv.values + ? textEvalEnv.values.props + : parentProps; // Interpolate {meta.key} and {props.key} — runtime, no journal - const interpolated = interpolate(healed, parentMeta, parentProps); + const interpolated = interpolate(protectedEscapes, parentMeta, textProps); // Interpolate bare {name} refs from eval bindings (spec §6.4/§6.6). // Runs after meta/props interpolation so component contract takes // precedence. Only runs when a binding environment is in scope. - const textEvalEnv = yield* env; const final = textEvalEnv ? interpolateEvalBindings(interpolated, textEvalEnv.values) : interpolated; - result.push({ type: "text", content: final }); + result.push({ + type: "text", + content: final.replaceAll(ESCAPED_BRACE_PLACEHOLDER, "{"), + }); break; } @@ -618,14 +635,7 @@ export function* expandSegments( // the ambient error mode, so they are appended as they are. const projection = yield* ActiveProjection.get(); if (projection && projection.claims(segment)) { - yield* projection.expandClaimed( - segment, - parentMeta, - parentProps, - hideSet, - result, - elementPath, - ); + yield* projection.expandClaimed(segment, result, elementPath); break; } } @@ -1087,12 +1097,7 @@ function* expandEach( // expandComponent, so a projected resolves both lexical caller // bindings and the current component's bindings. const contextEnv = yield* env; - const callerEnv = segment.projectedEnv - ? derivedEnvironment(segment.projectedEnv, { - ...segment.projectedEnv.values, - ...(contextEnv?.values ?? {}), - }) - : contextEnv; + const callerEnv = layerEnvironments(segment.projectedEnv, contextEnv); const parentEvalScope = yield* evalScope; const enclosingLoop = yield* ActiveLoop.get(); @@ -1906,14 +1911,10 @@ function* expandComponent( // For multi-level nesting (Root → Provider → Instruction → ReviewBody), // the projectedEnv from the outer caller must be merged with the current // 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. const contextEnv = yield* env; - const callerEvalEnv = projectedEnv - ? derivedEnvironment(projectedEnv, { - ...projectedEnv.values, - ...(contextEnv?.values ?? {}), - }) - : contextEnv; + const callerEvalEnv = layerEnvironments(projectedEnv, contextEnv); // Recurse with augmented hide set. // Each component gets its own fresh binding environment so that @@ -1928,7 +1929,7 @@ function* expandComponent( // where innermost middleware runs first (innermost-wins), and // next() delegates to the parent scope's middleware. const newHideSet = new Set([...hideSet, name]); - const componentEnv: EvalEnv = { values: { ...validatedProps } }; + const componentEnv: EvalEnv = propsEnvironment(validatedProps); liveEnvironment(componentEnv); // Children are caller-provided content, not the component's own body. @@ -1964,10 +1965,18 @@ function* expandComponent( invocation, enclosing, children, - callerEnv: capturedCallerEnv, - meta: definition.meta, - props: validatedProps, - hideSet, + caller: { + env: capturedCallerEnv, + meta: callerMeta, + props: callerProps, + hideSet, + }, + authored: { + env: componentEnv, + meta: definition.meta, + props: validatedProps, + hideSet: newHideSet, + }, counter, callerLoop: siteLoop, ownPath: path, @@ -2299,10 +2308,7 @@ function* expandFunctionComponent( // Resolved once, here: an operand is what the call site meant, not what the // component's own body later did to the environment. const siteEnv = yield* env; - const captureEnv: Record = { - ...(projectedEnv?.values ?? {}), - ...(siteEnv?.values ?? {}), - }; + const captureEnv = layerEnvironments(projectedEnv, siteEnv)?.values ?? {}; const expansion = snapshot(path, name, position); @@ -2317,15 +2323,18 @@ function* expandFunctionComponent( invocation, enclosing, children, - // A function component's content inherits whatever environment the - // component installed for it — see ProjectionState.callerEnv. - callerEnv: undefined, - // ...but the content itself is the CALLER's markdown, so `{meta.x}` - // and `{props.x}` in it resolve against the expansion that wrote it, - // which projection has to be handed. - meta: callerMeta, - props: callerProps, - hideSet, + caller: { + env: undefined, + meta: callerMeta, + props: callerProps, + hideSet, + }, + authored: { + env: undefined, + meta: callerMeta, + props: callerProps, + hideSet, + }, counter, callerLoop: siteLoop, ownPath: path, @@ -2398,16 +2407,11 @@ function* expandFunctionComponent( }, { at: "min" }, ); - // Projection honored the way expandComponent and expandEach honor it: - // a component written inside content projected through sees - // 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. if (projectedEnv) { const siteEnv = yield* env; - const projectedSiteEnv = derivedEnvironment(projectedEnv, { - ...projectedEnv.values, - ...(siteEnv?.values ?? {}), - }); + const projectedSiteEnv = layerEnvironments(projectedEnv, siteEnv) ?? { values: {} }; yield* Component.around( { env: () => projectedSiteEnv, @@ -2619,10 +2623,7 @@ function* expressionEnv( // (contextEnv). The component's env takes priority because its eval // blocks run before and may define bindings that children // reference. The caller's env provides fallback bindings from the - const evalEnv = - explicitEnv && contextEnv - ? { values: { ...explicitEnv.values, ...contextEnv.values } } - : (contextEnv ?? explicitEnv); + const evalEnv = layerEnvironments(explicitEnv, contextEnv); if (!evalEnv) { throw new Error( @@ -2810,15 +2811,14 @@ function makeProjectFn(callerEnv: EvalEnv | undefined): ProjectFn { /** * Replace `` / `` in a segment list with the - * caller's children (partitioned by slot) and interpolate {meta}/{props} in - * text. Slot validation errors are emitted once, at the first projection - * point, tracked via the shared `state`. + * caller's children (partitioned by slot). Text interpolation waits until the + * expansion frame is installed, so a current binding named `props` is used. + * Slot validation errors are emitted once, at the first projection point, + * tracked via the shared `state`. */ function substituteSegmentList( segments: Segment[], slots: SlotMap, - meta: Record, - props: Record, project: ProjectFn, state: SubstitutionState, claim: ClaimFn, @@ -2842,9 +2842,6 @@ function substituteSegmentList( const element: ComponentElement = { ...segment, children: projected, selfClosing: false }; return [...pendingErrors, claim(element)]; } - if (segment.type === "text") { - return [{ ...segment, content: interpolate(segment.content, meta, props) }]; - } return [segment]; }); } @@ -2852,7 +2849,6 @@ function substituteSegmentList( /** * Replace `` and `` invocations with the * caller's children, partitioned by slot assignment. - * Also interpolates {meta.key} and {props.key} in text segments. * * When no `slot` props are present anywhere, this behaves identically * to the original single-slot substituteContent. @@ -2860,15 +2856,13 @@ function substituteSegmentList( function substituteContent( bodySegments: Segment[], children: Segment[], - meta: Record, - props: Record, callerEnv: EvalEnv | undefined, claim: ClaimFn, ): Segment[] { const slots = partitionBySlot(children); const state: SubstitutionState = { errorsEmitted: false }; const project = makeProjectFn(callerEnv); - return substituteSegmentList(bodySegments, slots, meta, props, project, state, claim); + return substituteSegmentList(bodySegments, slots, project, state, claim); } interface BodyChunk { @@ -3137,8 +3131,6 @@ function validateOutputProps(segment: ComponentElement): ErrorSegment | undefine function buildBody( bodySegments: Segment[], children: Segment[], - meta: Record, - props: Record, callerEnv: EvalEnv | undefined, claim: ClaimFn, path: string, @@ -3155,15 +3147,7 @@ function buildBody( chunks.push({ output: true, segments: [propsError], declaration: true }); continue; } - const outputSegments = substituteSegmentList( - segment.children, - slots, - meta, - props, - project, - state, - claim, - ); + const outputSegments = substituteSegmentList(segment.children, slots, project, state, claim); chunks.push({ output: true, segments: outputSegments, @@ -3172,7 +3156,7 @@ function buildBody( continue; } - const docSegments = substituteSegmentList([segment], slots, meta, props, project, state, claim); + const docSegments = substituteSegmentList([segment], slots, project, state, claim); chunks.push({ output: false, segments: docSegments, indexBase: index }); } @@ -3212,11 +3196,11 @@ export function* expandBody( path: string = "", ): Operation { if (!bodyHasOutput(bodySegments)) { - const substituted = substituteContent(bodySegments, children, meta, props, callerEnv, claim); + const substituted = substituteContent(bodySegments, children, callerEnv, claim); return yield* expandSegments(substituted, meta, props, hideSet, counter, owner, path); } - const chunks = buildBody(bodySegments, children, meta, props, callerEnv, claim, path); + const chunks = buildBody(bodySegments, children, callerEnv, claim, path); const output: Segment[] = owner ?? []; for (const chunk of chunks) { @@ -3345,7 +3329,7 @@ function* expandValueBody( produced = { value: yield* resolveReturnValue(componentName, returns, segment) }; continue; } - const docSegments = substituteSegmentList([segment], slots, meta, props, project, state, claim); + const docSegments = substituteSegmentList([segment], slots, project, state, claim); yield* runDocumentation(docSegments, meta, props, hideSet, counter, path, index); } diff --git a/packages/core/src/interpolate.ts b/packages/core/src/interpolate.ts index 8407f098..c0e07113 100644 --- a/packages/core/src/interpolate.ts +++ b/packages/core/src/interpolate.ts @@ -4,8 +4,6 @@ * Runtime operation — deterministic from inputs, no journal entry. */ -import type { Json } from "./types.ts"; - /** * Replace `{meta.key}` and `{props.key}` references in text. * @@ -15,11 +13,7 @@ import type { Json } from "./types.ts"; * - Arrays → comma-joined: `{meta.tags}` → `"alpha, beta"` * - Escaped braces: `\{not interpolated\}` → literal `{not interpolated}` */ -export function interpolate( - text: string, - meta: Record, - props: Record, -): string { +export function interpolate(text: string, meta: Record, props: unknown): string { return text.replace( /\\?\{(meta|props)\.([^}]+)\}/g, (match, namespace: string, keyPath: string) => { @@ -28,10 +22,7 @@ export function interpolate( return match.slice(1); } - const source = - namespace === "meta" - ? (meta as Record) - : (props as Record); + const source = namespace === "meta" ? meta : props; const value = getNestedValue(source, keyPath); if (value === undefined || value === null) { @@ -48,8 +39,13 @@ export function interpolate( /** * Access a nested value via dot-separated path. */ -export function getNestedValue(obj: Record, path: string): unknown { - return path - .split(".") - .reduce((current, key) => (current as Record)?.[key], obj as unknown); +export function getNestedValue(obj: unknown, path: string): unknown { + let current = obj; + for (const key of path.split(".")) { + if (current === null || typeof current !== "object") { + return undefined; + } + current = Reflect.get(current, key); + } + return current; } diff --git a/packages/core/src/projection.ts b/packages/core/src/projection.ts index 49e4701b..a72fad77 100644 --- a/packages/core/src/projection.ts +++ b/packages/core/src/projection.ts @@ -26,7 +26,7 @@ import { createContext } from "effection"; import type { Context, Operation } from "effection"; import type { ErrorMode } from "./errors.ts"; -import type { ComponentElement, Json, Segment } from "./types.ts"; +import type { ComponentElement, Segment } from "./types.ts"; /** * What to project. @@ -56,9 +56,6 @@ export interface ProjectionHandle { */ expandClaimed( element: ComponentElement, - meta: Record, - props: Record, - hideSet: Set, owner: Segment[], /** * This `` element's own structural path, which already descends diff --git a/packages/core/tests/cli-journal.test.ts b/packages/core/tests/cli-journal.test.ts index f21208bb..1fa569b9 100644 --- a/packages/core/tests/cli-journal.test.ts +++ b/packages/core/tests/cli-journal.test.ts @@ -151,4 +151,71 @@ describe("CLI journal integration", () => { (yield* readJournal(journalPath)).some((event) => event.description?.type === "exec"), ).toBe(true); }); + + it("CJ7: a review-style Output root exits zero for ordinary finding text", function* () { + const tmpDir = makeTmpDir(); + const documentPath = path.join(tmpDir, "review.md"); + const journalPath = path.join(tmpDir, "review.jsonl"); + yield* ensure(() => rm(tmpDir, { recursive: true, force: true })); + + yield* writeTextFile( + documentPath, + [ + "", + "", + "", + "", + "🔴 Finding text is report content, not an execution failure.", + "", + "", + ].join("\n"), + ); + + const result = yield* runCli( + ["run", documentPath, `--journal=${journalPath}`, "--raw"], + RUN, + ).expect(); + + expect(result.code).toBe(0); + expect(result.stdout).toContain("Finding text is report content"); + }); + + it("CJ8: an execution error beneath Output exits nonzero without postflight parsing", function* () { + const tmpDir = makeTmpDir(); + const documentPath = path.join(tmpDir, "review.md"); + const journalPath = path.join(tmpDir, "review.jsonl"); + yield* ensure(() => rm(tmpDir, { recursive: true, force: true })); + + yield* writeTextFile( + documentPath, + ["", "", "", "", ""].join("\n"), + ); + + const result = yield* runCli( + ["run", documentPath, `--journal=${journalPath}`, "--raw"], + RUN, + ).join(); + + expect(result.code).not.toBe(0); + }); + + it("CJ9: a failed Output run leaves its configured journal available", function* () { + const tmpDir = makeTmpDir(); + const documentPath = path.join(tmpDir, "review.md"); + const journalPath = path.join(tmpDir, "review.jsonl"); + yield* ensure(() => rm(tmpDir, { recursive: true, force: true })); + + yield* writeTextFile( + documentPath, + ["", "", "", "", ""].join("\n"), + ); + + const result = yield* runCli( + ["run", documentPath, `--journal=${journalPath}`, "--raw"], + RUN, + ).join(); + + expect(result.code).not.toBe(0); + expect(yield* exists(journalPath)).toBe(true); + }); }); diff --git a/packages/core/tests/eval-interpolate.test.ts b/packages/core/tests/eval-interpolate.test.ts index 61a5b3b8..a79946e7 100644 --- a/packages/core/tests/eval-interpolate.test.ts +++ b/packages/core/tests/eval-interpolate.test.ts @@ -148,4 +148,21 @@ describe("Tier P — Eval binding interpolation", () => { }); expect(result).toBe("5 files, port 3000"); }); + + it("P20: props dotted path resolves nested values", function* () { + const result = interpolateEvalBindings("version={props.release.version}", { + props: { release: { version: "1.2.3" } }, + }); + expect(result).toBe("version=1.2.3"); + }); + + it("P21: props dotted path preserves missing and null intermediates", function* () { + const result = interpolateEvalBindings( + "{props.release.version.value} {props.release.name.value}", + { + props: { release: { version: null } }, + }, + ); + expect(result).toBe("{props.release.version.value} {props.release.name.value}"); + }); }); diff --git a/packages/core/tests/function-components.test.ts b/packages/core/tests/function-components.test.ts index cbc1dfa7..7d8af85a 100644 --- a/packages/core/tests/function-components.test.ts +++ b/packages/core/tests/function-components.test.ts @@ -255,6 +255,44 @@ describe("Tier FC — Function components", () => { } }); + it("FC2-defaults: receives the validated object directly", function* () { + const tmpDir = makeTempDir(); + try { + writeFiles(tmpDir, { + "components/Defaults.ts": [ + "export const props = {", + ' type: "object",', + " properties: {", + " nested: {", + ' type: "object",', + ' properties: { value: { type: "string", default: "default" } },', + " additionalProperties: false,", + " },", + " },", + " additionalProperties: false,", + "};", + "", + "export default function*(props) {", + " const before = props.nested.value;", + ' props.nested.value = "changed";', + " return `${before}->${props.nested.value}`;", + "}", + ].join("\n"), + "doc.md": "", + }); + const output = yield* collect( + yield* execute({ + path: path.join(tmpDir, "doc.md"), + stream: new InMemoryStream(), + componentDirs: [path.join(tmpDir, "components"), tmpDir], + }), + ); + expect(output).toContain("default->changed"); + } finally { + cleanup(tmpDir); + } + }); + it("FC3: function component renders its content with content()", function* () { const tmpDir = makeTempDir(); try { diff --git a/packages/core/tests/loop.test.ts b/packages/core/tests/loop.test.ts index 18acce45..9dfaf355 100644 --- a/packages/core/tests/loop.test.ts +++ b/packages/core/tests/loop.test.ts @@ -1063,7 +1063,7 @@ describe("Tier LOOP — execution records", () => { "```js eval", "entered.push(1);", "output('STEP');", - "if (stall && entered.length === 3) { yield* suspend(); }", + "if (props.stall && entered.length === 3) { yield* suspend(); }", "```", "", "", @@ -1226,7 +1226,7 @@ describe("Tier LOOP — replay validates the terminal record", () => { "", "", "step", - "", + "", "", "", "", @@ -1261,7 +1261,7 @@ describe("Tier LOOP — replay validates the terminal record", () => { "---", "", "", - "step", + "step", "", "", "", diff --git a/packages/core/tests/named-slots.test.ts b/packages/core/tests/named-slots.test.ts index 1e3feb24..706f1276 100644 --- a/packages/core/tests/named-slots.test.ts +++ b/packages/core/tests/named-slots.test.ts @@ -134,10 +134,10 @@ function stubProvider(componentName: string): string { "```js persist eval", "yield* Sample.around({", " *sample([context], next) {", - " if (context.model !== undefined && context.model !== model) {", + " if (context.model !== undefined && context.model !== props.model) {", " return yield* next(context);", " }", - " return '[sampled-by-' + model + ':' + context.content.trim() + ']';", + " return '[sampled-by-' + props.model + ':' + context.content.trim() + ']';", " },", "}, { at: 'min' });", "```", diff --git a/packages/core/tests/props-binding.test.ts b/packages/core/tests/props-binding.test.ts new file mode 100644 index 00000000..a81da2e7 --- /dev/null +++ b/packages/core/tests/props-binding.test.ts @@ -0,0 +1,388 @@ +import { describe, it } from "@executablemd/test-support/bdd"; +import { expect } from "@executablemd/test-support/expect"; +import { InMemoryStream } from "@executablemd/durable-streams"; +import type { Json } from "@executablemd/durable-streams"; +import { useEchoExec, useStubFs } from "@executablemd/runtime/test"; +import { forEach } from "@effectionx/stream-helpers"; +import { execute } from "../src/execute.ts"; +import { beforeAll } from "@executablemd/test-support/bdd"; +import { useTempFileCompiler } from "../src/temp-file-compiler.ts"; +import type { Operation } from "effection"; + +function* runDocument( + files: Record, + props?: Record, +): Operation { + yield* useStubFs(files); + yield* useEchoExec(); + const execution = yield* execute({ path: "root.md", stream: new InMemoryStream(), props }); + const output = yield* forEach(function* () {}, execution.output); + const result = yield* execution; + return { output, result }; +} + +interface OperationResult { + output: string; + result: { ok: boolean }; +} + +const ROOT_PROPS = [ + "---", + "props:", + " type: object", + " properties:", + " name: { type: string }", + " declaredOnly: { type: string }", + " tags:", + " type: array", + " items: { type: string }", + " release:", + " type: object", + " properties:", + " version: { type: string }", + " required: [version]", + " additionalProperties: false", + " required: [name, release]", + " additionalProperties: false", + "---", +].join("\n"); + +describe("props binding", () => { + beforeAll(() => useTempFileCompiler()); + it("installs root props for text, eval, expression, and executable interpolation", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ + ROOT_PROPS, + "", + "text={props.name} bare={name} dotted={props.release.version} tags={props.tags} missing={props.release.missing} escaped=\\{props.name}", + "", + "```js eval", + "return `eval=${props.name} dotted=${props.release.version} bare=${typeof declaredOnly}`;", + "```", + "", + "```bash exec", + "echo {props.name}/{props.release.version}", + "```", + "", + ].join("\n"), + }, + { name: "Ada", declaredOnly: "field", tags: ["a", "b"], release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(true); + expect(output).toContain( + "text=Ada bare={name} dotted=1.2.3 tags=a, b missing= escaped={props.name}", + ); + expect(output).toContain("eval=Ada dotted=1.2.3 bare=undefined"); + expect(output).toContain("Ada/1.2.3"); + }); + + it("keeps eval-created locals independent from props", function* () { + const { output } = yield* runDocument( + { + "root.md": [ + ROOT_PROPS, + "", + "```js eval", + 'const name = "local";', + "return `${name}/${props.name}`;", + "```", + "local={name} prop={props.name}", + "", + ].join("\n"), + }, + { name: "Ada", release: { version: "1.2.3" } }, + ); + + expect(output).toContain("local/Ada"); + expect(output).toContain("local=local prop=Ada"); + }); + + it("does not begin body effects for invalid root props", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ROOT_PROPS, "", "```bash exec", "echo BODY_EFFECT", "```", ""].join("\n"), + }, + { name: 42, release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(false); + expect(output).not.toContain("BODY_EFFECT"); + }); + + it("uses caller props for projected content and callee props for authored content", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ + ROOT_PROPS, + "", + "```js eval", + 'const label = "caller";', + "```", + '', + "projected={props.name}", + "projected-label={label}", + "", + "```bash exec", + "echo {props.name}", + "```", + "", + "", + ].join("\n"), + "Wrapper.md": [ + "---", + "props:", + " type: object", + " properties:", + " name: { type: string }", + " forwarded: { type: string }", + " required: [name, forwarded]", + " additionalProperties: false", + "---", + "```js eval", + 'const label = "callee";', + "```", + "authored={props.name} forwarded={props.forwarded} authored-label={label}", + "```js eval", + "return `component-eval=${props.name}`;", + "```", + "```bash exec", + "echo authored-exec={props.name}", + "```", + "", + "```js eval", + 'return yield* render("rendered={props.name} label={label}");', + "```", + "", + "", + ].join("\n"), + "Child.md": [ + "---", + "props:", + " type: object", + " properties:", + " value: { type: string }", + " required: [value]", + " additionalProperties: false", + "---", + "child={props.value}", + "", + ].join("\n"), + }, + { name: "caller", release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(true); + expect(output).toContain("authored=callee"); + expect(output).toContain("forwarded=caller"); + expect(output).toContain("authored-label=callee"); + expect(output).toContain("component-eval=callee"); + expect(output).toContain("authored-exec=callee"); + expect(output).toContain("rendered=callee label=callee"); + expect(output).toContain("projected=caller"); + expect(output).toContain("projected-label=callee"); + expect(output).toContain("child=caller"); + expect(output).toContain("caller"); + expect(output).not.toContain("ERROR"); + }); + + it("restores the enclosing props binding after scoped and nested expansion", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ + ROOT_PROPS, + "", + 'text={props.name}', + "```js eval", + "return `eval=${props.name}`;", + "```", + "```bash exec", + "echo exec={props.name}", + "```", + "", + "after={props.name}", + '', + "", + ].join("\n"), + "Outer.md": [ + "---", + "props:", + " name: { type: string }", + "required: [name]", + "---", + 'outer={props.name}after={props.name}', + "", + ].join("\n"), + "Inner.md": [ + "---", + "props:", + " name: { type: string }", + "required: [name]", + "---", + "inner={props.name}", + "", + ].join("\n"), + }, + { name: "caller", release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(true); + expect(output).toContain("text=shadow"); + expect(output).toContain("eval=shadow"); + expect(output).toContain("exec=shadow"); + expect(output).toContain("after=caller"); + expect(output).toContain("outer=outer"); + expect(output).toContain("inner=inner"); + expect(output).toContain("after=outer"); + }); + + it("uses one validated object for the props binding and text interpolation", function* () { + const { output, result } = yield* runDocument({ + "root.md": '\n', + "Mutator.md": [ + "---", + "props:", + " type: object", + " properties:", + " nested:", + " type: object", + " properties:", + " value: { type: string }", + " required: [value]", + " additionalProperties: false", + " required: [nested]", + " additionalProperties: false", + "---", + "```js eval", + 'props.nested.value = "mutated";', + 'return yield* render("{props.nested.value}");', + "```", + "", + ].join("\n"), + }); + + expect(result.ok).toBe(true); + expect(output).toContain("mutated"); + }); + + it("uses defaults before root and Markdown-component body effects", function* () { + const { output, result } = yield* runDocument({ + "root.md": [ + "---", + "props:", + " type: object", + " properties:", + " name: { type: string, default: root-default }", + " additionalProperties: false", + "---", + "root-text={props.name}", + "```bash exec", + "echo root-effect={props.name}", + "```", + "", + "", + ].join("\n"), + "Defaulted.md": [ + "---", + "props:", + " type: object", + " properties:", + " name: { type: string, default: component-default }", + " additionalProperties: false", + "---", + "component-text={props.name}", + "```bash exec", + "echo component-effect={props.name}", + "```", + "", + ].join("\n"), + }); + + expect(result.ok).toBe(true); + expect(output).toContain("root-text=root-default"); + expect(output).toContain("root-effect=root-default"); + expect(output).toContain("component-text=component-default"); + expect(output).toContain("component-effect=component-default"); + }); + + it("does not begin a Markdown-component body effect for invalid props", function* () { + const { output, result } = yield* runDocument({ + "root.md": "\n", + "Invalid.md": [ + "---", + "props:", + " type: object", + " properties:", + " name: { type: string }", + " required: [name]", + " additionalProperties: false", + "---", + "```bash exec", + "echo INVALID_BODY_EFFECT", + "```", + "body={props.name}", + "", + ].join("\n"), + }); + + expect(result.ok).toBe(true); + expect(output).not.toContain("INVALID_BODY_EFFECT"); + expect(output).not.toContain("body="); + expect(output).toContain("Invalid"); + }); + + it("uses one shadowed props binding for text, eval, and exec, then restores it", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ROOT_PROPS, "", "", "after={props.name}", ""].join("\n"), + "Shadow.md": [ + "---", + "props:", + " type: object", + " properties: {}", + " additionalProperties: false", + "---", + "```js eval", + 'const props = { name: "shadow" };', + "return `eval=${props.name}`;", + "```", + "text={props.name}", + "```js eval", + "return `after-eval=${props.name}`;", + "```", + "```bash exec", + "echo exec={props.name}", + "```", + "", + ].join("\n"), + }, + { name: "caller", release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(true); + expect(output).toContain("eval=shadow"); + expect(output).toContain("after-eval=shadow"); + expect(output).toContain("text=shadow"); + expect(output).toContain("exec=shadow"); + expect(output).toContain("after=caller"); + }); + + it("allows an authored props binding to shadow the namespace locally", function* () { + const { output, result } = yield* runDocument( + { + "root.md": [ + ROOT_PROPS, + "", + 'shadowed{props} {props.name}', + "", + ].join("\n"), + }, + { name: "caller", release: { version: "1.2.3" } }, + ); + + expect(result.ok).toBe(true); + expect(output).toContain("shadowed"); + expect(output).not.toContain("caller"); + }); +}); diff --git a/packages/core/tests/root-props.test.ts b/packages/core/tests/root-props.test.ts index 1aa5132b..6c36a530 100644 --- a/packages/core/tests/root-props.test.ts +++ b/packages/core/tests/root-props.test.ts @@ -93,7 +93,7 @@ function* runDoc(files: Record, path: string, props?: Record { - it("RP1: supplied props reach interpolation and bare bindings", function* () { + it("RP1: supplied props reach interpolation without bare prop bindings", function* () { const { output, result } = yield* runDoc({ "hello.md": GREETING }, "hello.md", { name: "Ada", loud: true, @@ -101,7 +101,7 @@ describe("Tier RP — root document properties", () => { expect(result.ok).toBe(true); expect(output).toContain("Hello, Ada!"); expect(output).toContain("loud=true"); - expect(output).toContain("bare=Ada"); + expect(output).toContain("bare={name}"); }); it("RP2: schema defaults apply to the root", function* () { @@ -232,7 +232,7 @@ describe("Tier RS — concise props declarations", () => { expect(result.ok).toBe(true); expect(output).toContain("Hello, Ada!"); expect(output).toContain("loud=false"); - expect(output).toContain("bare=Ada"); + expect(output).toContain("bare={name}"); }); it("RS2: the concise and full spellings behave identically", function* () { diff --git a/packages/core/tests/sample-component.test.ts b/packages/core/tests/sample-component.test.ts index bd8d3d18..6ff83b23 100644 --- a/packages/core/tests/sample-component.test.ts +++ b/packages/core/tests/sample-component.test.ts @@ -58,10 +58,10 @@ function stubProviderWithInstructions(componentName: string): string { "```js persist eval", "yield* Sample.around({", " *sample([context], next) {", - " if (context.model !== undefined && context.model !== model) {", + " if (context.model !== undefined && context.model !== props.model) {", " return yield* next(context);", " }", - " return '[sampled-by-' + model + ':' + context.content.trim() + '|system:' + (context.system || 'none') + ']';", + " return '[sampled-by-' + props.model + ':' + context.content.trim() + '|system:' + (context.system || 'none') + ']';", " },", "}, { at: 'min' });", "```", @@ -88,10 +88,10 @@ function stubProvider(componentName: string): string { "```js persist eval", "yield* Sample.around({", " *sample([context], next) {", - " if (context.model !== undefined && context.model !== model) {", + " if (context.model !== undefined && context.model !== props.model) {", " return yield* next(context);", " }", - " return '[sampled-by-' + model + ':' + context.content.trim() + ']';", + " return '[sampled-by-' + props.model + ':' + context.content.trim() + ']';", " },", "}, { at: 'min' });", "```", diff --git a/smoke-test/Guide/Summary.md b/smoke-test/Guide/Summary.md index 14ce853c..1328d3aa 100644 --- a/smoke-test/Guide/Summary.md +++ b/smoke-test/Guide/Summary.md @@ -39,7 +39,7 @@ cat <<'TABLE' | attached-service provider | persist ephemeral eval scopes middleware | | provider pattern | StubProvider installs Sample middleware | | per-component eval scope | Each provider gets isolated middleware | -| props in env.values | model prop available in eval blocks | +| props namespace in env.values | `props.model` available in eval blocks | | Sample component | , with children | | output() function | Sample component calls output() | | renderChildren() closure | Sample component captures children | diff --git a/smoke-test/InnerStubProvider.md b/smoke-test/InnerStubProvider.md index 1f67750c..271530aa 100644 --- a/smoke-test/InnerStubProvider.md +++ b/smoke-test/InnerStubProvider.md @@ -14,11 +14,11 @@ props: ```js persist eval yield* Sample.around({ *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } const sys = context.system ? '|system:' + context.system : ''; - return '[response-from-' + model + sys + ']'; + return '[response-from-' + props.model + sys + ']'; }, }, { at: 'min' }); ``` diff --git a/smoke-test/StubProvider.md b/smoke-test/StubProvider.md index 41707620..a35d101d 100644 --- a/smoke-test/StubProvider.md +++ b/smoke-test/StubProvider.md @@ -14,11 +14,11 @@ props: ```js persist eval yield* Sample.around({ *sample([context], next) { - if (context.model !== undefined && context.model !== model) { + if (context.model !== undefined && context.model !== props.model) { return yield* next(context); } const sys = context.system ? '|system:' + context.system : ''; - return '[response-from-' + model + sys + '|content:' + context.content + ']'; + return '[response-from-' + props.model + sys + '|content:' + context.content + ']'; }, }, { at: 'min' }); ``` diff --git a/smoke-test/TypedList.md b/smoke-test/TypedList.md index c33f5d49..cb4145d7 100644 --- a/smoke-test/TypedList.md +++ b/smoke-test/TypedList.md @@ -11,5 +11,5 @@ props: --- ```ts eval -return files.join(", "); +return props.files.join(", "); ``` diff --git a/smoke-test/TypedRows.md b/smoke-test/TypedRows.md index 97d674c7..07cca43b 100644 --- a/smoke-test/TypedRows.md +++ b/smoke-test/TypedRows.md @@ -23,5 +23,5 @@ props: --- ```ts eval -return rows.map((row) => `${row.symbol}@${row.line}:${row.level}`).join(", "); +return props.rows.map((row) => `${row.symbol}@${row.line}:${row.level}`).join(", "); ``` diff --git a/smoke-test/Verdict.md b/smoke-test/Verdict.md index 370da5c5..7ed86052 100644 --- a/smoke-test/Verdict.md +++ b/smoke-test/Verdict.md @@ -18,8 +18,8 @@ VERDICT_DOC_LEAK never reaches the caller. ```ts eval const verdict = { - passed: findings.length === 0, - summary: findings.length === 0 ? "no findings" : `${findings.length} findings`, + passed: props.findings.length === 0, + summary: props.findings.length === 0 ? "no findings" : `${props.findings.length} findings`, }; ``` diff --git a/specs/code-review-agent-spec.md b/specs/code-review-agent-spec.md index 53b9f01c..aac4620c 100644 --- a/specs/code-review-agent-spec.md +++ b/specs/code-review-agent-spec.md @@ -92,7 +92,7 @@ interface PR { deleted: DiffFile[]; directories: Set; addedSource: string; - diffPreview: string; // addedSource truncated to 80K chars + diffPreview: string; // addedSource truncated to 40K chars stats: { totalFiles: number; additions: number; @@ -149,7 +149,8 @@ function parseDiff( - Test file detection: `*.test.ts`, `*.spec.ts`, `__tests__/`, `test/` - Config file detection: `*.config.*`, `.*rc`, `tsconfig*`, `package.json` - Type declaration detection: `*.d.ts` -- `diffPreview`: `addedSource` truncated to 80,000 characters +- `diffPreview`: `addedSource` truncated to 40,000 characters so the + correctness prompt stays within the provider context limit on large PRs - `directories`: unique top-level dirs at depth 2 ### 3.4 Package structure @@ -1049,6 +1050,8 @@ Zero eval blocks. title: PR Review --- + + ```ts eval const BASE_SHA = process.env.BASE_SHA ?? "HEAD~1"; const HEAD_SHA = process.env.HEAD_SHA ?? "HEAD"; @@ -1087,6 +1090,8 @@ const pr = parseDiff(rawDiff, rawFiles, { + + ```` ### 7.2 `.reviews/ReviewPR.local.md` (local with Ollama) @@ -1160,32 +1165,56 @@ jobs: - uses: denoland/setup-deno@v2 - - uses: actions/cache@v4 - with: - path: .reviews/journal.jsonl - key: xmd-review-${{ github.event.pull_request.head.sha }} - restore-keys: | - xmd-review-${{ github.event.pull_request.base.sha }} + - name: Install dependencies + run: deno task deps + + - name: Build the checked-out xmd binary + run: deno task build - name: Run review env: PR_NUMBER: ${{ github.event.pull_request.number }} PR_TITLE: ${{ github.event.pull_request.title }} - PR_BODY: ${{ github.event.pull_request.body }} BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_REPOSITORY: ${{ github.repository }} DEEPINFRA_TOKEN: ${{ secrets.DEEPINFRA_TOKEN }} - run: deno task xmd run .reviews/ReviewPR.md + run: | + ./dist/xmd run .reviews/ReviewPR.md \ + --component-dir .reviews/components \ + --component-dir .reviews/policies \ + --component-dir packages/core/components \ + -j .reviews/journal.jsonl \ + --verbose ``` -### Journal caching +The executable body of the CI root is enclosed in one top-level `` +region. An execution failure therefore remains a failed `DocumentResult`, +fails `execute()`, and makes `xmd run` exit nonzero. Review findings are report +text and do not fail the run. The workflow does not inspect journal records or +rendered error markers after the command; it only uploads the journal with +`if: always()`. + +### Journal artifact + +The journal is uploaded by head SHA after every run, including failed runs, so +an execution failure remains inspectable without making the artifact a second +workflow result channel. + +### Durable-boundary safety + +Doctor compatibility probes emit aggregate facts only: diagnostic count, +import-noise count or ratio, file counts, crash state, and available rule IDs. +Actual Oxlint output is normalized before it enters a capture or durable event +to `message`, `ruleId` or `code`, `severity`, `file`, and the minimal line and +column span. PR review keeps diagnostics for changed files; repository analysis +may keep all files. Source excerpts, causes, rendered source, URLs, and other +arbitrary payload are discarded. -The journal is cached by head SHA. On re-run of the same SHA, full -replay — no git commands, no API calls, no LLM calls. On new commits, -`restore-keys` falls back to the base SHA for partial replay of -shared component imports. +GitHub credentials are created inside non-serializable header factories at the +request site. Tokens are not placed in eval bindings or durable output, and +secret detection remains enabled. --- diff --git a/specs/decisions.md b/specs/decisions.md index 22555d97..fa8342ed 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -394,10 +394,8 @@ calling task. The Sample component has three optional props: `prompt`, `model`, `params` — declared in the props schema but absent from its `required` -array. When such a prop has no `default` and is not provided, Ajv -`useDefaults` leaves the key off the validated props, so it is absent -from `env.values`. The variable is then undefined in the eval block -scope, causing `ReferenceError`. +array. The component keeps a complete, predictable validated props object +for its body and routing semantics. ### Decision @@ -405,11 +403,6 @@ All three props declare `default: ""` in the props schema. The eval block converts empty strings to `undefined` for routing semantics: `model || undefined`, `params || undefined`. -**Why not just leave the prop optional:** Without a `default`, the -variable simply doesn't exist in `env.values`, so `transformBlock` -doesn't include it in the preamble. The eval block would get -`ReferenceError: params is not defined`. - **Why empty string, not undefined:** an optional property with no `default` never adds the key to validated props. Empty string is a legitimate default that ensures the key exists. The `|| undefined` diff --git a/specs/executable-mdx-spec.md b/specs/executable-mdx-spec.md index 6e3c3d7b..2ab311b4 100644 --- a/specs/executable-mdx-spec.md +++ b/specs/executable-mdx-spec.md @@ -1247,8 +1247,10 @@ export interface EvalEnv { } ``` -Created fresh at the start of component expansion. Each eval block reads -bindings from `values` (via env preamble) and writes new bindings back +Created fresh at the start of root or Markdown-component expansion. A root or +Markdown component installs the exact validated, defaulted props object under +the single `values.props` key; it does not spread properties into `values`. +Each eval block reads bindings from `values` (via env preamble) and writes new bindings back (via env-write transforms). The current environment is read contextually via the `env` value (§5.5); the expansion engine provides it scope-locally around each component body, so eval blocks within a @@ -1626,8 +1628,14 @@ const rendered = yield* render("# Dynamic heading\n\n"); Both closures are injected in `expandComponent()` (in `src/expand.ts`) after the component's `EvalEnv` is created but before `expandSegments` -processes the component body. They capture the expansion context -(meta, validated props, hide set, eval scope) at injection time. +processes the component body. They capture explicit projection frames: +`renderChildren()` and `useContent()` use the caller's metadata, validated +props, hide set, and ordinary binding environment; `render(markdown)` uses the +component-authored metadata, validated props, hide set, and environment. +Structural `` projection preserves the caller's props object while +retaining the existing ordinary-binding layering of the current authored +frame. No projection surface promises that every caller ordinary binding is +visible. Both use `parentEvalScope`, not `childEvalScope`. Children are caller-provided content and expand in the caller's scope context. @@ -1641,10 +1649,10 @@ tree. Inner components create their own child scopes off `parentEvalScope`, and ancestor middleware is visible through Effection's scope prototype chain. -Both install the caller's binding environment and eval scope as -scope-local Component providers (§5.5) around their `expandSegments` -calls, so the full expansion context is available regardless of which -task the closure runs in (e.g., inside `evalScope.eval()`). +Each installs its selected binding environment and eval scope as scope-local +Component providers (§5.5) around its `expandSegments` call, so the full +expansion context is available regardless of which task the closure runs in +(e.g., inside `evalScope.eval()`). #### Non-serializable @@ -2789,8 +2797,9 @@ Expanding a component invocation proceeds as: props are validated against the declared props (§6.3.5, §6.5). - **Body expansion.** The caller's children are substituted into `` positions (§6.3), and the body is expanded in a fresh binding environment - seeded with the validated props, exposing `renderChildren()` / `render()` - (§4.8) to eval blocks. Expression props resolve in the caller's scope. When + whose `props` binding points at the validated props object, exposing + `renderChildren()` / `render()` (§4.8) to eval blocks. Expression props + resolve in the caller's scope. When the component declares ``, its placement is validated before any body content executes and only its declared regions render (§6.9). - **Capture (`as=`).** With `as="binding"`, the rendered result is written to @@ -2883,10 +2892,15 @@ the once-only slot errors of §6.3.3 — and the resolved segments ride on the invocation's content scope (§4.4), so a resource projected content creates stops when that scope is halted, before the component releases its own. -Only the resource scope moves. The binding environment, `{meta.key}` / -`{props.key}` inputs, cycle-detection hide set and block counter are the body's, -exactly as they are for content spliced in place, and expression props on -projected children still resolve against the caller's environment. +Only the resource scope moves. Projected content keeps the caller's metadata, +validated props object, cycle-detection hide set, and block counter. For +ordinary bindings, `renderChildren()` and `useContent()` retain their caller +environment, while structural `` layers the caller's props binding +with the existing authored/current environment precedence. Component-authored +body content and `render(markdown)` keep the component frame. Expression props +on projected children resolve against the caller's environment, while authored +content resolves against the component's environment. The props guarantee is +deliberate; #305 does not broaden ordinary-binding lookup. The error mode travels with them. A content task does not inherit the documentation or `` frame the `` sits in, so the error mode is @@ -2912,8 +2926,7 @@ the content scope. function substituteContent( bodySegments: Segment[], children: Segment[], - meta: Record, - props: Record, + callerEnv: EvalEnv | undefined, ): Segment[] { const slots = partitionBySlot(children); return bodySegments.flatMap((segment) => { @@ -2926,17 +2939,15 @@ function substituteContent( // Default slot projection return slots.default; } - if (segment.type === "text") { - return [{ - ...segment, - content: interpolate(segment.content, meta, props), - }]; - } return [segment]; }); } ``` +Text interpolation is deferred until the expansion frame is installed. This +keeps a scoped authored binding named `props` authoritative for text just as it +is for eval blocks and executable-block interpolation. + #### 6.3.5 Reserved prop names The names `slot` and `as` are reserved. Declaring either in a @@ -2976,7 +2987,7 @@ against the JSX props passed from the invocation site. function interpolate( text: string, meta: Record, - props: Record, + props: unknown, ): string { return text.replace(/\{(meta|props)\.([^}]+)\}/g, (match, namespace, keyPath) => { const source = namespace === "meta" ? meta : props; @@ -2987,11 +2998,13 @@ function interpolate( }); } -function getNestedValue(obj: Record, path: string): unknown { - return path.split(".").reduce( - (current, key) => (current as Record)?.[key], - obj as unknown, - ); +function getNestedValue(obj: unknown, path: string): unknown { + let current = obj; + for (const key of path.split(".")) { + if (current === null || typeof current !== "object") return undefined; + current = Reflect.get(current, key); + } + return current; } ``` @@ -3013,15 +3026,16 @@ Text segments undergo two interpolation passes in sequence: ``` text segment → remend (heal markdown) - → interpolate {meta.key}, {props.key} ← first pass - → interpolateEvalBindings {name} ← second pass + → interpolate {meta.key}, {props.key} from the current frame ← first pass + → interpolateEvalBindings {name}, {props.name} ← second pass → output ``` The second pass (`interpolateEvalBindings`) runs on text segments when an `EvalEnv` is present on the scope. It resolves bare `{name}` -references from `env.values`. This allows eval block exports to flow -into surrounding prose naturally: +and dotted `{props.name}` references from `env.values`. This allows eval +block exports and the validated props namespace to flow into surrounding +prose naturally: ````markdown ```ts eval @@ -3034,13 +3048,14 @@ const dashboard = "https://status.example.test/staging"; Renders: `staging status: https://status.example.test/staging.` -**Precedence:** `{meta.*}` and `{props.*}` resolve first because they -are the component's declared interface. If a component declares -`props: { title: ... }` and an eval block also exports `title`, the -prop wins (accessed via `{props.title}`). Bare `{title}` resolves via -the second pass against `env.values`. There is no actual collision -because the two passes match different syntax: dotted (`{ns.key}`) vs -bare (`{identifier}`). +**Precedence:** Text `{meta.*}` references resolve from the dedicated metadata +frame. Text `{props.*}` references, eval blocks, and executable-block content +all read the current `env.values.props` binding when it exists; a frame's +validated props object is the fallback for expansion without an environment. +An authored binding named `props` therefore shadows the validated namespace in +all three surfaces and normal scope, commit, and restoration rules apply. A +declared prop does not create `{title}`; bare `{title}` resolves only when an +eval, capture, loop, or component-return binding named `title` exists. **Escaping:** `\{name}` is left as literal `{name}` in the output. Both passes respect `\{` escaping — the backslash is consumed and @@ -3709,24 +3724,27 @@ substituted content is used to build the subprocess command. #### Interpolation syntax and precedence -Bare `{name}` references use JavaScript identifier syntax: +Eval-binding references use JavaScript identifier syntax with optional dotted +paths: ``` -\{([a-zA-Z_$][a-zA-Z0-9_$]*)\} +\{([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\} ``` -Namespaced references (`{meta.*}`, `{props.*}`) contain a `.` and -are excluded — they are handled by the existing interpolation pass -for text segments. Bare references only match against `env.values`. -If `env.values` has no key `name`, the reference `{name}` is left +`{meta.*}` is handled by the text interpolation pass. `{props.*}` is +handled by that pass in text and by this pass in executable block content; +both use the `props` root in `env.values`. If `env.values` has no matching +root or an intermediate path is missing/null, the reference is left verbatim. Non-string values are converted via `String()`. -Note: `{meta.*}` and `{props.*}` interpolation applies only to -**text segments**, not to code block content. Code blocks receive -only eval binding interpolation (`{name}`). To use a prop value in a -code block, capture it into a binding via an `eval` block first. -Text segments receive both passes: `{meta.*}`/`{props.*}` first, -then bare `{name}` from `env.values`. +Note: `{meta.*}` interpolation applies only to text segments. The +validated `props` object is installed at `env.values.props`, so +`{props.name}` uses the same dotted-path interpolation in executable block +content and in text. Eval blocks read `props.name` directly through the env +preamble. A prop does not create a bare `{name}` binding; use an eval, +capture, loop, or component-return binding when bare interpolation is wanted. +Text segments receive the text pass for `{meta.*}` and `{props.*}`, then the +eval-binding pass. #### Where interpolation runs @@ -3735,14 +3753,15 @@ Eval binding interpolation runs in `expandSegments` in two places: 1. **Code blocks** — immediately before the modifier chain is composed for a `codeBlock` segment. By the time any modifier factory receives `ctx.content`, the content is already fully interpolated — modifiers - are not responsible for text preparation. + are not responsible for text preparation. This resolves `{props.name}` + from `env.values.props` without changing the modifier API. 2. **Text segments** — after `{meta.*}`/`{props.*}` interpolation - (§6.4). The second pass resolves bare `{name}` references from + (§6.4). The second pass resolves bare and dotted references from `env.values` when an `EvalEnv` is present on the scope. Eval blocks skip interpolation entirely — they access bindings directly -via the env preamble (`const { name } = env;`). Interpolating would +via the env preamble (`const { props } = env;`). Interpolating would mangle JS template literals like `` `${name}` `` into `$`. ```typescript @@ -3753,8 +3772,17 @@ function interpolateEvalBindings( // Protect escaped braces: \{ → placeholder const escaped = content.replaceAll("\\{", PLACEHOLDER); const interpolated = escaped.replace( - /\{([a-zA-Z_$][a-zA-Z0-9_$]*)\}/g, - (match, key) => key in bindings ? String(bindings[key]) : match, + /\{([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\}/g, + (match, key) => { + let value: unknown = bindings; + for (const part of key.split(".")) { + if (value == null || typeof value !== "object" || !(part in value)) { + return match; + } + value = value[part]; + } + return String(value); + }, ); // Restore escaped braces: placeholder → literal { return interpolated.replaceAll(PLACEHOLDER, "{"); @@ -3810,7 +3838,7 @@ props: --- ```bash service=server exec -{command} +{props.command} ``` ```ts persist ephemeral eval @@ -3860,6 +3888,26 @@ eval, interpolation and the journal cannot observe the endpoint. Partial replay runs both blocks again to reconstruct a current process and middleware chain. A completed replay expands nothing and starts no process. +#### Props namespace (DEC-EX-09) + +Root documents and Markdown components install one `props` binding whose +value is the exact object returned by validation. They do not spread declared +properties into `env.values`: + +```typescript +const componentEnv: EvalEnv = { values: { props: validatedProps } }; +``` + +Validation and defaults complete before this environment is installed or any +body effect starts. Text interpolation continues to resolve `{props.command}` +through the text pass. Executable block content resolves `{props.command}` +through eval binding interpolation, and eval blocks read `props.command` +directly. Eval-created values such as `endpoint` remain ordinary bare +bindings. A declaration of `command` therefore does not create `{command}`. + +Function components keep their separate contract: their function receives the +validated object directly and does not receive the Markdown environment helper. + #### Nesting providers Provider components nest naturally — each establishes its own eval @@ -3920,7 +3968,7 @@ props: ```js persist eval const childrenOutput = yield* renderChildren(); -const content = childrenOutput || prompt || ''; +const content = childrenOutput || props.prompt || ''; const sampleResult = yield* Sample.operations.sample({ stdout: content, @@ -3928,9 +3976,9 @@ const sampleResult = yield* Sample.operations.sample({ exitCode: 0, command: content, language: 'markdown', - params: params || undefined, + params: props.params || undefined, componentName: 'Sample', - model: model || undefined, + model: props.model || undefined, }); output(sampleResult); @@ -3941,7 +3989,7 @@ output(sampleResult); 1. `renderChildren()` expands and renders the component's children. For self-closing invocations, this returns an empty string. -2. `content` falls back to the `prompt` prop if children are empty. +2. `content` falls back to the `props.prompt` value if children are empty. 3. `Sample.operations.sample()` is called directly from the eval block. The enclosing eval operation journals the block result, including output. 4. `output(sampleResult)` sets the block's rendered output to the @@ -5958,7 +6006,7 @@ visible warning blocks, gather into a separate error report). | C6 | Mutual cycle | A→B→A → ErrorSegment | | C7 | Depth limit | 65 levels deep → ErrorSegment | | C8 | Frontmatter interpolation | `{meta.title}` → replaced with value | -| C9 | Props interpolation | `{props.name}` → replaced with invocation prop | +| C9 | Props namespace interpolation | `{props.name}` → replaced with the validated invocation prop in text, eval, and executable block content | | C10 | Missing interpolation key | `{meta.nonexistent}` → empty string | | C11 | Nested key access | `{meta.config.db.host}` → deep value | | C12 | No Content slot | Children silently discarded | @@ -5998,6 +6046,8 @@ visible warning blocks, gather into a separate error report). | C45 | **Object-shape rejected** | A nested object with `required: [symbol]` / `additionalProperties: false` rejects a missing `symbol` or an unknown key → PropValidationError | | C46 | **Nested default filled** | A row omitting `line` (declared `{ type: number, default: 0 }`) resolves with `line` set to `0` | | C47 | **Nested enum rejected** | A property with `enum: [a, b]` nested inside an object/array item rejects a value outside the set → PropValidationError | +| C48 | **No bare prop binding** | Declaring `name` makes `{props.name}` available but leaves `{name}` verbatim until authored code creates that binding | +| C49 | **Validated object identity** | The environment and function-component argument observe the exact defaulted object returned by validation | ### Tier D — Code execution and modifier middleware @@ -6189,11 +6239,11 @@ visible warning blocks, gather into a separate error report). | # | Test | Verify | |---|------|--------| -| K1 | Fresh env per component | Each component expansion gets its own `EvalEnv` | +| K1 | Fresh env per Markdown component | Each root or Markdown component expansion gets its own `EvalEnv` with one `props` namespace; function components receive their argument directly | | K2 | Env shared across blocks in same component | Block 1 and block 2 in same component share `env.values` | | K3 | `serializeExports` filters non-JSON | Functions, symbols, circular refs excluded | | K4 | `serializeExports` preserves JSON values | Numbers, strings, objects, arrays round-trip correctly | -| K5 | Eval merges serializable bindings | After the block, `env.values` contains current exports | +| K5 | Eval merges serializable bindings | After the block, `env.values` contains current exports alongside `props`, without spreading prop fields | | K6 | Component `as` writes to invocation env | Binding is visible to downstream siblings at call site | | K7 | `` is not a component boundary | Eval/exec inside `` use parent env/scope and journal normally | @@ -6628,7 +6678,7 @@ Defined in [Workflow runs](./workflow-spec.md) §9.4 and §9.6–§9.7. |---|------|--------| | P1 | Bare binding resolves from `env.values` | `{port}` with `env.values.port = 49821` → `"49821"` in content | | P2 | Bare binding with no env entry left verbatim | `{port}` with no `port` in `env.values` → `"{port}"` unchanged | -| P3 | Bare binding does not match namespaced refs | `{meta.title}` and `{props.name}` not affected by eval binding pass | +| P3 | Dotted props binding resolves | `{props.release.version}` traverses `env.values.props` and missing/null intermediate paths remain verbatim | | P4 | Multiple bindings in one content | `{host}:{port}` → both substituted | | P5 | Non-string binding converted via `String()` | `env.values.port = 49821` (number) → `"49821"` | | P6 | Binding interpolation runs before modifier chain | Resulting `ctx.content` in modifier contains substituted value | @@ -7073,7 +7123,7 @@ must preserve the trace for diagnosis or remove it before starting a new run. | 36 | `daemon` is a terminal modifier that ignores `next` | Process lifetime ≠ command result; `exec` in the chain satisfies the §3.2 detection rule without invoking `durableExec` | | 37 | `daemon` uses `evalScope`, not the durable run scope | Lifetime matches component expansion — daemon lives for `` and dies with the component, not the whole document run | | 38 | `daemon` produces no journal entry | The process is an ephemeral resource and starts on every run | -| 39 | Eval binding interpolation uses bare `{name}` syntax | Distinct from `{meta.key}` and `{props.key}` namespaces; local eval bindings are local variables, not namespaced data; regex excludes names containing `.` to avoid conflicts | +| 39 | Eval binding interpolation uses authored binding syntax | Bare `{name}` resolves authored eval/capture/loop/return bindings; dotted `{props.name}` traverses the validated props namespace, while `{meta.key}` remains text interpolation | | 40 | Eval binding interpolation runs in the expansion engine, not inside modifier factories | Modifiers transform execution results — they are not responsible for preparing source text; one interpolation site in `expandSegments` is consistent with how text segment interpolation already works, and keeps modifier factories free of knowledge about the binding environment | | 41 | Service allocation belongs to a host adapter | Holding an OS-selected port across spawn is host process and networking behavior; shared runtime and document code use provider-neutral `API.Service` | | 42 | Service endpoints are live bindings | The endpoint identifies an execution-owned process and is reconstructed during partial replay, so it cannot enter durable eval, interpolation or the journal | @@ -7088,13 +7138,13 @@ must preserve the trace for diagnosis or remove it before starting a new run. | 51 | `renderChildren`/`render` install the caller's environment and `parentEvalScope` as scope-local providers | Children are caller-provided content and expand in the caller's scope context; the component's `childEvalScope` sequential channel is for its own `persist eval` blocks, not for expanding caller content; children may create resources (nested components, daemons) but their lifecycle is bound by their place in the expansion tree; installing providers inside the closure ensures the correct context is visible regardless of which task it runs in | | 52 | `durableSample` routes through `EvalScope` | Sample Api middleware installed by provider components with `persist ephemeral eval` lives in the eval scope's task hierarchy; routing through `evalScope.eval()` ensures the middleware chain is found | | 53 | Sample component calls `Sample.operations.sample()` directly | The enclosing eval operation journals the complete block result | -| 54 | Sample component props default to empty string, not undefined | `validateProps` omits optional props with no default from `env.values`, causing `ReferenceError` in eval blocks; empty-string defaults ensure the variables exist; `model \|\| undefined` converts empty to undefined for routing semantics | +| 54 | Sample component props default to empty string, not undefined | Defaults remain part of the validated `props` object; `model \|\| undefined` and `params \|\| undefined` preserve routing semantics without creating bare prop bindings | | 55 | `daemon()` uses `shell: true` | Matches `bash exec` block semantics — the same command string passed to `bash -c` is passed to the shell; handles shell expansions and PATH lookups correctly | | 56 | Provider installs middleware inside its invocation | Middleware closes over the current live endpoint and remains lexically scoped to the subtree that owns the service | | 57 | Routing key is `model`, not a separate `name` prop | Model identity is the natural key — it unifies "which server to route to" with "which model to request"; a separate `name` prop would require keeping two values in sync with no added expressiveness | | 58 | `context.model === undefined` routes to innermost provider | Omitting a model is the common case for single-provider documents; innermost-wins matches how middleware chains work — handlers installed later sit higher in the chain and are traversed first | | 59 | Provider components use ordinary generated-module imports | Provider-specific client functions may be imported explicitly; executable.md supplies `Sample`, `when`, `fetch` and the contextual document bindings | -| 60 | Props pre-populated into `env.values` at component invocation | Code block content uses bare `{name}` binding interpolation from `env.values`; props must enter `env.values` at invocation time to be accessible in code blocks; consistent with how eval bindings work | +| 60 | Props namespaced in `env.values` at root and Markdown-component invocation | Validation completes before the exact object is installed as `env.values.props`; text and executable content use `{props.name}`, eval blocks read `props.name`, and declared fields are not spread as bare bindings | | 61 | Provider HTTP calls use `@effectionx/fetch` | Calls remain Effection operations under structured cancellation and the provider's lexical middleware | | 62 | The XMD service handshake and application health are separate | The handshake record proves the attached service owns the assigned endpoint; an application may still use `when` for a later domain-specific condition | | 63 | `stdio: "inherit"` is the default for `daemon()` | During development, seeing server logs in the terminal is valuable; production deployments can pass `stdio: "ignore"`; the executable.md `daemonFactory` passes no stdio option, defaulting to `"inherit"` | @@ -7120,13 +7170,13 @@ must preserve the trace for diagnosis or remove it before starting a new run. | 83 | Capture trims trailing whitespace | Exec stdout commonly ends with newline; trimming avoids downstream interpolation/comparison bugs while preserving leading/interior whitespace | | 84 | Capture assignment is not independently journaled | Captured value is derived during current expansion; no extra journal entry is needed | | 88 | Eval binding interpolation extends to text segments | Documents should be readable prose with embedded data references, not JavaScript template literals inside eval blocks | -| 89 | `{meta.*}` / `{props.*}` resolve before bare `{name}` | Component contract (frontmatter) takes precedence over internal eval state; dotted vs bare syntax prevents actual collisions | +| 89 | Lexical props namespace and authored binding precedence | `{props.*}` uses the validated component/root namespace, projected caller content keeps the caller's props object, authored component content uses the callee's frame, and a local binding named `props` follows normal shadow/restoration rules | | 90 | `\{` escaping applies to both passes | Consistent escaping behavior regardless of which pass would match; pre-existing gap in §6.6 fixed for both code blocks and text segments | | 85 | Eval block `return` as rendered output | Eval blocks can produce output via `return "text"` in addition to `output("text")`; `output()` wins if both used; null/undefined returns produce no output; lets a component's whole body be one conditional expression | | 86 | `sample` modifier removed | All LLM calls go through the `` component; provider-specific call helpers are not built-in modifier behavior | | 87 | `SampleContext` simplified to content-centric shape | Changed from exec-centric `{stdout, stderr, exitCode, command, language}` to content-centric `{content, model?, params?, system?, componentName?}`; providers build their own messages directly instead of relying on `buildDefaultMessages` | -| 91 | Projected children carry caller's eval env | Children substituted via `` are tagged with `projectedEnv`. Expression props on projected children resolve against merged env (caller + component), with component bindings taking precedence. Follows React's lexical scoping model. | -| 92 | Multi-level projection env propagation | When `expandComponent` receives `projectedEnv`, it merges it with the current context env before tagging the next level's children. Creates a cumulative chain: Root → Provider → Instruction → ReviewBody all carry root bindings. Innermost-wins on collision. | +| 91 | Projected children preserve caller props without changing ordinary lookup | Children substituted via `` preserve the caller's metadata, validated props object, hide set, and counter. Structural projection keeps the existing ordinary-binding layering of the authored/current frame; `renderChildren()` and `useContent()` retain the caller's ordinary environment. Authored content keeps the component frame. | +| 92 | Multi-level projection preserves caller props | When `expandComponent` receives `projectedEnv`, it layers ordinary bindings while retaining the lexical caller's `props` object. Nested projections never replace caller props with the callee's initial namespace. | | 93 | AST-based user import extraction in eval blocks | `ImportDeclaration` nodes in eval blocks are extracted via acorn's `allowImportExportEverywhere` and hoisted to module top level by `compileBlock`. TypeScript `import type` normalized to spaces before parse, extracted from original source. | | 94 | `` uses CSS selectors via remark + `unist-util-select` | Standard CSS selector syntax on markdown AST (mdast); reuses existing remark dependency; supports attribute selectors, combinators, pseudo-classes; matches Web platform conventions for querying tree structures | | 95 | `select` falls back to full content on no match | Non-destructive — authors can add `select` to existing Captures without breaking behavior if the selector doesn't match; avoids silent data loss | diff --git a/specs/oxlint-sensor-spec.md b/specs/oxlint-sensor-spec.md index 3c36a205..92c67747 100644 --- a/specs/oxlint-sensor-spec.md +++ b/specs/oxlint-sensor-spec.md @@ -694,15 +694,22 @@ Skipping type-aware probe — prerequisites not met. ```bash silent exec -RESULT=$(npx oxlint --type-aware --tsconfig {tsconfigPath} --format json 2>.reviews/probe-stderr.tmp || true) -STDERR=$(cat .reviews/probe-stderr.tmp 2>/dev/null || echo "") -rm -f .reviews/probe-stderr.tmp -echo "{\"diagnostics\":$RESULT,\"stderr\":\"$STDERR\"}" +RESULT=$(npx oxlint --type-aware --tsconfig {tsconfigPath} --format json 2>/dev/null || true) +printf '%s' "$RESULT" | jq -c ' + def entries: + if type == "array" then . + elif (.diagnostics? | type) == "array" then .diagnostics + else [] + end; + def rule: if (.ruleId? | type) == "string" then .ruleId elif (.code? | type) == "string" then .code else "unknown" end; + entries | map({message: (.message // ""), ruleId: rule, file: (.file // .filename // "")}) as $diagnostics + | {diagnosticCount: ($diagnostics | length), importNoiseCount: ([$diagnostics[] | select((.message | ascii_downcase | contains("cannot find module")) or (.ruleId | ascii_downcase | contains("import")))] | length), filesAnalyzed: ([$diagnostics[].file | select(length > 0)] | unique | length), filesSkipped: 0, importErrors: ([$diagnostics[] | select((.message | ascii_downcase | contains("cannot find module")) or (.ruleId | ascii_downcase | contains("import")))] | length), availableRuleIds: ([$diagnostics[].ruleId] | unique), tsgolintCrashed: false} +' || echo '{"diagnosticCount":0,"importNoiseCount":0,"filesAnalyzed":0,"filesSkipped":0,"importErrors":0,"availableRuleIds":[],"tsgolintCrashed":false}' ``` -{"diagnostics":[],"stderr":""} +{"diagnosticCount":0,"importNoiseCount":0,"filesAnalyzed":0,"filesSkipped":0,"importErrors":0,"availableRuleIds":[],"tsgolintCrashed":false} @@ -733,27 +740,24 @@ const TYPE_AWARE_RULES = [ "no-unnecessary-boolean-literal-compare", ]; -let probe = { diagnostics: [], stderr: "" }; -try { probe = JSON.parse(probeResult); } catch { /* malformed */ } - -const diagnostics = Array.isArray(probe.diagnostics) - ? probe.diagnostics : []; - -const importNoise = diagnostics.filter(d => - d.message?.includes("Cannot find module") - || d.message?.includes("cannot find") - || d.ruleId?.includes("import") -); - -const fileSet = new Set(diagnostics.map(d => d.file).filter(Boolean)); -const noiseRatio = diagnostics.length > 0 - ? importNoise.length / diagnostics.length : 0; +let probe = { + diagnosticCount: 0, + importNoiseCount: 0, + filesAnalyzed: 0, + filesSkipped: 0, + importErrors: 0, + availableRuleIds: [], + tsgolintCrashed: false, +}; +try { probe = { ...probe, ...JSON.parse(probeResult) }; } catch { } -const tsgolintCrashed = typeof probe.stderr === "string" - && probe.stderr.includes("tsgolint") - && (probe.stderr.includes("panic") - || probe.stderr.includes("OOM") - || probe.stderr.includes("fatal")); +const diagnosticCount = typeof probe.diagnosticCount === "number" + ? probe.diagnosticCount : 0; +const importNoiseCount = typeof probe.importNoiseCount === "number" + ? probe.importNoiseCount : 0; +const noiseRatio = diagnosticCount > 0 + ? importNoiseCount / diagnosticCount : 0; +const tsgolintCrashed = probe.tsgolintCrashed === true; const typeAwareAvailable = canProbeTypeAware && !tsgolintCrashed; @@ -779,9 +783,10 @@ const doctor = { tsconfigExists, nodeModulesExists, typeAwareAvailable, - filesAnalyzed: fileSet.size, - filesSkipped: new Set(importNoise.map(d => d.file).filter(Boolean)).size, - importErrors: importNoise.length, + filesAnalyzed: typeof probe.filesAnalyzed === "number" ? probe.filesAnalyzed : 0, + filesSkipped: typeof probe.filesSkipped === "number" ? probe.filesSkipped : 0, + importErrors: typeof probe.importErrors === "number" ? probe.importErrors : 0, + availableRuleIds: Array.isArray(probe.availableRuleIds) ? probe.availableRuleIds : [], bloatRulesAvailable, bloatRulesMissing, recommendation, @@ -801,8 +806,8 @@ return JSON.stringify(doctor); Type-aware linting available. {bloatRulesAvailable.length} bloat -rules active across {fileSet.size} files. -Import noise: {importNoise.length} diagnostics +rules active across {probe.filesAnalyzed} files. +Import noise: {probe.importNoiseCount} diagnostics ({(noiseRatio * 100).toFixed(1)}%). @@ -1064,6 +1069,8 @@ DIFF: title: PR Review --- + + ```ts eval const BASE_SHA = process.env.BASE_SHA ?? "HEAD~1"; const HEAD_SHA = process.env.HEAD_SHA ?? "HEAD"; @@ -1158,6 +1165,8 @@ const diagnostics = parseDiagnostics(rawDiagnostics, pr, doctor); + + ```` ### 8.2 `ReviewPR.local.md` (local with Ollama) @@ -1287,9 +1296,11 @@ jobs: - uses: denoland/setup-deno@v2 - - run: deno install + - name: Install dependencies + run: deno task deps - - run: npm install -g oxlint oxlint-tsgolint + - name: Build the checked-out xmd binary + run: deno task build - name: Run review env: @@ -1300,7 +1311,13 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_REPOSITORY: ${{ github.repository }} DEEPINFRA_TOKEN: ${{ secrets.DEEPINFRA_TOKEN }} - run: deno task review --verbose + run: | + ./dist/xmd run .reviews/ReviewPR.md \ + --component-dir .reviews/components \ + --component-dir .reviews/policies \ + --component-dir packages/core/components \ + -j .reviews/journal.jsonl \ + --verbose - name: Upload journal if: always() @@ -1311,8 +1328,27 @@ jobs: retention-days: 30 ``` -`deno install` creates `node_modules/` (required by tsgolint). -`npm install -g oxlint oxlint-tsgolint` provides the binaries. +The workflow builds and runs the checked-out `./dist/xmd` after +`deno task deps`, so the review documents and executable use one revision. +The root document encloses its executable body in ``; execution +failures therefore exit nonzero through the normal `DocumentResult` path. +Review finding text remains ordinary report output and does not fail the job. + +The workflow uploads the journal with `if: always()` and does not interpret +journal records or rendered Markdown after XMD exits. + +### Durable diagnostic boundary + +The Doctor compatibility probe emits only aggregate facts: availability or +crash state, diagnostic and import-noise counts, file counts, and available +rule identifiers. Actual Oxlint output is normalized before capture to +`message`, `ruleId` or `code`, `severity`, `file`, and minimal line/column +information. PR review restricts diagnostics to changed files; repository +analysis retains all files. Source excerpts, causes, rendered source, URLs, +and arbitrary payload do not cross the exec/durable boundary. + +Credential-bearing headers are created inside non-serializable functions at +request time. Default secret detection remains enabled. ### Separate enforcement jobs (unchanged from base spec) diff --git a/specs/release-process-spec.md b/specs/release-process-spec.md index efa68c0f..1d478011 100644 --- a/specs/release-process-spec.md +++ b/specs/release-process-spec.md @@ -71,9 +71,9 @@ they already satisfy the lockfile. Left alone they keep the previous release's number. Only the members whose `name` is an `@executablemd` package change; an unrelated dependency that happens to share the old version number must not. -The bump touches nothing else. PR Review and Repo Analysis install -the latest published release rather than a pinned version, so preparing a -release never depends on a binary that release has not published yet. +The bump touches nothing else. PR Review and Repo Analysis build the checked-out +revision with the repository-pinned Deno version, so their executable documents +run against the source they review rather than against a published binary. ## 3. Workflows @@ -84,6 +84,19 @@ release never depends on a binary that release has not published yet. version is already released, the draft's notes carry a warning banner saying the manifests need bumping; once bumped, the banner clears and the draft's tag and title default to the guard-passing `v`. +- **`review.yml`** (`pull_request`): checks out the pull request, installs the + repository-pinned Deno version, runs `deno task deps`, and runs `deno task + build`. It executes the resulting `./dist/xmd` against `.reviews/ReviewPR.md`. + The root review body is enclosed in a top-level `` region, so an + execution failure propagates through `DocumentResult` and makes the command + fail. Review findings remain ordinary report text and do not fail execution. + The workflow uploads `.reviews/journal.jsonl` with `if: always()` and does + not parse journal records or rendered error markers after XMD exits. +- **`repo-analysis.yml`** (`workflow_dispatch`): checks out the requested ref, + prepares and builds that ref's `./dist/xmd`, and executes + `.reviews/AnalyzeRepoCI.md`, whose root body uses the same `` failure + contract. Its report, journal, and run metadata remain unconditional + artifacts. - **`release.yml`** (`push: tags v*`): a preflight job validates the tag against `packages/cli/deno.json`; on mismatch it flags the just-published release on the Releases page — caution note in the notes, a failed title, and the diff --git a/specs/root-document-props-spec.md b/specs/root-document-props-spec.md index b4c96085..f55feffc 100644 --- a/specs/root-document-props-spec.md +++ b/specs/root-document-props-spec.md @@ -223,10 +223,53 @@ local references, and `additionalProperties`. Resolved properties have the same behavior at the root as props passed to an imported Markdown component: -- `{props.name}` interpolates the property explicitly. -- Root eval blocks receive each property as a binding. -- Bare binding interpolation such as `{name}` reads the root evaluation - environment. +- The validated, defaulted object is installed under one `props` binding. +- `{props.name}` interpolates the property explicitly in text and executable + block content. +- Root eval blocks read `props.name` directly. +- A declaration does not create a bare `{name}` binding. Bare references read + only bindings authored by eval, capture, loop, or component-return behavior. + +The object under `props` is the exact object returned by validation. Defaults +are present before the first body effect, and nested properties remain live +references for the duration of the component invocation. + +For example, a root can use the namespace in every execution surface: + +````markdown +--- +props: + type: object + properties: + name: { type: string } + release: + type: object + properties: { version: { type: string } } + required: [name, release] +--- + +Hello {props.name}, release {props.release.version}. + +```ts eval +const greeting = `Hi ${props.name}`; +``` + +```bash exec +echo {props.release.version} +``` +```` + +The declaration does not make `{name}` available. An authored `const name = +...` or capture may create that independent bare binding. When content is +projected through nested Markdown components, the caller's `props` object stays +lexical for projected content while the component's own body and +`render(markdown)` use the callee's namespace. + +Text, direct eval, and executable-block interpolation all use the current +`props` binding. An authored binding literally named `props` shadows the +validated namespace in its scope and the validated binding is restored when +that scope ends. This rule changes only the props root; existing ordinary +binding lookup during projection remains unchanged. The core validation boundary applies to every host. Command-line parsing may report an error earlier, but it does not replace whole-object validation.