Conversation
# Conflicts: # core/planning-suggestions.ts # docs/implementation/planning-suggestions.md # test/planning-suggestions.test.ts
Contributor
Author
|
@copilot review |
mchwang
marked this pull request as ready for review
September 24, 2026 18:26
Contributor
Author
|
@copilot review |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Real authoring-output validation remains blocked on the D5 provider and settlement gate.
Review effort: Lite
Findings: None
What changed in this PR
Adds synthetic planning acceptance fixtures and Store-backed integration tests, while documenting the remaining D5 live-provider dependency.
Changes:
- Added acceptance tests for imports, persistence, identity, replay, invalidation, malformed responses, hostile input, and command approval.
- Added synthetic plan, edit, and hostile-input fixtures.
- Documented acceptance coverage and deferred real-provider validation.
| File | Description |
|---|---|
test/planning-acceptance.test.ts |
Integrated planning acceptance tests |
test/fixtures/planning/synthetic-plan.json |
Synthetic plan fixture |
test/fixtures/planning/synthetic-edits.json |
Synthetic suggestion fixture |
test/fixtures/planning/hostile-input.json |
Hostile-input fixture |
docs/implementation/planning-acceptance.md |
Acceptance matrix and D5 dependency |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mchwang
marked this pull request as draft
September 24, 2026 18:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes the automatically closed stacked draft #37 after E3's base branch was merged and deleted.
Adds dedicated synthetic integration fixtures for JSON/YAML import, the eight T18 broken-plan cases, stable identity, replay/sibling invalidation, SQLite reopen, malformed responses, hostile prompt input, and exact command approval. It composes merged E2/E3 with the real Store and its revision/snapshot-bound request lifecycle. No fixture is represented as vendor output or isolation evidence.
Depends on merged E3 #35; ownership #29. Acceptance matrix and D5 handoff:
docs/implementation/planning-acceptance.md.Validated pushed head:
43e228404b28f5c508180b72e0843476a1193f77.npm test— 393 passed across 17 filesnpm run typecheck— passednpm run test:browser— 43 passedgit diff --check— passedThe synthetic fixture slice is complete and self-reviewed. Copilot review round 1 on the exact head returned Findings: None with zero inline threads. No findings were declined.
The recorded real Claude/Codex authoring-output portion remains blocked by D5 in #28. D3 is currently in PR #44; D4 and D5 have not landed. This draft must not merge or claim T9/T18 completion until D5 supplies the pinned provider adapters, bounded extraction, permissions, and settlement gate needed to capture and replay real outputs.
Historical review on #37 found one persistence assertion gap. The generated-draft test now verifies the changed summary, full current and reopened Store value, and unchanged historical revision. The lesson is covered by existing
AGENTS.mddurable-versus-visible state guidance; no duplicate rule is needed. The current round added no reusable lesson.