You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Enhance Markdown viewing inside the existing Wizard and SDD canvas previews, without introducing another plugin or canvas.
Add a shared GFM reader with a heading outline, responsive TOC drawer, related-artifact selection, local-reference navigation, history, and revision-aware refresh.
Keep reading and navigation separate from workflow execution. Bind reads and clarification controls to the existing canvas workspace, context, artifact, revision, and owning command.
Package the reader and parser independently in both plugins, with dependency notices and source/hash/import-closure verification. Installed plugins do not need reader development dependencies or a sibling checkout.
SDD retains feature targeting, constitution, progress, optional quality gates, new-feature flow, targeted clarification, and artifact deep links.
Regression checks cover conventional template paths, active composition-stack preset overrides, and workflow progress/focus restoration after returning from the reader.
Intentional behavior changes: existing documents are readable while setup or execution is gated; Run/Rerun remains gated. Clarification markers in code examples or HTML comments are inert, and stale reviewed answers require refresh/reconfirmation.
Scope And Review Guide
Reader-only changes for both canvases. No repository browsing/authentication/cloning, preview installers or distribution, generated agent files, historical release records, branding, version bumps, or changes to unrelated plugins/skills/presets.
No upstream files deleted.
94 files total, including 16 generated runtime/copy/notice files and one build lockfile. Generated payloads are marked linguist-generated; canonical implementation remains visible for review.
39 actual-shell browser journeys passed, including responsive layout, navigation, freshness, clarification, focus, and bounded-document performance.
Typecheck, lint, build, package verification, Markdown links, and CI configuration checks passed. Both dependency audits report zero vulnerabilities. No candidate tests skipped.
Untouched upstream-test replay: all 281 Wizard tests passed; 3 of 4 SDD tests passed. The remaining static assertion required hiding feature cards before setup and is intentionally replaced by coverage for readable artifacts with execution still gated.
Native And Hosted Acceptance
Final candidate: 5e3b459a1666ec28a3087f3595ca68e60efc44bc.
Hosted Windows and Linux validation: both jobs passed on this exact candidate. These are fork-hosted checks; upstream PR check runs are not currently attached.
Tested in GitHub Copilot App 1.1.20 on Windows using an owned synthetic repository and real project-local Wizard and SDD providers. Served payloads were hash-verified; normal installed plugins and real projects were not modified.
Both original artifact entry points, rendered revisions/GFM, related-document navigation/history, and return focus passed.
Native compact SDD pointer close and restored focus passed after fixing the reader stacking context. Wizard compact visibility and accessibility dismissal passed; its pointer hit-testing is covered by actual-shell browser tests, not claimed as a completed native pointer check.
Real SDD Analyze ran once through the generated skill: all 146 tracked files remained unchanged, with no additions.
Real targeted SDD clarification ran once with 30 days.: only the synthetic specification changed; 145 other tracked files remained unchanged, with no additions. Its resolved question disappeared.
Wizard draft save/reopen/cancel/back-discard passed without workflow dispatch. All 37 source/configuration files remained unchanged; only expected Wizard control-plane state changed.
Explicit native Refresh recovered evicted contexts in both final canvases, retained the selected document, reset history, and did not rebind discarded/resolved answers.
Native testing found and regression-tested two reader defects: compact drawer stacking and explicit refresh of expired contexts. Recovery revalidates selected-document membership and clears old context bindings; automatic updates do not renew the existing 15-minute expiry. A temporary App WebView loading/focus issue was resolved by reacquiring native controls and opening the preview normally; the final SDD recovery check then passed without another code change.
Limits: actual Windows App acceptance, not native macOS/Linux certification or marketplace-release validation. Raw local evidence and diagnostic captures remain local. PR remains draft; no merge or release performed.
Add one shared reader and independently packaged assets inside the existing canvas preview surfaces. Preserve existing workflows and action contracts, and add scoped reads, revision-bound clarification validation, navigation, and regression coverage. Keep preview distribution and repository discovery outside this change.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Relative links can escape the selected review scope, while clarification nesting, polling cost, and incomplete parser type-check coverage also remain unresolved.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
Each SDD poll calls signature(), which recursively rediscovers and stats up to 10,000 entries for every retained review context. Contexts remain retained after the reader closes, so the 1.5-second timer can repeatedly perform tens or hundreds of thousands of filesystem operations for up to 15 minutes. Cache/incrementally update the signature or explicitly unregister closed reader contexts instead of rescanning them on every poll. plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/markdown-reader/src/clarificationParser.ts:24
This check excludes only text whose immediate parent is a link. Markdown such as [**[NEEDS CLARIFICATION: Q?]**](research.md) has an emphasis/strong parent, so it becomes a trusted clarification binding and the renderer can place a button inside a navigable anchor. Exclude text with any link ancestor in both the parser and renderer transform. plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/markdown-reader/tsconfig.json:20
npm run typecheck does not include the newly added parser Vite configuration, so type errors in that build path are silently skipped even though CI reports the reader configuration as typechecked.
Relative links are read before checking that the normalized target belongs to the current feature/composition scope. A client can POST ../../other-feature/private.md (with a valid source revision) and receive any workspace Markdown file, contradicting the scope boundary. Reject targets outside the scope’s explicit candidates and allowed roots before reading them.
The 1.5-second SDD poll calls discoverCandidates for every still-valid selection, while closing the reader does not remove selections and up to 20 contexts remain valid for 15 minutes. Each discovery can inspect 10,000 entries before this loop increments inspected only for returned Markdown candidates, so repeated opens—or directories containing mostly non-Markdown files—can cause hundreds of thousands of filesystem operations per poll. Deduplicate/cache signatures by scope and retire selections on close/expiry; also account for discovered.inspectedCount rather than only candidate count.
Relative-link resolution reads and registers any Markdown path that normalizes inside the workspace, without checking that it belongs to the current feature/composition scope. For example, a link from specs/001-feature/spec.md to ../002-other/private.md normalizes to a valid path and bypasses the membership boundary. Check the target against the current scope's candidates/roots before readArtifact, and return an inert result when it is outside that scope.
The new parser Vite configuration is omitted from the TypeScript project, so npm run typecheck does not check it despite the validation workflow's typecheck step. Include vite.parser.config.ts alongside the reader config so both build configurations are covered.
Relative links are normalized before authorization, so ../002-other/spec.md from specs/001-fixture/spec.md becomes specs/002-other/spec.md, passes path validation, and is read even though it is outside the current feature/composition scope. Revalidate the normalized target against the context's candidates or allowed roots before reading and registering it; the synchronized SDD copy needs the same correction.
While an SSE client is connected, this runs every 1.5 seconds and review.signature() rescans up to 10,000 entries and stats every candidate. Large feature trees will therefore cause continuous high filesystem load even when nothing changes. Cache discovery/signature state or invalidate it from scoped filesystem changes rather than performing a full scan on every poll.
const signature = await entry.review.signature();
const sig = stateSignature(state, signature);
Files reviewed: 78/94 changed files
Comments generated: 0 new
Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Choose available Markdown siblings in natural filename order, preserve revision-matched positions, and prevent background refresh from leaving the reader unmounted. Add folder-boundary, loading, refresh-race and actual-shell regressions.
This recomputes a full bounded discovery separately for every retained context on every 1.5-second SDD poll. Closing the reader does not remove its context, so repeated opens can leave up to 20 contexts and trigger up to 20 directory scans (each potentially inspecting 10,000 entries) per poll for 15 minutes. Deduplicate scans by current scope and prune/invalidate contexts when a reader closes, or maintain one watcher-derived signature per scope.
Extension phase IDs are shaped like commands/speckit.assess.intake, so this fallback produces speckit.commands/speckit.assess.intake. Because the later command loop skips the same artifact path, the selected descriptor keeps this invalid owner; the reader then filters out its clarification bindings and cannot rerun the real extension command. Derive the owner from the matching snapshot command, or strip the commands/ prefix.
Refresh control is a no-op after context load failure
When opening /api/review/context fails, showState("error") still renders a Refresh control, but activeContext remains null and this guard makes that control a permanent no-op. Let an explicit refresh reopen contextSelection when no context exists, so the advertised retry can recover from an initial transient failure.
Nested clarification markers inside links or headings are trusted
Checking only the immediate parent does not actually exclude markers inside headings or links when formatting is nested. For example, [**[NEEDS CLARIFICATION: Q?]**](research.md) visits the text with a strong parent, so it becomes a trusted clarification binding and the renderer can place a Clarify button inside the link. Track ancestors and reject any text beneath heading, link, or linkReference.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enhance Markdown viewing inside the existing Wizard and SDD canvas previews, without introducing another plugin or canvas.
Existing Offerings Preserved
Intentional behavior changes: existing documents are readable while setup or execution is gated; Run/Rerun remains gated. Clarification markers in code examples or HTML comments are inert, and stale reviewed answers require refresh/reconfirmation.
Scope And Review Guide
Reader-only changes for both canvases. No repository browsing/authentication/cloning, preview installers or distribution, generated agent files, historical release records, branding, version bumps, or changes to unrelated plugins/skills/presets.
linguist-generated; canonical implementation remains visible for review.Verification
Verified locally on Windows with Node 24.19.0 and Edge 153.0.4234.32:
Native And Hosted Acceptance
Final candidate:
5e3b459a1666ec28a3087f3595ca68e60efc44bc.Hosted Windows and Linux validation: both jobs passed on this exact candidate. These are fork-hosted checks; upstream PR check runs are not currently attached.
Tested in GitHub Copilot App 1.1.20 on Windows using an owned synthetic repository and real project-local Wizard and SDD providers. Served payloads were hash-verified; normal installed plugins and real projects were not modified.
30 days.: only the synthetic specification changed; 145 other tracked files remained unchanged, with no additions. Its resolved question disappeared.Native testing found and regression-tested two reader defects: compact drawer stacking and explicit refresh of expired contexts. Recovery revalidates selected-document membership and clears old context bindings; automatic updates do not renew the existing 15-minute expiry. A temporary App WebView loading/focus issue was resolved by reacquiring native controls and opening the preview normally; the final SDD recovery check then passed without another code change.
Limits: actual Windows App acceptance, not native macOS/Linux certification or marketplace-release validation. Raw local evidence and diagnostic captures remain local. PR remains draft; no merge or release performed.