Fix LspJournalCli journal failures on a clean checkout (#313)#336
Draft
anderson-joyle wants to merge 1 commit into
Draft
Fix LspJournalCli journal failures on a clean checkout (#313)#336anderson-joyle wants to merge 1 commit into
anderson-joyle wants to merge 1 commit into
Conversation
The `all-the-things-indexing` journal failed on a clean `main`, and the
`--all` suite aborted with a `Text hash mismatch` while leaving the working
tree dirty. Two independent causes:
1. Fixture mutation / cross-journal contamination: the language server
rewrites workspace files while indexing them (e.g. migrating legacy
`# Name:` comment headers into `mcs.metadata:` blocks). Running a journal
directly against the committed fixtures mutated them, and during `--all`
one journal's rewrites broke a later journal's text-hash check, aborting
the run.
2. Line-ending churn: `SerializationOptions.Indented` left `NewLine`
unset, so regenerated baselines used `Environment.NewLine` (CRLF on
Windows), producing spurious diffs against the LF-committed baselines.
Fix (LspJournalCli harness only -- no language-server behavior changed):
- Add `TempWorkspaceCopy`: run the server against a throwaway copy of the
fixture workspace so committed fixtures stay pristine and journals never
contaminate one another. Text-hash validation stays bound to the pristine
committed fixture, so recorded hashes remain stable across runs and OSes.
- Wire the sandbox into `RunCommand`, and reorder the baseline-promotion so
params are relativized back to `${workspace}` before scrubbing (scrubbing
then hashes the committed fixture, not the rewritten sandbox copy).
- Pin `SerializationOptions.Indented` to `NewLine = "\n"` so regenerated
baselines are byte-stable LF across operating systems.
- Refresh five stale baselines that the `--all` abort had been masking:
`all-the-things-indexing` (comment-only knowledge file now parses as a
source file and correctly yields empty diagnostics) plus four completion
journals (additive schema growth -- new properties/actions, no removals).
Adds unit tests covering the isolation invariant (`TempWorkspaceCopyTests`)
and LF serialization (`SerializationOptionsTests`). Verified: single journal
and `--all` (13/13) pass against latest `main` with no fixture mutation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8272c534-802c-40ae-9342-ce6d1d0d181e
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes LspJournalCli reliability on clean checkouts by preventing fixture mutation during journal runs and making regenerated journal baselines byte-stable across operating systems. It updates the journal harness to run the LSP server against an isolated temp copy, pins JSON indentation newlines to LF, and refreshes several recorded journal baselines to match current server output.
Changes:
- Run journals against a disposable workspace copy to prevent committed fixture mutation and cross-journal contamination.
- Pin
System.Text.Jsonindented output to\nto avoid Windows CRLF churn in committed journal baselines. - Refresh multiple journal baselines and add unit tests covering workspace-copy isolation and newline behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/Transport/SerializationOptions.cs | Pins indented JSON newline to LF for byte-stable baselines. |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/Execution/TempWorkspaceCopy.cs | Adds disposable temp workspace copy used to sandbox server indexing/migrations. |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/Commands/RunCommand.cs | Wires sandbox workspace into journal execution and adjusts baseline promotion/scrubbing order. |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli.UnitTests/TempWorkspaceCopyTests.cs | Adds tests ensuring fixture isolation and cleanup behavior. |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli.UnitTests/SerializationOptionsTests.cs | Adds test asserting indented JSON output uses LF (not CRLF). |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/TestAssets/journals/all-the-things-indexing.journal.json | Updates expected diagnostics for indexing scenario (knowledge file yields empty diagnostics). |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/TestAssets/journals/all-the-things-completions.journal.json | Refreshes expected completion results to match current schema surface area. |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/TestAssets/journals/completion-smoke.journal.json | Refreshes expected completion results (adds new completion items). |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/TestAssets/journals/completion-error-context.journal.json | Refreshes expected completion results in error-state scenarios (adds new completion items). |
| src/LanguageServers/PowerPlatformLS/Tools/LspJournalCli/TestAssets/journals/completion-depth.journal.json | Refreshes expected completion results (adds new properties/actions). |
Comment on lines
+54
to
+56
| var destination = Path.Combine(Path.GetTempPath(), "lsp-journal-" + Guid.NewGuid().ToString("N")); | ||
| CopyDirectory(source, destination); | ||
| return new TempWorkspaceCopy(destination); |
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.
Summary
Fixes #313.
LspJournalClidid not pass on a clean checkout ofmain:all-the-things-indexingjournal failed at step 21 (missingpublishDiagnostics).-- --allaborted withText hash mismatch for 'topics/Greeting.mcs.yml'.TestAssets/fixtures/, leaving the working tree dirty.Root causes
# Name:comment headers intomcs.metadata:blocks). Running a journal directly against the committed fixtures mutated them, and during--allone journal''s rewrites broke a later journal''s text-hash check, aborting the whole run.SerializationOptions.IndentedleftNewLineunset, soSystem.Text.JsonusedEnvironment.NewLine(CRLF on Windows) when regenerating baselines, producing spurious diffs against the LF-committed baselines.Fix (harness only — no language-server behavior changed)
TempWorkspaceCopy: the server now runs against a throwaway copy of the fixture workspace, so committed fixtures stay pristine and journals never contaminate one another. Text-hash validation stays bound to the pristine committed fixture, so recorded hashes are stable across runs and operating systems.RunCommand: wires in the sandbox and reorders baseline promotion so params are relativized back to${workspace}before scrubbing (scrubbing then hashes the committed fixture, not the rewritten sandbox copy).SerializationOptions.Indented: pinned toNewLine = "\n"so regenerated baselines are byte-stable LF.--allabort had been masking:all-the-things-indexing— the comment-only knowledge file now parses as a source file and correctly yields empty diagnostics.all-the-things-completions,completion-depth,completion-error-context,completion-smoke— purely additive schema growth (new properties/actions such asipa,soundsLike,sensitivityLevel,InvokeMcpToolAction); no items removed, so no regressions.Tests
TempWorkspaceCopyTests— proves the isolation invariant (mutating the copy never touches the source).SerializationOptionsTests— proves indented journal output is LF, not CRLF.Verification
main(includes Add IntelliSense (warning diagnostic + quick fix) for inline comments #334 inline-comment rule and Fix knowledge file projection in CLI agent #335 projection fix) and rebuilt the server.-- --allboth pass (13/13), with no fixture mutation (git statusonTestAssets/fixtures/stays clean).Note
NotificationSetMatchingTests.Notifications_AreVerifiedAsMultisetflakes under full-suite parallel load (passes in isolation, ~30ms). It reproduces without these changes and is a pre-existing, timing-based flake unrelated to #313 — left untouched here; worth a separate follow-up.