detached d5782db

fix(snapshot): address coderabbit findings

commit
d5782db54ab82fd1e62a12c8f46c00b06d6fd19c
author
brice · · 2026-07-06 08:34 UTC
parent
049957e
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +44 -7
Split
UnifiedSplit
M internal/usecase/daemon/identity.go +1 -1
@@ -17,7 +17,7 @@ func newStableID(prefix string) (string, error) {
1717 return prefix + "_" + strings.ToLower(enc), nil
1818 }
1919
20-func mustTestStableID(prefix string) string {
20+func fallbackStableID(prefix string) string {
2121 id, err := newStableID(prefix)
2222 if err != nil {
2323 return prefix + "_unknown"
@@ -17,7 +17,7 @@ func newStableID(prefix string) (string, error) {
17 return prefix + "_" + strings.ToLower(enc), nil17 return prefix + "_" + strings.ToLower(enc), nil
18 }18 }
19 19
20-func mustTestStableID(prefix string) string {20+func fallbackStableID(prefix string) string {
21 id, err := newStableID(prefix)21 id, err := newStableID(prefix)
22 if err != nil {22 if err != nil {
23 return prefix + "_unknown"23 return prefix + "_unknown"
M internal/usecase/daemon/pane.go +1 -1
@@ -35,7 +35,7 @@ type pane struct {
3535 }
3636
3737 func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane {
38- return newPaneWithStableID(id, mustTestStableID("p"), pty, sz)
38+ return newPaneWithStableID(id, fallbackStableID("p"), pty, sz)
3939 }
4040
4141 func newPaneWithStableID(id layout.PaneID, stableID string, pty ports.PTY, sz domain.Size) *pane {
@@ -35,7 +35,7 @@ type pane struct {
35 }35 }
36 36
37 func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane {37 func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane {
38- return newPaneWithStableID(id, mustTestStableID("p"), pty, sz)38+ return newPaneWithStableID(id, fallbackStableID("p"), pty, sz)
39 }39 }
40 40
41 func newPaneWithStableID(id layout.PaneID, stableID string, pty ports.PTY, sz domain.Size) *pane {41 func newPaneWithStableID(id layout.PaneID, stableID string, pty ports.PTY, sz domain.Size) *pane {
M internal/usecase/daemon/session.go +1 -1
@@ -291,7 +291,7 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error {
291291 }
292292
293293 func newTab(pty ports.PTY, sz domain.Size) *tab {
294- return newTabWithStableID(mustTestStableID("t"), mustTestStableID("p"), pty, sz)
294+ return newTabWithStableID(fallbackStableID("t"), fallbackStableID("p"), pty, sz)
295295 }
296296
297297 func newTabWithStableID(tabStableID, paneStableID string, pty ports.PTY, sz domain.Size) *tab {
@@ -291,7 +291,7 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error {
291 }291 }
292 292
293 func newTab(pty ports.PTY, sz domain.Size) *tab {293 func newTab(pty ports.PTY, sz domain.Size) *tab {
294- return newTabWithStableID(mustTestStableID("t"), mustTestStableID("p"), pty, sz)294+ return newTabWithStableID(fallbackStableID("t"), fallbackStableID("p"), pty, sz)
295 }295 }
296 296
297 func newTabWithStableID(tabStableID, paneStableID string, pty ports.PTY, sz domain.Size) *tab {297 func newTabWithStableID(tabStableID, paneStableID string, pty ports.PTY, sz domain.Size) *tab {
M internal/usecase/daemon/snapshot.go +4 -4
@@ -73,7 +73,6 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
7373 }
7474 cancel()
7575 }
76- restoreCommands := make(map[layout.PaneID]string)
7776 allowlist := d.restoreProcessAllowlistSnapshot()
7877 for _, tabSnap := range snap.Tabs {
7978 if err := ctx.Err(); err != nil {
@@ -110,8 +109,9 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
110109 tb.ctx, tb.cancel = context.WithCancel(sctx)
111110 opened = append(opened, tb)
112111 for _, paneSnap := range tabSnap.Panes {
112+ restoreCommand := ""
113113 if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore {
114- restoreCommands[paneSnap.ID] = decision.Command
114+ restoreCommand = decision.Command
115115 }
116116 if err := ctx.Err(); err != nil {
117117 closeOpened()
@@ -141,8 +141,8 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
141141 p.ctx, p.cancel = context.WithCancel(tb.ctx)
142142 p.rect = contentRect
143143 seedPaneRows(p, paneSnap.Scrollback, paneSnap.Visible)
144- if command := restoreCommands[paneSnap.ID]; command != "" {
145- if _, err := pty.Write([]byte(command + "\n")); err != nil {
144+ if restoreCommand != "" {
145+ if _, err := pty.Write([]byte(restoreCommand + "\n")); err != nil {
146146 d.log.Warn("writing snapshot restore command failed", "err", err, "session", snap.Name, "pane", paneSnap.ID)
147147 }
148148 }
@@ -73,7 +73,6 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
73 }73 }
74 cancel()74 cancel()
75 }75 }
76- restoreCommands := make(map[layout.PaneID]string)
77 allowlist := d.restoreProcessAllowlistSnapshot()76 allowlist := d.restoreProcessAllowlistSnapshot()
78 for _, tabSnap := range snap.Tabs {77 for _, tabSnap := range snap.Tabs {
79 if err := ctx.Err(); err != nil {78 if err := ctx.Err(); err != nil {
@@ -110,8 +109,9 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
110 tb.ctx, tb.cancel = context.WithCancel(sctx)109 tb.ctx, tb.cancel = context.WithCancel(sctx)
111 opened = append(opened, tb)110 opened = append(opened, tb)
112 for _, paneSnap := range tabSnap.Panes {111 for _, paneSnap := range tabSnap.Panes {
112+ restoreCommand := ""
113 if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore {113 if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore {
114- restoreCommands[paneSnap.ID] = decision.Command114+ restoreCommand = decision.Command
115 }115 }
116 if err := ctx.Err(); err != nil {116 if err := ctx.Err(); err != nil {
117 closeOpened()117 closeOpened()
@@ -141,8 +141,8 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
141 p.ctx, p.cancel = context.WithCancel(tb.ctx)141 p.ctx, p.cancel = context.WithCancel(tb.ctx)
142 p.rect = contentRect142 p.rect = contentRect
143 seedPaneRows(p, paneSnap.Scrollback, paneSnap.Visible)143 seedPaneRows(p, paneSnap.Scrollback, paneSnap.Visible)
144- if command := restoreCommands[paneSnap.ID]; command != "" {144+ if restoreCommand != "" {
145- if _, err := pty.Write([]byte(command + "\n")); err != nil {145+ if _, err := pty.Write([]byte(restoreCommand + "\n")); err != nil {
146 d.log.Warn("writing snapshot restore command failed", "err", err, "session", snap.Name, "pane", paneSnap.ID)146 d.log.Warn("writing snapshot restore command failed", "err", err, "session", snap.Name, "pane", paneSnap.ID)
147 }147 }
148 }148 }
M internal/usecase/daemon/snapshot_restore_test.go +37 -0
@@ -137,6 +137,43 @@ func TestRestoreSnapshotsSkipsDeniedProcessCommands(t *testing.T) {
137137 require.Empty(t, factory.opens[0].pty.writes)
138138 }
139139
140+func TestRestoreSnapshotsKeepsProcessCommandsScopedToPane(t *testing.T) {
141+ store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "scoped", Data: mustSnapshotBytes(t, snapcodec.Session{
142+ Name: "scoped",
143+ Tabs: []snapcodec.Tab{
144+ {
145+ Cols: 80,
146+ Rows: 24,
147+ Tree: layout.NewTree("pane-1"),
148+ Panes: []snapcodec.Pane{{
149+ ID: "pane-1",
150+ Cwd: "/one",
151+ Process: &snapcodec.Process{Argv: []string{"less", "README.md"}, Strategy: processStrategyGeneric},
152+ }},
153+ },
154+ {
155+ Cols: 80,
156+ Rows: 24,
157+ Tree: layout.NewTree("pane-1"),
158+ Panes: []snapcodec.Pane{{
159+ ID: "pane-1",
160+ Cwd: "/two",
161+ }},
162+ },
163+ },
164+ })}}}
165+ factory := &restorePTYFactory{}
166+ d := newTestDaemon(t, factory, stubClock{})
167+ WithSnapshotStore(store)(d)
168+ d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true, RestoreProcesses: []string{"less"}}})
169+
170+ d.restoreSnapshots(context.Background())
171+
172+ require.Len(t, factory.opens, 2)
173+ require.Equal(t, []string{"less README.md\n"}, factory.opens[0].pty.writes)
174+ require.Empty(t, factory.opens[1].pty.writes)
175+}
176+
140177 func TestRestoreSnapshotsWritesAgentIDCommandAndWriteFailureIsNonFatal(t *testing.T) {
141178 proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}}
142179 store := processRestoreTestStore(t, "agent", proc)
@@ -137,6 +137,43 @@ func TestRestoreSnapshotsSkipsDeniedProcessCommands(t *testing.T) {
137 require.Empty(t, factory.opens[0].pty.writes)137 require.Empty(t, factory.opens[0].pty.writes)
138 }138 }
139 139
140+func TestRestoreSnapshotsKeepsProcessCommandsScopedToPane(t *testing.T) {
141+ store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "scoped", Data: mustSnapshotBytes(t, snapcodec.Session{
142+ Name: "scoped",
143+ Tabs: []snapcodec.Tab{
144+ {
145+ Cols: 80,
146+ Rows: 24,
147+ Tree: layout.NewTree("pane-1"),
148+ Panes: []snapcodec.Pane{{
149+ ID: "pane-1",
150+ Cwd: "/one",
151+ Process: &snapcodec.Process{Argv: []string{"less", "README.md"}, Strategy: processStrategyGeneric},
152+ }},
153+ },
154+ {
155+ Cols: 80,
156+ Rows: 24,
157+ Tree: layout.NewTree("pane-1"),
158+ Panes: []snapcodec.Pane{{
159+ ID: "pane-1",
160+ Cwd: "/two",
161+ }},
162+ },
163+ },
164+ })}}}
165+ factory := &restorePTYFactory{}
166+ d := newTestDaemon(t, factory, stubClock{})
167+ WithSnapshotStore(store)(d)
168+ d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true, RestoreProcesses: []string{"less"}}})
169+
170+ d.restoreSnapshots(context.Background())
171+
172+ require.Len(t, factory.opens, 2)
173+ require.Equal(t, []string{"less README.md\n"}, factory.opens[0].pty.writes)
174+ require.Empty(t, factory.opens[1].pty.writes)
175+}
176+
140 func TestRestoreSnapshotsWritesAgentIDCommandAndWriteFailureIsNonFatal(t *testing.T) {177 func TestRestoreSnapshotsWritesAgentIDCommandAndWriteFailureIsNonFatal(t *testing.T) {
141 proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}}178 proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}}
142 store := processRestoreTestStore(t, "agent", proc)179 store := processRestoreTestStore(t, "agent", proc)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close