Repository navigation
Close the loose ends the last round of merges left - #169
Merged
Merged
Conversation
A post-merge review of everything that landed on 19 and 20 August. The combined state builds clean and all 2332 tests pass, the solution matches what is on disk, and the two headline renames are coherent — so this is the residue rather than a repair. A locked session file no longer costs an agent its whole list. Both readers guarded the parse of a live-session file but not the open of it, and these files are written by the process they describe: a sharing violation escaped to LocalAgentSessionSource, which reads an IOException as "this agent's folder cannot be read" and blanks every row it would have produced. The Claude reader's own comment already said losing one row is the proportionate response to a file being written underneath it; now the code does that. Two tests hold a real FileShare.None handle over one file and assert the sibling row survives — both fail against the unfixed readers, and the Copilot one aborted the reader outright. The storybook's design references are now enforced rather than hoped for. The restructure moved the rules into .design and had each page name a chapter and anchor, which DesignGuideline renders beside the component. Rename a heading and the page silently draws a sentence saying the rule is missing, in a host nothing in CI opens. Every other convention in this library is a test; this one is too now, slugging headings the same way GitHub does because that is what resolves these anchors everywhere else they are written. README had drifted furthest, because four merges added projects to it and none updated it. Twelve of the forty-six were missing — all of Sessions, the Roadmap module and its Abstractions, Infrastructure.Sqlite and three other adapters, the storybook host, six test projects. Worse than incomplete: it still called Infrastructure.FileSystem the canonical local data, which ADR 0003 made untrue; still called Roadmap scaffolding; still described the .backlog folder join that went with the repository integration; still listed a Monitoring module that has no project; and still linked to domain/ and architecture/ folders that the restructure replaced. Every project is named now and every link resolves. Two stale comments from the merge seams: the surface switcher said "both takeover features" after Sessions made it three, and CLAUDE.md wrote src/harness where the solution file, the architecture tests and every other document write src/Harness. Left alone deliberately. The FileSystem adapter's settings and feature-flag tests sit in the desktop UI project because the collection fixture they serialize on is defined there, so the split is documented in the README row rather than papered over. And arc42's storage concept does not mention the roadmap plan's own JSON; that is a governed chapter and belongs to orch-arc42. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
A post-merge review of everything that landed on 19 and 20 August (#137 through #167), asking whether the pieces combined well and what they left behind.
Mostly they did. The combined state builds clean with no warnings, all tests pass, the solution matches what is on disk with nothing orphaned or dangling, no conflict markers survived any of the six
Merge origin/mainresolutions, every_metagraph is current, and all 55.designanchors the storybook names resolve. The two headline renames are coherent rather than half-finished: theBacklogEntry→TaskItemdrift in.domain/is deliberately recorded innaming.mdas deferred toorch-domain, and ADR 0003 explicitly addresses why Roadmap Planning stayed on JSON while tasks moved to SQLite. The three riskiest seams all merged correctly —WorkspaceSurfacegrew a fourth state with matching header segment, gating and Escape handling;DataTablearrived from the Sessions branch withRowKeyset toKind/Idso cross-agent id collisions cannot corrupt Blazor's keyed diff; Sessions' adapters follow theDashboard.UI/Adapters/precedent.So this PR is the residue, not a repair.
Changes
A locked session file no longer costs an agent its whole list. Both readers guarded the parse of a live-session file but not the open of it — and these files are written by the process they describe, so a sharing violation is an ordinary event. The
IOExceptionescaped toLocalAgentSessionSource.Collect, which reads it as "this agent's folder cannot be read" and blanks every row it would have produced. The Claude reader's own comment already said that losing one row is the proportionate response to a file being rewritten underneath it; now the code does that.The storybook's design references are enforced rather than hoped for. The restructure moved the rules into
.designand had each page name a chapter and anchor, whichDesignGuidelinerenders beside the component. Rename a heading and the page silently draws a sentence saying the rule is missing — in a host nothing in CI opens. Every other convention in this library is a test, so this one is now too, slugging headings the way GitHub does because that is what resolves these anchors everywhere else they are written.README.mdhad drifted furthest, because four merges added projects to it and none updated it. Twelve of the forty-six were missing: all of Sessions, the Roadmap module and its Abstractions,Infrastructure.Sqliteand three other adapters, the storybook host, six test projects. Worse than incomplete — it still calledInfrastructure.FileSystemthe canonical local data, which ADR 0003 made untrue; still called Roadmap "scaffolding only so far"; still described the.backlogfolder join that went with the repository integration; still listed aMonitoringmodule that has no project; and still linked to thedomain/andarchitecture/folders the restructure replaced.Two stale comments from the merge seams: the surface switcher said "both takeover features" after Sessions made it three, and
CLAUDE.mdwrotesrc/harnesswhere the solution file, the architecture tests and every other document writesrc/Harness.Testing
dotnet build Backlog.slnclean with 0 warnings anddotnet test Backlog.slnfully green — run both before the fixes and again after mergingorigin/main(which had moved five commits ahead ontoHome.razor, one of the files here).The two new reader tests hold a real
FileShare.Nonehandle over one session file and assert the sibling row survives. Both were checked against the unfixed readers and fail there — the Copilot one aborted the whole reader. The new architecture test was checked the same way, by renaming a.designheading and confirming it goes red. It also passes againstcolor-scheme.mdas #167 rewrote it, which is the guard doing its job on content it never saw.No Aspire instance was started for this review, so there was none to stop.
Related
origin/main(Give every repository one colour, and show it everywhere #167, repository colours) into this branch so the overlap onHome.razorwas resolved here rather than by a reviewer.Two things left for you to decide
tests/Backlog.Infrastructure.FileSystem.UnitTestsnow holds onlyJsonRoadmapPlanRepositoryTests. That adapter's own workspace-settings and feature-flag tests sit inBacklog.Desktop.UI.UnitTests, because they serialize on a collection fixture defined there. Not trivially movable, so the split is documented in the README row rather than papered over..arc42/08-crosscutting-concepts.mdnames SQLite for tasks and JSON for settings and flags, but not the roadmap plan's own_roadmap/plan.json. A governed chapter, so it belongs toorch-arc42rather than a drive-by edit here.Also latent but harmless: the working directory is
src/harnesswhile git's index recordssrc/Harness. Invisible on Windows undercore.ignorecase, and since the index carries the casing everything references, a case-sensitive checkout gets the right name — nothing is broken, but it is whyfindandgit ls-filesdisagree in this repo.Checklist
🤖 Generated with Claude Code