Reopen the workspace m6t was last closed in - #60
Merged
Conversation
Closes #58. A new service, internal/session, owns session.yaml beside the registry in the same configuration directory: the open project, and per project its editor tabs, its tree shape and its terminal tabs, plus the window-wide settings around them. Two bindings carry it — one read at launch, one write when the workspace settles. It is a second file rather than a section of projects.yaml because the two answer a broken file in opposite ways. A registry that will not parse is an error the user is shown, since starting from empty would look like an app that had forgotten every project they have. A session that will not parse is replaced by the defaults without a word, and left on disk rather than repaired, so a bad one can be read afterwards. One package cannot hold both rules, and the day someone applied the wrong one it is the registry that would be lost. Restoring is per project and lazy. The window-wide settings come back at launch; a project's tabs come back the first time it is the project on screen. Restoring every registered repository's at launch would read a file and start a shell for each of their tabs before the user had asked for any of it. The tree is the exception and is restored on every switch, because the tree hook resets on one — #59 is the ticket that makes the tree keep its state instead, and narrows this to the first activation like the rest. Everything restored is checked against a workspace that may have moved on while the app was closed. A file that will not read is a tab that does not come back, silently: `open` is a user asking for a specific file and owes them an error, but a session is a description that may have gone stale, and answering a deleted file with a tab full of error text would make every restart after a branch switch an exercise in closing tabs. A terminal whose directory is gone opens at the project root. A project the registry no longer holds loses its record. A sidebar too wide for the window it is being drawn in is fitted to it, which is also what a session saved on a docked display needs on a laptop. The split of who checks what is deliberate. internal/session applies the structural rules — the schema version, the caps, a selection that names a tab which exists, an index inside its strip — and answers the one filesystem question it can, whether a terminal's absolute cwd is still a directory. It does not clamp a font size or a pane width: those are the frontend's rules, held in theme.ts and panes.ts, and a second copy of them in Go would be two answers to one question. It cannot check an editor tab's path either, which is relative to a root only the registry knows, so those are validated where the answer is already being fetched. Normalization runs on the way out as well as in. The frontend is not a trusted producer: a bug that sent a duplicated tab list would otherwise be written and read back as truth. The write is a trailing edge rather than a restarting debounce, and that distinction is the whole feature. The workbench re-renders continuously on its own, so a timer cancelled and re-armed on each render that found a change is pushed past its deadline forever and the file never appears at all — which is exactly what the first version of this did. The first change arms the timer, nothing re-arms it, and what lands is the newest workspace rather than the one that tripped it. There is no shutdown flush to fall back on: Wails ends the process from the Go side and a webview is not guaranteed to run a handler on the way out, so a save wired to one would work in development and quietly not on a user's machine. A failed write is swallowed, deliberately, for the reason the tree's manifest classification already swallows its own: there is no user action behind "your pane layout could not be saved". The binding reports the failure so the contract is honest; the caller is what decides to ignore it. The saved shape is flat — the window settings on State, the tree's on Project — rather than nested Workspace and Tree structs. revive caps a package's exported structs at five, and for a schema that is the right shape of pressure: five names that each mean something on the wire beat seven where two exist only to indent the file. Ratchets: internal/app 700 -> 760 LOC, its coordinator 6 -> 7 fields and 23 -> 25 methods, and a pin for the new package at 550 LOC and 9 exported. Two methods is also the ceiling on what this feature can cost that surface — the whole state crosses in one call, so the next control the UI gains is a field here and a line in the frontend's snapshot rather than another binding. Verified against the built app, not only in tests: with the session file deleted it is written; with one naming the other project and a terminal, that project's git is what runs at startup and a shell appears in the recorded directory.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
+ Coverage 93.95% 94.04% +0.08%
==========================================
Files 33 36 +3
Lines 1621 1763 +142
==========================================
+ Hits 1523 1658 +135
- Misses 62 66 +4
- Partials 36 39 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
codecov/patch failed PR #60 at 84.50% against the 85% floor. Part of that is the known divergence in #29 — codecov counts a partially executed line against you and `go tool cover` has no such category, so the local gate read 87.8% on the same diff — but it was not only that. Eleven lines were genuinely untested, all of them in the write: internal/session/store.go was at 72.7%. So this is real coverage rather than an argument about arithmetic. The package goes to 96.7%, the patch to 96.1%. Two of the paths cannot be reached through Save at all, which is why these tests are in the package rather than beside it: a write and a sync only fail on a handle the store would never hand itself. A closed file produces the first and a pipe the second — a pipe takes the bytes and refuses to be made durable, which is the one honest way to fail a durability step without a filesystem that lies. What both assert is not that an error came back but that it names the step and the file, since that string is the only report a user gets when their workspace stops persisting. The rename failure is reachable and worth having: a directory sitting where the session file goes is a state a config directory can be left in by hand, and the test pins the part that matters after it — that the scratch file is removed, so one failed save does not leave a file every later save trips over. The rest close the gaps the caps and the empty lists left: a project with no terminals and no expanded directories comes back with both absent rather than as empty lists the encoder would write out as noise, and the two caps that no other test exercises are held to their numbers.
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
1 participant
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.
Closes #58
What this does
Reopening m6t resumes the workspace instead of rebuilding it by hand: the project that was open, and per project its editor tabs, the tree's shape and its terminal tabs, plus the font size, both pane splits and the changed-files filter around them.
A new service,
internal/session, ownssession.yamlin the same configuration directory as the registry. It is a second file rather than a section ofprojects.yamlbecause the two answer a broken file in opposite ways: a registry that will not parse is an error the user is shown, since starting from empty would look like an app that had forgotten every project they have, while a session that will not parse is replaced by the defaults without a word — and left on disk rather than repaired, so a bad one can still be read afterwards. One package cannot hold both rules.Two bindings,
SessionStateandSaveSession. The whole state crosses in one call rather than a setter per field, which is what keeps this a two-method surface as the workspace grows: the next control the UI gains is a field ininternal/sessionand a line in the frontend's snapshot, not another binding.Restoring is per project and lazy. The window-wide settings come back at launch; a project's tabs come back the first time it is the project on screen, because restoring every registered repository's would read a file and start a shell for each of their tabs before the user had asked for any of it. The tree is the exception and is restored on every switch, because the tree hook resets on one — #59 is the ticket that makes a project tab keep its state instead, and narrows this to the first activation like the rest.
Everything restored is checked against a workspace that may have moved on. A file that will not read is a tab that does not come back, silently:
openis a user asking for a specific file and owes them an error, but a session is a description that may have gone stale, and answering a deleted file with a tab full of error text would make every restart after a branch switch an exercise in closing tabs. A terminal whose directory is gone opens at the project root. A project the registry no longer holds loses its record. A sidebar too wide for the window it is drawn in is fitted to it, which is also what a session saved on a docked display needs on a laptop.The split of who validates what is deliberate.
internal/sessionapplies the structural rules — schema version, caps, a selection that names a tab which exists, an index inside its strip — and answers the one filesystem question it can, whether a terminal's absolute cwd is still a directory. It does not clamp a font size or a pane width: those are the frontend's rules, held intheme.tsandpanes.ts, and a second copy in Go would be two answers to one question. It cannot check an editor tab's path either, which is relative to a root only the registry knows, so those are validated where the answer is already being fetched. Normalization runs on the way out as well as in, because the frontend is not a trusted producer either.The write is a trailing edge rather than a restarting debounce, and that distinction is the whole feature working or not working. The workbench re-renders continuously on its own, so a timer cancelled and re-armed on every render that found a change is pushed past its deadline forever and the file never appears — which is exactly what the first version of this did, and what running the built app caught. There is no shutdown flush to fall back on: Wails ends the process from the Go side and a webview is not guaranteed to run a handler on the way out, so a save wired to one would work in development and quietly not on a user's machine.
How it was verified
make verifygreen on this branch, rebased onto62c616c: patch coverage above the 85% floor, total coverage 95%, no lint suppressions, bindings regenerated, dead-code and structural gates clean.Tests:
internal/sessioncovers the round trip, the five ways a file can be unusable (each answering with defaults), the unusable file being left on disk, owner-only permissions, the atomic write leaving no scratch file, and every normalization rule against a hand-written file rather than through a mock. The frontend covers the pure restore and record functions,restoreon each of the three hooks, anduseSessiondriving the real hooks end to end — including that a project the app was only passing through at startup is not hydrated, that a strip is never recorded before its tabs are put back, that each project's own tree survives a switch and back, and that a write still lands while the workbench keeps re-rendering.Against the built app on macOS, not only in tests. With the session file deleted, it is written. With one hand-written naming the other project and a terminal: 31 git invocations against the restored project's repository versus 3 against the registry's first pick, a
/bin/zshspawned in the recorded directory, and the font size and both pane sizes restored and recorded back unchanged.Acceptance criteria from #58 are met, with three deviations called out in the issue thread and worth repeating here. Appearance is not persisted: it follows the OS live and has no in-app override, so storing it would fight
watchAppearance. There is no shutdown flush, for the reason above. The changed-files filter is window-wide rather than per project, because the tree deliberately carries it across a switch and a per-project copy would let a switch turn it off.Ratchets moved, each with the argument in the diff:
internal/app700 -> 760 LOC, its coordinator 6 -> 7 fields and 23 -> 25 methods, a pin for the new package at 550 LOC and 9 exported, and the import graph pinned for the one new edge. The saved shape is flat — window settings onState, the tree's onProject— rather than nestedWorkspaceandTreestructs, because revive caps a package's exported structs at five and for a schema that is the right pressure: five names that each mean something on the wire beat seven where two exist only to indent the file.One finding that is not this PR's to fix: an idle window with one project open runs
git status --porcelain=v2about four times a second, measured identically on62c616cand on this branch. It is recorded in #59.Checklist