Merge pull request 'fix: post-v0.7.0 review findings' (#18) from fix/post-v0.7.0-review into main
49 files · +415 -493
M AGENTS.md +1 -1
| @@ -31,7 +31,7 @@ go test ./internal/adapters/ipc ./internal/usecase/daemon -run '^$' -bench=. -be | |||
| 31 | 31 | The exhaustive production matrix and separate test-import policy are enforced by `boundary_test.go`; package ownership is documented in `docs/architecture.md`. | |
| 32 | 32 | ||
| 33 | 33 | - `pkg/` is reusable and never imports `internal/`. | |
| 34 | - | - Production use cases import only `internal/ports`, semantic `internal/protocol` packages, `internal/domain`, `internal/snapshotcodec`, and approved sibling use cases. | |
| 34 | + | - Production use cases import only `internal/ports`, semantic `internal/protocol` packages, `internal/domain`, `internal/snapshotcodec` (daemon only), and approved sibling use cases. | |
| 35 | 35 | - Production use cases never import `internal/protocol/wire`, concrete adapters, `internal/app`, `internal/persist`, or `internal/platform`. | |
| 36 | 36 | - `internal/ports` owns application seams, not codecs, raw frames, environment policy, or worker implementations. | |
| 37 | 37 | ||
| @@ -31,7 +31,7 @@ go test ./internal/adapters/ipc ./internal/usecase/daemon -run '^$' -bench=. -be | |||||
| 31 | The exhaustive production matrix and separate test-import policy are enforced by `boundary_test.go`; package ownership is documented in `docs/architecture.md`. | 31 | The exhaustive production matrix and separate test-import policy are enforced by `boundary_test.go`; package ownership is documented in `docs/architecture.md`. | ||
| 32 | 32 | ||||
| 33 | - `pkg/` is reusable and never imports `internal/`. | 33 | - `pkg/` is reusable and never imports `internal/`. | ||
| 34 | - | - Production use cases import only `internal/ports`, semantic `internal/protocol` packages, `internal/domain`, `internal/snapshotcodec`, and approved sibling use cases. | 34 | + | - Production use cases import only `internal/ports`, semantic `internal/protocol` packages, `internal/domain`, `internal/snapshotcodec` (daemon only), and approved sibling use cases. |
| 35 | - Production use cases never import `internal/protocol/wire`, concrete adapters, `internal/app`, `internal/persist`, or `internal/platform`. | 35 | - Production use cases never import `internal/protocol/wire`, concrete adapters, `internal/app`, `internal/persist`, or `internal/platform`. | ||
| 36 | - `internal/ports` owns application seams, not codecs, raw frames, environment policy, or worker implementations. | 36 | - `internal/ports` owns application seams, not codecs, raw frames, environment policy, or worker implementations. | ||
| 37 | 37 | ||||
M boundary_test.go +13 -5
| @@ -96,6 +96,7 @@ func TestImportBoundaryNegativeFixtures(t *testing.T) { | |||
| 96 | 96 | {"snapshot adapter accepts codec", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/snapshotcodec", false, true}, | |
| 97 | 97 | {"daemon accepts snapshot codec", modulePath + "/internal/usecase/daemon", modulePath + "/internal/snapshotcodec", false, true}, | |
| 98 | 98 | {"other adapter rejects snapshot codec", modulePath + "/internal/adapters/ipc", modulePath + "/internal/snapshotcodec", false, false}, | |
| 99 | + | {"client rejects snapshot codec", modulePath + "/internal/usecase/client", modulePath + "/internal/snapshotcodec", false, false}, | |
| 99 | 100 | {"snapshot codec rejects usecase", modulePath + "/internal/snapshotcodec", modulePath + "/internal/usecase/daemon", false, false}, | |
| 100 | 101 | {"snapshot codec rejects adapter", modulePath + "/internal/snapshotcodec", modulePath + "/internal/adapters/snapshot", false, false}, | |
| 101 | 102 | {"snapshot adapter rejects usecase", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/usecase/daemon", false, false}, | |
| @@ -216,8 +217,10 @@ func dependencyAllowed(source, target string, testFile bool) (bool, error) { | |||
| 216 | 217 | ||
| 217 | 218 | // packageImportDenied encodes ADR 001 ownership at package granularity for | |
| 218 | 219 | // production files. Layer rules alone permit any usecase-to-usecase import; | |
| 219 | - | // other, and keep the broker free of every sibling use case. Test files stay | |
| 220 | - | // exempt so composition tests can wire adapters and orchestrators together. | |
| 220 | + | // this keeps client and daemon from importing each other, keeps the broker | |
| 221 | + | // free of every sibling use case, and limits the snapshot codec to the daemon | |
| 222 | + | // and the snapshot adapter. Test files stay exempt so composition tests can | |
| 223 | + | // wire adapters and orchestrators together. | |
| 221 | 224 | func packageImportDenied(source, target string) bool { | |
| 222 | 225 | brokerPkg := modulePath + "/internal/usecase/broker" | |
| 223 | 226 | clientPkg := modulePath + "/internal/usecase/client" | |
| @@ -227,9 +230,14 @@ func packageImportDenied(source, target string) bool { | |||
| 227 | 230 | return path == root || strings.HasPrefix(path, root+"/") | |
| 228 | 231 | } | |
| 229 | 232 | // The durable snapshot format is shared by the daemon and its one | |
| 230 | - | // repository adapter; no other adapter may depend on it. | |
| 231 | - | if target == modulePath+"/internal/snapshotcodec" && strings.HasPrefix(source, modulePath+"/internal/adapters/") { | |
| 232 | - | return source != modulePath+"/internal/adapters/snapshot" | |
| 233 | + | // repository adapter; no other adapter or use case may depend on it. | |
| 234 | + | if target == modulePath+"/internal/snapshotcodec" { | |
| 235 | + | switch { | |
| 236 | + | case strings.HasPrefix(source, modulePath+"/internal/adapters/"): | |
| 237 | + | return source != modulePath+"/internal/adapters/snapshot" | |
| 238 | + | case strings.HasPrefix(source, usecasePrefix): | |
| 239 | + | return !under(source, daemonPkg) | |
| 240 | + | } | |
| 233 | 241 | } | |
| 234 | 242 | switch { | |
| 235 | 243 | case under(source, brokerPkg): | |
| @@ -96,6 +96,7 @@ func TestImportBoundaryNegativeFixtures(t *testing.T) { | |||||
| 96 | {"snapshot adapter accepts codec", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/snapshotcodec", false, true}, | 96 | {"snapshot adapter accepts codec", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/snapshotcodec", false, true}, | ||
| 97 | {"daemon accepts snapshot codec", modulePath + "/internal/usecase/daemon", modulePath + "/internal/snapshotcodec", false, true}, | 97 | {"daemon accepts snapshot codec", modulePath + "/internal/usecase/daemon", modulePath + "/internal/snapshotcodec", false, true}, | ||
| 98 | {"other adapter rejects snapshot codec", modulePath + "/internal/adapters/ipc", modulePath + "/internal/snapshotcodec", false, false}, | 98 | {"other adapter rejects snapshot codec", modulePath + "/internal/adapters/ipc", modulePath + "/internal/snapshotcodec", false, false}, | ||
| 99 | + | {"client rejects snapshot codec", modulePath + "/internal/usecase/client", modulePath + "/internal/snapshotcodec", false, false}, | |||
| 99 | {"snapshot codec rejects usecase", modulePath + "/internal/snapshotcodec", modulePath + "/internal/usecase/daemon", false, false}, | 100 | {"snapshot codec rejects usecase", modulePath + "/internal/snapshotcodec", modulePath + "/internal/usecase/daemon", false, false}, | ||
| 100 | {"snapshot codec rejects adapter", modulePath + "/internal/snapshotcodec", modulePath + "/internal/adapters/snapshot", false, false}, | 101 | {"snapshot codec rejects adapter", modulePath + "/internal/snapshotcodec", modulePath + "/internal/adapters/snapshot", false, false}, | ||
| 101 | {"snapshot adapter rejects usecase", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/usecase/daemon", false, false}, | 102 | {"snapshot adapter rejects usecase", modulePath + "/internal/adapters/snapshot", modulePath + "/internal/usecase/daemon", false, false}, | ||
| @@ -216,8 +217,10 @@ func dependencyAllowed(source, target string, testFile bool) (bool, error) { | |||||
| 216 | 217 | ||||
| 217 | // packageImportDenied encodes ADR 001 ownership at package granularity for | 218 | // packageImportDenied encodes ADR 001 ownership at package granularity for | ||
| 218 | // production files. Layer rules alone permit any usecase-to-usecase import; | 219 | // production files. Layer rules alone permit any usecase-to-usecase import; | ||
| 219 | - | // other, and keep the broker free of every sibling use case. Test files stay | 220 | + | // this keeps client and daemon from importing each other, keeps the broker |
| 220 | - | // exempt so composition tests can wire adapters and orchestrators together. | 221 | + | // free of every sibling use case, and limits the snapshot codec to the daemon |
| 222 | + | // and the snapshot adapter. Test files stay exempt so composition tests can | |||
| 223 | + | // wire adapters and orchestrators together. | |||
| 221 | func packageImportDenied(source, target string) bool { | 224 | func packageImportDenied(source, target string) bool { | ||
| 222 | brokerPkg := modulePath + "/internal/usecase/broker" | 225 | brokerPkg := modulePath + "/internal/usecase/broker" | ||
| 223 | clientPkg := modulePath + "/internal/usecase/client" | 226 | clientPkg := modulePath + "/internal/usecase/client" | ||
| @@ -227,9 +230,14 @@ func packageImportDenied(source, target string) bool { | |||||
| 227 | return path == root || strings.HasPrefix(path, root+"/") | 230 | return path == root || strings.HasPrefix(path, root+"/") | ||
| 228 | } | 231 | } | ||
| 229 | // The durable snapshot format is shared by the daemon and its one | 232 | // The durable snapshot format is shared by the daemon and its one | ||
| 230 | - | // repository adapter; no other adapter may depend on it. | 233 | + | // repository adapter; no other adapter or use case may depend on it. |
| 231 | - | if target == modulePath+"/internal/snapshotcodec" && strings.HasPrefix(source, modulePath+"/internal/adapters/") { | 234 | + | if target == modulePath+"/internal/snapshotcodec" { |
| 232 | - | return source != modulePath+"/internal/adapters/snapshot" | 235 | + | switch { |
| 236 | + | case strings.HasPrefix(source, modulePath+"/internal/adapters/"): | |||
| 237 | + | return source != modulePath+"/internal/adapters/snapshot" | |||
| 238 | + | case strings.HasPrefix(source, usecasePrefix): | |||
| 239 | + | return !under(source, daemonPkg) | |||
| 240 | + | } | |||
| 233 | } | 241 | } | ||
| 234 | switch { | 242 | switch { | ||
| 235 | case under(source, brokerPkg): | 243 | case under(source, brokerPkg): | ||
M docs/architecture.md +1 -1
| @@ -60,7 +60,7 @@ main → app → usecase → ports | |||
| 60 | 60 | carriage adapters | |
| 61 | 61 | ``` | |
| 62 | 62 | ||
| 63 | - | `internal/app` may import every layer to compose the process. Adapters depend inward on ports, protocol, and domain. `internal/snapshotcodec` owns the durable snapshot format (session codec, manifests, objects, preflight); it depends only on domain and ports, and both the daemon use case and the snapshot adapter consume it. Production dependency rules and the separate test-import policy are executable in `boundary_test.go`. | |
| 63 | + | `internal/app` may import every layer to compose the process. Adapters depend inward on ports, protocol, and domain. `internal/snapshotcodec` owns the durable snapshot format (session codec, manifests, objects, preflight); it depends only on domain and ports, and only the daemon use case and the snapshot adapter consume it. Production dependency rules and the separate test-import policy are executable in `boundary_test.go`. | |
| 64 | 64 | ||
| 65 | 65 | ## UI observation and control | |
| 66 | 66 | ||
| @@ -60,7 +60,7 @@ main → app → usecase → ports | |||||
| 60 | carriage adapters | 60 | carriage adapters | ||
| 61 | ``` | 61 | ``` | ||
| 62 | 62 | ||||
| 63 | - | `internal/app` may import every layer to compose the process. Adapters depend inward on ports, protocol, and domain. `internal/snapshotcodec` owns the durable snapshot format (session codec, manifests, objects, preflight); it depends only on domain and ports, and both the daemon use case and the snapshot adapter consume it. Production dependency rules and the separate test-import policy are executable in `boundary_test.go`. | 63 | + | `internal/app` may import every layer to compose the process. Adapters depend inward on ports, protocol, and domain. `internal/snapshotcodec` owns the durable snapshot format (session codec, manifests, objects, preflight); it depends only on domain and ports, and only the daemon use case and the snapshot adapter consume it. Production dependency rules and the separate test-import policy are executable in `boundary_test.go`. |
| 64 | 64 | ||||
| 65 | ## UI observation and control | 65 | ## UI observation and control | ||
| 66 | 66 | ||||
M internal/adapters/brokeripc/server.go +4 -0
| @@ -364,6 +364,10 @@ func (l *listener) startSession(transport wire.BoundedTransport, release func()) | |||
| 364 | 364 | session.shutdown() | |
| 365 | 365 | return nil, ErrListenerClosed | |
| 366 | 366 | } | |
| 367 | + | // The session owns the carriage from here: drop it from pending in the | |
| 368 | + | // same critical section, so a Register racing admit's own untrack never | |
| 369 | + | // sees a stale pending carriage and refuses to retire. | |
| 370 | + | delete(l.pending, transport) | |
| 367 | 371 | l.sessions[session] = struct{}{} | |
| 368 | 372 | l.mu.Unlock() | |
| 369 | 373 | go session.run() | |
| @@ -364,6 +364,10 @@ func (l *listener) startSession(transport wire.BoundedTransport, release func()) | |||||
| 364 | session.shutdown() | 364 | session.shutdown() | ||
| 365 | return nil, ErrListenerClosed | 365 | return nil, ErrListenerClosed | ||
| 366 | } | 366 | } | ||
| 367 | + | // The session owns the carriage from here: drop it from pending in the | |||
| 368 | + | // same critical section, so a Register racing admit's own untrack never | |||
| 369 | + | // sees a stale pending carriage and refuses to retire. | |||
| 370 | + | delete(l.pending, transport) | |||
| 367 | l.sessions[session] = struct{}{} | 371 | l.sessions[session] = struct{}{} | ||
| 368 | l.mu.Unlock() | 372 | l.mu.Unlock() | ||
| 369 | go session.run() | 373 | go session.run() | ||
M internal/app/broker_composition_test.go +2 -2
| @@ -128,8 +128,8 @@ func TestProductionBrokerRemoteMuxTransports(t *testing.T) { | |||
| 128 | 128 | for _, tc := range []struct { | |
| 129 | 129 | name, helper string | |
| 130 | 130 | }{ | |
| 131 | - | {hostTransportSSH, brokerMuxStdioCommand}, | |
| 132 | - | {hostTransportQUIC, brokerMuxQUICBootstrapCommand}, | |
| 131 | + | {hostTransportSSH, ports.BrokerMuxStdioCommand}, | |
| 132 | + | {hostTransportQUIC, ports.BrokerMuxQUICBootstrapCommand}, | |
| 133 | 133 | } { | |
| 134 | 134 | t.Run(tc.name, func(t *testing.T) { | |
| 135 | 135 | binDir := shortTempDir(t, "vb") | |
| @@ -128,8 +128,8 @@ func TestProductionBrokerRemoteMuxTransports(t *testing.T) { | |||||
| 128 | for _, tc := range []struct { | 128 | for _, tc := range []struct { | ||
| 129 | name, helper string | 129 | name, helper string | ||
| 130 | }{ | 130 | }{ | ||
| 131 | - | {hostTransportSSH, brokerMuxStdioCommand}, | 131 | + | {hostTransportSSH, ports.BrokerMuxStdioCommand}, |
| 132 | - | {hostTransportQUIC, brokerMuxQUICBootstrapCommand}, | 132 | + | {hostTransportQUIC, ports.BrokerMuxQUICBootstrapCommand}, |
| 133 | } { | 133 | } { | ||
| 134 | t.Run(tc.name, func(t *testing.T) { | 134 | t.Run(tc.name, func(t *testing.T) { | ||
| 135 | binDir := shortTempDir(t, "vb") | 135 | binDir := shortTempDir(t, "vb") | ||
M internal/app/broker_offline_local_test.go +1 -1
| @@ -770,7 +770,7 @@ func TestBrokerLocalProbeRefusesNonLocalRoute(t *testing.T) { | |||
| 770 | 770 | sshRoute := loadTestRoute(t, map[string]any{ | |
| 771 | 771 | "kind": "ssh-stdio", | |
| 772 | 772 | "target": "user@host:2222", | |
| 773 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--offline-root", "/srv/remote"}, | |
| 773 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--offline-root", "/srv/remote"}, | |
| 774 | 774 | }) | |
| 775 | 775 | require.Equal(t, brokerconfig.RouteSSHStdio, sshRoute.Kind()) | |
| 776 | 776 | require.False(t, sshRoute.IsLocal(), "an ssh helper route is never dialed directly") | |
| @@ -770,7 +770,7 @@ func TestBrokerLocalProbeRefusesNonLocalRoute(t *testing.T) { | |||||
| 770 | sshRoute := loadTestRoute(t, map[string]any{ | 770 | sshRoute := loadTestRoute(t, map[string]any{ | ||
| 771 | "kind": "ssh-stdio", | 771 | "kind": "ssh-stdio", | ||
| 772 | "target": "user@host:2222", | 772 | "target": "user@host:2222", | ||
| 773 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--offline-root", "/srv/remote"}, | 773 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--offline-root", "/srv/remote"}, |
| 774 | }) | 774 | }) | ||
| 775 | require.Equal(t, brokerconfig.RouteSSHStdio, sshRoute.Kind()) | 775 | require.Equal(t, brokerconfig.RouteSSHStdio, sshRoute.Kind()) | ||
| 776 | require.False(t, sshRoute.IsLocal(), "an ssh helper route is never dialed directly") | 776 | require.False(t, sshRoute.IsLocal(), "an ssh helper route is never dialed directly") | ||
M internal/app/broker_offline_mux.go +7 -15
| @@ -63,14 +63,10 @@ import ( | |||
| 63 | 63 | // authentication, and the absence of a remote PTY are preserved: the helpers | |
| 64 | 64 | // only add explicit trust inputs that narrow verification, never disable it. | |
| 65 | 65 | ||
| 66 | - | const ( | |
| 67 | - | // brokerMuxStdioCommand is the hidden remote SSH stdio mux bridge. | |
| 68 | - | brokerMuxStdioCommand = ports.BrokerMuxStdioCommand | |
| 69 | - | // brokerMuxQUICBootstrapCommand is the hidden remote QUIC mux bootstrap. | |
| 70 | - | brokerMuxQUICBootstrapCommand = ports.BrokerMuxQUICBootstrapCommand | |
| 71 | - | // brokerMuxQUICProxyCommand is the hidden detached remote QUIC mux proxy. | |
| 72 | - | brokerMuxQUICProxyCommand = "_broker-mux-quic-proxy" | |
| 73 | - | ) | |
| 66 | + | // brokerMuxQUICProxyCommand is the hidden detached remote QUIC mux proxy. The | |
| 67 | + | // stdio and bootstrap helpers are named by ports.BrokerMuxStdioCommand and | |
| 68 | + | // ports.BrokerMuxQUICBootstrapCommand because durable routes persist them. | |
| 69 | + | const brokerMuxQUICProxyCommand = "_broker-mux-quic-proxy" | |
| 74 | 70 | ||
| 75 | 71 | // brokerMuxSetupTimeout bounds each side's bootstrap readiness wait. On the | |
| 76 | 72 | // broker side it caps the wait for the local ssh bootstrap child's readiness | |
| @@ -103,10 +99,6 @@ const brokerMuxWaitDelay = 2 * time.Second | |||
| 103 | 99 | // brokerDaemonStartArgvFlag's exact spelling, so the two sides can never drift. | |
| 104 | 100 | const brokerDaemonStartArg = "--daemon-start" | |
| 105 | 101 | ||
| 106 | - | // brokerScopeArg is the scope flag carried by every durable remote route argv | |
| 107 | - | // (see ports.BrokerRouteForTransport). Stored routes keep it verbatim. | |
| 108 | - | const brokerScopeArg = ports.BrokerRouteScopeArg | |
| 109 | - | ||
| 110 | 102 | // brokerMuxOptions is the parsed hidden mux helper invocation. | |
| 111 | 103 | type brokerMuxOptions struct { | |
| 112 | 104 | // startMode is the daemon-start authorization the broker propagated. An | |
| @@ -125,11 +117,11 @@ func parseBrokerMuxArgs(name string, kind cmdKind, args []string) (command, erro | |||
| 125 | 117 | scopeSet, startSet := false, false | |
| 126 | 118 | for index := 0; index < len(args); { | |
| 127 | 119 | switch args[index] { | |
| 128 | - | case brokerScopeArg: | |
| 120 | + | case ports.BrokerRouteScopeArg: | |
| 129 | 121 | // The scope flag is part of every durable remote route argv, so it | |
| 130 | 122 | // stays accepted; the helper always serves the per-user scope. | |
| 131 | 123 | if scopeSet { | |
| 132 | - | return command{}, usagef("`%s` received duplicate %s", name, brokerScopeArg) | |
| 124 | + | return command{}, usagef("`%s` received duplicate %s", name, ports.BrokerRouteScopeArg) | |
| 133 | 125 | } | |
| 134 | 126 | scopeSet = true | |
| 135 | 127 | index++ | |
| @@ -495,7 +487,7 @@ func runBrokerMuxQUICBootstrapCommand(ctx context.Context, options brokerMuxOpti | |||
| 495 | 487 | } | |
| 496 | 488 | defer func() { _ = devNull.Close() }() | |
| 497 | 489 | ||
| 498 | - | args := []string{brokerMuxQUICProxyCommand, brokerScopeArg} | |
| 490 | + | args := []string{brokerMuxQUICProxyCommand, ports.BrokerRouteScopeArg} | |
| 499 | 491 | flag, err := brokerDaemonStartArgvFlag(options.startMode) | |
| 500 | 492 | if err != nil { | |
| 501 | 493 | _ = writer.Close() | |
| @@ -63,14 +63,10 @@ import ( | |||||
| 63 | // authentication, and the absence of a remote PTY are preserved: the helpers | 63 | // authentication, and the absence of a remote PTY are preserved: the helpers | ||
| 64 | // only add explicit trust inputs that narrow verification, never disable it. | 64 | // only add explicit trust inputs that narrow verification, never disable it. | ||
| 65 | 65 | ||||
| 66 | - | const ( | 66 | + | // brokerMuxQUICProxyCommand is the hidden detached remote QUIC mux proxy. The |
| 67 | - | // brokerMuxStdioCommand is the hidden remote SSH stdio mux bridge. | 67 | + | // stdio and bootstrap helpers are named by ports.BrokerMuxStdioCommand and |
| 68 | - | brokerMuxStdioCommand = ports.BrokerMuxStdioCommand | 68 | + | // ports.BrokerMuxQUICBootstrapCommand because durable routes persist them. |
| 69 | - | // brokerMuxQUICBootstrapCommand is the hidden remote QUIC mux bootstrap. | 69 | + | const brokerMuxQUICProxyCommand = "_broker-mux-quic-proxy" |
| 70 | - | brokerMuxQUICBootstrapCommand = ports.BrokerMuxQUICBootstrapCommand | |||
| 71 | - | // brokerMuxQUICProxyCommand is the hidden detached remote QUIC mux proxy. | |||
| 72 | - | brokerMuxQUICProxyCommand = "_broker-mux-quic-proxy" | |||
| 73 | - | ) | |||
| 74 | 70 | ||||
| 75 | // brokerMuxSetupTimeout bounds each side's bootstrap readiness wait. On the | 71 | // brokerMuxSetupTimeout bounds each side's bootstrap readiness wait. On the | ||
| 76 | // broker side it caps the wait for the local ssh bootstrap child's readiness | 72 | // broker side it caps the wait for the local ssh bootstrap child's readiness | ||
| @@ -103,10 +99,6 @@ const brokerMuxWaitDelay = 2 * time.Second | |||||
| 103 | // brokerDaemonStartArgvFlag's exact spelling, so the two sides can never drift. | 99 | // brokerDaemonStartArgvFlag's exact spelling, so the two sides can never drift. | ||
| 104 | const brokerDaemonStartArg = "--daemon-start" | 100 | const brokerDaemonStartArg = "--daemon-start" | ||
| 105 | 101 | ||||
| 106 | - | // brokerScopeArg is the scope flag carried by every durable remote route argv | |||
| 107 | - | // (see ports.BrokerRouteForTransport). Stored routes keep it verbatim. | |||
| 108 | - | const brokerScopeArg = ports.BrokerRouteScopeArg | |||
| 109 | - | ||||
| 110 | // brokerMuxOptions is the parsed hidden mux helper invocation. | 102 | // brokerMuxOptions is the parsed hidden mux helper invocation. | ||
| 111 | type brokerMuxOptions struct { | 103 | type brokerMuxOptions struct { | ||
| 112 | // startMode is the daemon-start authorization the broker propagated. An | 104 | // startMode is the daemon-start authorization the broker propagated. An | ||
| @@ -125,11 +117,11 @@ func parseBrokerMuxArgs(name string, kind cmdKind, args []string) (command, erro | |||||
| 125 | scopeSet, startSet := false, false | 117 | scopeSet, startSet := false, false | ||
| 126 | for index := 0; index < len(args); { | 118 | for index := 0; index < len(args); { | ||
| 127 | switch args[index] { | 119 | switch args[index] { | ||
| 128 | - | case brokerScopeArg: | 120 | + | case ports.BrokerRouteScopeArg: |
| 129 | // The scope flag is part of every durable remote route argv, so it | 121 | // The scope flag is part of every durable remote route argv, so it | ||
| 130 | // stays accepted; the helper always serves the per-user scope. | 122 | // stays accepted; the helper always serves the per-user scope. | ||
| 131 | if scopeSet { | 123 | if scopeSet { | ||
| 132 | - | return command{}, usagef("`%s` received duplicate %s", name, brokerScopeArg) | 124 | + | return command{}, usagef("`%s` received duplicate %s", name, ports.BrokerRouteScopeArg) |
| 133 | } | 125 | } | ||
| 134 | scopeSet = true | 126 | scopeSet = true | ||
| 135 | index++ | 127 | index++ | ||
| @@ -495,7 +487,7 @@ func runBrokerMuxQUICBootstrapCommand(ctx context.Context, options brokerMuxOpti | |||||
| 495 | } | 487 | } | ||
| 496 | defer func() { _ = devNull.Close() }() | 488 | defer func() { _ = devNull.Close() }() | ||
| 497 | 489 | ||||
| 498 | - | args := []string{brokerMuxQUICProxyCommand, brokerScopeArg} | 490 | + | args := []string{brokerMuxQUICProxyCommand, ports.BrokerRouteScopeArg} |
| 499 | flag, err := brokerDaemonStartArgvFlag(options.startMode) | 491 | flag, err := brokerDaemonStartArgvFlag(options.startMode) | ||
| 500 | if err != nil { | 492 | if err != nil { | ||
| 501 | _ = writer.Close() | 493 | _ = writer.Close() | ||
M internal/app/broker_offline_mux_test.go +19 -19
| @@ -156,7 +156,7 @@ func remoteMuxFixture(t *testing.T, policy ports.BrokerPolicy) *brokerMuxFixture | |||
| 156 | 156 | ||
| 157 | 157 | // cleanup kills every recorded mux helper process. | |
| 158 | 158 | func (e *muxTestEnv) cleanup() { | |
| 159 | - | for _, role := range []string{brokerMuxStdioCommand, brokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand} { | |
| 159 | + | for _, role := range []string{ports.BrokerMuxStdioCommand, ports.BrokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand} { | |
| 160 | 160 | raw, err := os.ReadFile(filepath.Join(e.recordDir, role+".pids")) | |
| 161 | 161 | if err != nil { | |
| 162 | 162 | continue | |
| @@ -260,7 +260,7 @@ func TestParseBrokerMuxArgs(t *testing.T) { | |||
| 260 | 260 | } | |
| 261 | 261 | for _, tt := range tests { | |
| 262 | 262 | t.Run(tt.name, func(t *testing.T) { | |
| 263 | - | command, err := parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, tt.args) | |
| 263 | + | command, err := parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, tt.args) | |
| 264 | 264 | if tt.wantErr != "" { | |
| 265 | 265 | require.Error(t, err) | |
| 266 | 266 | require.Contains(t, err.Error(), tt.wantErr) | |
| @@ -279,8 +279,8 @@ func TestBrokerMuxHelpersHiddenFromPublicHelp(t *testing.T) { | |||
| 279 | 279 | name string | |
| 280 | 280 | kind cmdKind | |
| 281 | 281 | }{ | |
| 282 | - | {brokerMuxStdioCommand, kindBrokerMuxStdio}, | |
| 283 | - | {brokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap}, | |
| 282 | + | {ports.BrokerMuxStdioCommand, kindBrokerMuxStdio}, | |
| 283 | + | {ports.BrokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap}, | |
| 284 | 284 | {brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy}, | |
| 285 | 285 | } { | |
| 286 | 286 | require.NotContains(t, usageText, tt.name) | |
| @@ -305,7 +305,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||
| 305 | 305 | route := loadTestRoute(t, map[string]any{ | |
| 306 | 306 | "kind": "ssh-stdio", | |
| 307 | 307 | "target": "user@host:2222", | |
| 308 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | |
| 308 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, | |
| 309 | 309 | "trust": map[string]any{"knownHostsFile": "/etc/ssh/known_hosts", "connectTimeout": "30s"}, | |
| 310 | 310 | }) | |
| 311 | 311 | spec, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartIfNeeded) | |
| @@ -317,7 +317,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||
| 317 | 317 | require.Contains(t, spec.Args, "ConnectTimeout=30") | |
| 318 | 318 | joined := strings.Join(spec.Args, " ") | |
| 319 | 319 | require.Contains(t, joined, "-- user@host:2222 ") | |
| 320 | - | require.Contains(t, joined, brokerMuxStdioCommand) | |
| 320 | + | require.Contains(t, joined, ports.BrokerMuxStdioCommand) | |
| 321 | 321 | require.Contains(t, joined, "'"+brokerDaemonStartArg+"' 'if-needed'") | |
| 322 | 322 | require.NotContains(t, joined, "StrictHostKeyChecking") | |
| 323 | 323 | require.NotContains(t, joined, "BatchMode") | |
| @@ -333,7 +333,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||
| 333 | 333 | plain := loadTestRoute(t, map[string]any{ | |
| 334 | 334 | "kind": "ssh-quic", | |
| 335 | 335 | "target": "host", | |
| 336 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 336 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 337 | 337 | }) | |
| 338 | 338 | plainSpec, err := sshMuxCommandSpec(plain, ports.BrokerDaemonExistingOnly) | |
| 339 | 339 | require.NoError(t, err) | |
| @@ -350,7 +350,7 @@ func TestSSHMuxCommandSpecRefusesUnknownStartMode(t *testing.T) { | |||
| 350 | 350 | route := loadTestRoute(t, map[string]any{ | |
| 351 | 351 | "kind": "ssh-stdio", | |
| 352 | 352 | "target": "host", | |
| 353 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | |
| 353 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, | |
| 354 | 354 | }) | |
| 355 | 355 | _, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartMode(0)) | |
| 356 | 356 | require.Error(t, err) | |
| @@ -403,7 +403,7 @@ func TestBrokerQUICPeerAddr(t *testing.T) { | |||
| 403 | 403 | func TestBrokerMuxHelperReturnsTypedNoDaemonExit(t *testing.T) { | |
| 404 | 404 | emptyProductionBrokerLayout(t, "") | |
| 405 | 405 | emptyProductionBrokerLayout(t, "") | |
| 406 | - | parsed, err := parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, []string{"--production"}) | |
| 406 | + | parsed, err := parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, []string{"--production"}) | |
| 407 | 407 | require.NoError(t, err) | |
| 408 | 408 | err = runBrokerMuxStdioCommand(context.Background(), parsed.brokerMux) | |
| 409 | 409 | var coded *exitCoded | |
| @@ -419,11 +419,11 @@ func TestBrokerServeSSHStdioRouteRoundTrip(t *testing.T) { | |||
| 419 | 419 | policy := brokerTestPolicy() | |
| 420 | 420 | fixture := remoteMuxFixture(t, policy) | |
| 421 | 421 | argsFile := installFakeSSH(t, fakeSSHExecHelper) | |
| 422 | - | t.Setenv(fakeSSHModeEnv, brokerMuxStdioCommand) | |
| 422 | + | t.Setenv(fakeSSHModeEnv, ports.BrokerMuxStdioCommand) | |
| 423 | 423 | writeMuxRoot(t, map[string]any{ | |
| 424 | 424 | "kind": "ssh-stdio", | |
| 425 | 425 | "target": "user@muxhost:2222", | |
| 426 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | |
| 426 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, | |
| 427 | 427 | }) | |
| 428 | 428 | ||
| 429 | 429 | socketPath, stop := startMuxBrokerServe(t) | |
| @@ -435,7 +435,7 @@ func TestBrokerServeSSHStdioRouteRoundTrip(t *testing.T) { | |||
| 435 | 435 | argv := readFileString(t, argsFile) | |
| 436 | 436 | require.Contains(t, argv, "-T\n") | |
| 437 | 437 | require.Contains(t, argv, "--\nuser@muxhost:2222\n") | |
| 438 | - | require.Contains(t, argv, brokerMuxStdioCommand) | |
| 438 | + | require.Contains(t, argv, ports.BrokerMuxStdioCommand) | |
| 439 | 439 | require.NotContains(t, argv, "UserKnownHostsFile=") | |
| 440 | 440 | require.NotContains(t, argv, "StrictHostKeyChecking") | |
| 441 | 441 | require.NotContains(t, argv, "BatchMode") | |
| @@ -451,11 +451,11 @@ func TestBrokerServeSSHQUICRouteRoundTrip(t *testing.T) { | |||
| 451 | 451 | policy := brokerTestPolicy() | |
| 452 | 452 | fixture := remoteMuxFixture(t, policy) | |
| 453 | 453 | installFakeSSH(t, fakeSSHExecHelper) | |
| 454 | - | t.Setenv(fakeSSHModeEnv, brokerMuxQUICBootstrapCommand) | |
| 454 | + | t.Setenv(fakeSSHModeEnv, ports.BrokerMuxQUICBootstrapCommand) | |
| 455 | 455 | writeMuxRoot(t, map[string]any{ | |
| 456 | 456 | "kind": "ssh-quic", | |
| 457 | 457 | "target": "127.0.0.1", | |
| 458 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 458 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 459 | 459 | }) | |
| 460 | 460 | ||
| 461 | 461 | socketPath, stop := startMuxBrokerServe(t) | |
| @@ -484,7 +484,7 @@ func TestBrokerMuxQUICRejectsBadPinWithoutLeakingSecret(t *testing.T) { | |||
| 484 | 484 | route, target := loadTestRouteTarget(t, map[string]any{ | |
| 485 | 485 | "kind": "ssh-quic", | |
| 486 | 486 | "target": "127.0.0.1", | |
| 487 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 487 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 488 | 488 | }) | |
| 489 | 489 | ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) | |
| 490 | 490 | defer cancel() | |
| @@ -514,7 +514,7 @@ func TestBrokerMuxQUICRejectsExpiredReadiness(t *testing.T) { | |||
| 514 | 514 | route, target := loadTestRouteTarget(t, map[string]any{ | |
| 515 | 515 | "kind": "ssh-quic", | |
| 516 | 516 | "target": "127.0.0.1", | |
| 517 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 517 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 518 | 518 | }) | |
| 519 | 519 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | |
| 520 | 520 | require.Error(t, err) | |
| @@ -534,7 +534,7 @@ func TestBrokerMuxDialCancellationReapsBootstrap(t *testing.T) { | |||
| 534 | 534 | route, target := loadTestRouteTarget(t, map[string]any{ | |
| 535 | 535 | "kind": "ssh-quic", | |
| 536 | 536 | "target": "127.0.0.1", | |
| 537 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 537 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 538 | 538 | }) | |
| 539 | 539 | ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) | |
| 540 | 540 | defer cancel() | |
| @@ -562,7 +562,7 @@ func TestBrokerMuxDialFloodedStderrIsReapedAndSanitized(t *testing.T) { | |||
| 562 | 562 | route, target := loadTestRouteTarget(t, map[string]any{ | |
| 563 | 563 | "kind": "ssh-quic", | |
| 564 | 564 | "target": "127.0.0.1", | |
| 565 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 565 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 566 | 566 | }) | |
| 567 | 567 | ||
| 568 | 568 | var logs bytes.Buffer | |
| @@ -606,7 +606,7 @@ func TestBrokerMuxDialStalledBootstrapUsesSetupTimeout(t *testing.T) { | |||
| 606 | 606 | route, target := loadTestRouteTarget(t, map[string]any{ | |
| 607 | 607 | "kind": "ssh-quic", | |
| 608 | 608 | "target": "127.0.0.1", | |
| 609 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | |
| 609 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, | |
| 610 | 610 | }) | |
| 611 | 611 | start := time.Now() | |
| 612 | 612 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | |
| @@ -156,7 +156,7 @@ func remoteMuxFixture(t *testing.T, policy ports.BrokerPolicy) *brokerMuxFixture | |||||
| 156 | 156 | ||||
| 157 | // cleanup kills every recorded mux helper process. | 157 | // cleanup kills every recorded mux helper process. | ||
| 158 | func (e *muxTestEnv) cleanup() { | 158 | func (e *muxTestEnv) cleanup() { | ||
| 159 | - | for _, role := range []string{brokerMuxStdioCommand, brokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand} { | 159 | + | for _, role := range []string{ports.BrokerMuxStdioCommand, ports.BrokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand} { |
| 160 | raw, err := os.ReadFile(filepath.Join(e.recordDir, role+".pids")) | 160 | raw, err := os.ReadFile(filepath.Join(e.recordDir, role+".pids")) | ||
| 161 | if err != nil { | 161 | if err != nil { | ||
| 162 | continue | 162 | continue | ||
| @@ -260,7 +260,7 @@ func TestParseBrokerMuxArgs(t *testing.T) { | |||||
| 260 | } | 260 | } | ||
| 261 | for _, tt := range tests { | 261 | for _, tt := range tests { | ||
| 262 | t.Run(tt.name, func(t *testing.T) { | 262 | t.Run(tt.name, func(t *testing.T) { | ||
| 263 | - | command, err := parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, tt.args) | 263 | + | command, err := parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, tt.args) |
| 264 | if tt.wantErr != "" { | 264 | if tt.wantErr != "" { | ||
| 265 | require.Error(t, err) | 265 | require.Error(t, err) | ||
| 266 | require.Contains(t, err.Error(), tt.wantErr) | 266 | require.Contains(t, err.Error(), tt.wantErr) | ||
| @@ -279,8 +279,8 @@ func TestBrokerMuxHelpersHiddenFromPublicHelp(t *testing.T) { | |||||
| 279 | name string | 279 | name string | ||
| 280 | kind cmdKind | 280 | kind cmdKind | ||
| 281 | }{ | 281 | }{ | ||
| 282 | - | {brokerMuxStdioCommand, kindBrokerMuxStdio}, | 282 | + | {ports.BrokerMuxStdioCommand, kindBrokerMuxStdio}, |
| 283 | - | {brokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap}, | 283 | + | {ports.BrokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap}, |
| 284 | {brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy}, | 284 | {brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy}, | ||
| 285 | } { | 285 | } { | ||
| 286 | require.NotContains(t, usageText, tt.name) | 286 | require.NotContains(t, usageText, tt.name) | ||
| @@ -305,7 +305,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||||
| 305 | route := loadTestRoute(t, map[string]any{ | 305 | route := loadTestRoute(t, map[string]any{ | ||
| 306 | "kind": "ssh-stdio", | 306 | "kind": "ssh-stdio", | ||
| 307 | "target": "user@host:2222", | 307 | "target": "user@host:2222", | ||
| 308 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | 308 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, |
| 309 | "trust": map[string]any{"knownHostsFile": "/etc/ssh/known_hosts", "connectTimeout": "30s"}, | 309 | "trust": map[string]any{"knownHostsFile": "/etc/ssh/known_hosts", "connectTimeout": "30s"}, | ||
| 310 | }) | 310 | }) | ||
| 311 | spec, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartIfNeeded) | 311 | spec, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartIfNeeded) | ||
| @@ -317,7 +317,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||||
| 317 | require.Contains(t, spec.Args, "ConnectTimeout=30") | 317 | require.Contains(t, spec.Args, "ConnectTimeout=30") | ||
| 318 | joined := strings.Join(spec.Args, " ") | 318 | joined := strings.Join(spec.Args, " ") | ||
| 319 | require.Contains(t, joined, "-- user@host:2222 ") | 319 | require.Contains(t, joined, "-- user@host:2222 ") | ||
| 320 | - | require.Contains(t, joined, brokerMuxStdioCommand) | 320 | + | require.Contains(t, joined, ports.BrokerMuxStdioCommand) |
| 321 | require.Contains(t, joined, "'"+brokerDaemonStartArg+"' 'if-needed'") | 321 | require.Contains(t, joined, "'"+brokerDaemonStartArg+"' 'if-needed'") | ||
| 322 | require.NotContains(t, joined, "StrictHostKeyChecking") | 322 | require.NotContains(t, joined, "StrictHostKeyChecking") | ||
| 323 | require.NotContains(t, joined, "BatchMode") | 323 | require.NotContains(t, joined, "BatchMode") | ||
| @@ -333,7 +333,7 @@ func TestSSHMuxCommandSpecPreservesTrustAndNoPTY(t *testing.T) { | |||||
| 333 | plain := loadTestRoute(t, map[string]any{ | 333 | plain := loadTestRoute(t, map[string]any{ | ||
| 334 | "kind": "ssh-quic", | 334 | "kind": "ssh-quic", | ||
| 335 | "target": "host", | 335 | "target": "host", | ||
| 336 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 336 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 337 | }) | 337 | }) | ||
| 338 | plainSpec, err := sshMuxCommandSpec(plain, ports.BrokerDaemonExistingOnly) | 338 | plainSpec, err := sshMuxCommandSpec(plain, ports.BrokerDaemonExistingOnly) | ||
| 339 | require.NoError(t, err) | 339 | require.NoError(t, err) | ||
| @@ -350,7 +350,7 @@ func TestSSHMuxCommandSpecRefusesUnknownStartMode(t *testing.T) { | |||||
| 350 | route := loadTestRoute(t, map[string]any{ | 350 | route := loadTestRoute(t, map[string]any{ | ||
| 351 | "kind": "ssh-stdio", | 351 | "kind": "ssh-stdio", | ||
| 352 | "target": "host", | 352 | "target": "host", | ||
| 353 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | 353 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, |
| 354 | }) | 354 | }) | ||
| 355 | _, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartMode(0)) | 355 | _, err := sshMuxCommandSpec(route, ports.BrokerDaemonStartMode(0)) | ||
| 356 | require.Error(t, err) | 356 | require.Error(t, err) | ||
| @@ -403,7 +403,7 @@ func TestBrokerQUICPeerAddr(t *testing.T) { | |||||
| 403 | func TestBrokerMuxHelperReturnsTypedNoDaemonExit(t *testing.T) { | 403 | func TestBrokerMuxHelperReturnsTypedNoDaemonExit(t *testing.T) { | ||
| 404 | emptyProductionBrokerLayout(t, "") | 404 | emptyProductionBrokerLayout(t, "") | ||
| 405 | emptyProductionBrokerLayout(t, "") | 405 | emptyProductionBrokerLayout(t, "") | ||
| 406 | - | parsed, err := parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, []string{"--production"}) | 406 | + | parsed, err := parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, []string{"--production"}) |
| 407 | require.NoError(t, err) | 407 | require.NoError(t, err) | ||
| 408 | err = runBrokerMuxStdioCommand(context.Background(), parsed.brokerMux) | 408 | err = runBrokerMuxStdioCommand(context.Background(), parsed.brokerMux) | ||
| 409 | var coded *exitCoded | 409 | var coded *exitCoded | ||
| @@ -419,11 +419,11 @@ func TestBrokerServeSSHStdioRouteRoundTrip(t *testing.T) { | |||||
| 419 | policy := brokerTestPolicy() | 419 | policy := brokerTestPolicy() | ||
| 420 | fixture := remoteMuxFixture(t, policy) | 420 | fixture := remoteMuxFixture(t, policy) | ||
| 421 | argsFile := installFakeSSH(t, fakeSSHExecHelper) | 421 | argsFile := installFakeSSH(t, fakeSSHExecHelper) | ||
| 422 | - | t.Setenv(fakeSSHModeEnv, brokerMuxStdioCommand) | 422 | + | t.Setenv(fakeSSHModeEnv, ports.BrokerMuxStdioCommand) |
| 423 | writeMuxRoot(t, map[string]any{ | 423 | writeMuxRoot(t, map[string]any{ | ||
| 424 | "kind": "ssh-stdio", | 424 | "kind": "ssh-stdio", | ||
| 425 | "target": "user@muxhost:2222", | 425 | "target": "user@muxhost:2222", | ||
| 426 | - | "argv": []any{"vev", brokerMuxStdioCommand, "--production"}, | 426 | + | "argv": []any{"vev", ports.BrokerMuxStdioCommand, "--production"}, |
| 427 | }) | 427 | }) | ||
| 428 | 428 | ||||
| 429 | socketPath, stop := startMuxBrokerServe(t) | 429 | socketPath, stop := startMuxBrokerServe(t) | ||
| @@ -435,7 +435,7 @@ func TestBrokerServeSSHStdioRouteRoundTrip(t *testing.T) { | |||||
| 435 | argv := readFileString(t, argsFile) | 435 | argv := readFileString(t, argsFile) | ||
| 436 | require.Contains(t, argv, "-T\n") | 436 | require.Contains(t, argv, "-T\n") | ||
| 437 | require.Contains(t, argv, "--\nuser@muxhost:2222\n") | 437 | require.Contains(t, argv, "--\nuser@muxhost:2222\n") | ||
| 438 | - | require.Contains(t, argv, brokerMuxStdioCommand) | 438 | + | require.Contains(t, argv, ports.BrokerMuxStdioCommand) |
| 439 | require.NotContains(t, argv, "UserKnownHostsFile=") | 439 | require.NotContains(t, argv, "UserKnownHostsFile=") | ||
| 440 | require.NotContains(t, argv, "StrictHostKeyChecking") | 440 | require.NotContains(t, argv, "StrictHostKeyChecking") | ||
| 441 | require.NotContains(t, argv, "BatchMode") | 441 | require.NotContains(t, argv, "BatchMode") | ||
| @@ -451,11 +451,11 @@ func TestBrokerServeSSHQUICRouteRoundTrip(t *testing.T) { | |||||
| 451 | policy := brokerTestPolicy() | 451 | policy := brokerTestPolicy() | ||
| 452 | fixture := remoteMuxFixture(t, policy) | 452 | fixture := remoteMuxFixture(t, policy) | ||
| 453 | installFakeSSH(t, fakeSSHExecHelper) | 453 | installFakeSSH(t, fakeSSHExecHelper) | ||
| 454 | - | t.Setenv(fakeSSHModeEnv, brokerMuxQUICBootstrapCommand) | 454 | + | t.Setenv(fakeSSHModeEnv, ports.BrokerMuxQUICBootstrapCommand) |
| 455 | writeMuxRoot(t, map[string]any{ | 455 | writeMuxRoot(t, map[string]any{ | ||
| 456 | "kind": "ssh-quic", | 456 | "kind": "ssh-quic", | ||
| 457 | "target": "127.0.0.1", | 457 | "target": "127.0.0.1", | ||
| 458 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 458 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 459 | }) | 459 | }) | ||
| 460 | 460 | ||||
| 461 | socketPath, stop := startMuxBrokerServe(t) | 461 | socketPath, stop := startMuxBrokerServe(t) | ||
| @@ -484,7 +484,7 @@ func TestBrokerMuxQUICRejectsBadPinWithoutLeakingSecret(t *testing.T) { | |||||
| 484 | route, target := loadTestRouteTarget(t, map[string]any{ | 484 | route, target := loadTestRouteTarget(t, map[string]any{ | ||
| 485 | "kind": "ssh-quic", | 485 | "kind": "ssh-quic", | ||
| 486 | "target": "127.0.0.1", | 486 | "target": "127.0.0.1", | ||
| 487 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 487 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 488 | }) | 488 | }) | ||
| 489 | ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) | 489 | ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) | ||
| 490 | defer cancel() | 490 | defer cancel() | ||
| @@ -514,7 +514,7 @@ func TestBrokerMuxQUICRejectsExpiredReadiness(t *testing.T) { | |||||
| 514 | route, target := loadTestRouteTarget(t, map[string]any{ | 514 | route, target := loadTestRouteTarget(t, map[string]any{ | ||
| 515 | "kind": "ssh-quic", | 515 | "kind": "ssh-quic", | ||
| 516 | "target": "127.0.0.1", | 516 | "target": "127.0.0.1", | ||
| 517 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 517 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 518 | }) | 518 | }) | ||
| 519 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | 519 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | ||
| 520 | require.Error(t, err) | 520 | require.Error(t, err) | ||
| @@ -534,7 +534,7 @@ func TestBrokerMuxDialCancellationReapsBootstrap(t *testing.T) { | |||||
| 534 | route, target := loadTestRouteTarget(t, map[string]any{ | 534 | route, target := loadTestRouteTarget(t, map[string]any{ | ||
| 535 | "kind": "ssh-quic", | 535 | "kind": "ssh-quic", | ||
| 536 | "target": "127.0.0.1", | 536 | "target": "127.0.0.1", | ||
| 537 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 537 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 538 | }) | 538 | }) | ||
| 539 | ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) | 539 | ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) | ||
| 540 | defer cancel() | 540 | defer cancel() | ||
| @@ -562,7 +562,7 @@ func TestBrokerMuxDialFloodedStderrIsReapedAndSanitized(t *testing.T) { | |||||
| 562 | route, target := loadTestRouteTarget(t, map[string]any{ | 562 | route, target := loadTestRouteTarget(t, map[string]any{ | ||
| 563 | "kind": "ssh-quic", | 563 | "kind": "ssh-quic", | ||
| 564 | "target": "127.0.0.1", | 564 | "target": "127.0.0.1", | ||
| 565 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 565 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 566 | }) | 566 | }) | ||
| 567 | 567 | ||||
| 568 | var logs bytes.Buffer | 568 | var logs bytes.Buffer | ||
| @@ -606,7 +606,7 @@ func TestBrokerMuxDialStalledBootstrapUsesSetupTimeout(t *testing.T) { | |||||
| 606 | route, target := loadTestRouteTarget(t, map[string]any{ | 606 | route, target := loadTestRouteTarget(t, map[string]any{ | ||
| 607 | "kind": "ssh-quic", | 607 | "kind": "ssh-quic", | ||
| 608 | "target": "127.0.0.1", | 608 | "target": "127.0.0.1", | ||
| 609 | - | "argv": []any{"vev", brokerMuxQUICBootstrapCommand, "--production"}, | 609 | + | "argv": []any{"vev", ports.BrokerMuxQUICBootstrapCommand, "--production"}, |
| 610 | }) | 610 | }) | ||
| 611 | start := time.Now() | 611 | start := time.Now() | ||
| 612 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | 612 | _, err := dialBrokerRoute(context.Background(), route, target, slog.New(slog.NewTextHandler(io.Discard, nil))) | ||
M internal/app/broker_routes_test.go +1 -1
| @@ -52,7 +52,7 @@ func TestBrokerRoutesDurableFirstAddAndRestart(t *testing.T) { | |||
| 52 | 52 | route, err := brokerconfig.RouteFromSpec(spec) | |
| 53 | 53 | require.NoError(t, err) | |
| 54 | 54 | require.Equal(t, brokerconfig.RouteSSHStdio, route.Kind()) | |
| 55 | - | require.Equal(t, []string{"vev", brokerMuxStdioCommand, "--production"}, route.Argv()) | |
| 55 | + | require.Equal(t, []string{"vev", ports.BrokerMuxStdioCommand, "--production"}, route.Argv()) | |
| 56 | 56 | identity, err := registry.BindAuthenticatedIdentity(ctx, ports.BrokerIdentityBindingRequest{Fence: target.Fence, Policy: policy, Identity: "daemon-test"}) | |
| 57 | 57 | require.NoError(t, err) | |
| 58 | 58 | _, err = routes.AuthorizeRemoteDial(ctx, target) | |
| @@ -52,7 +52,7 @@ func TestBrokerRoutesDurableFirstAddAndRestart(t *testing.T) { | |||||
| 52 | route, err := brokerconfig.RouteFromSpec(spec) | 52 | route, err := brokerconfig.RouteFromSpec(spec) | ||
| 53 | require.NoError(t, err) | 53 | require.NoError(t, err) | ||
| 54 | require.Equal(t, brokerconfig.RouteSSHStdio, route.Kind()) | 54 | require.Equal(t, brokerconfig.RouteSSHStdio, route.Kind()) | ||
| 55 | - | require.Equal(t, []string{"vev", brokerMuxStdioCommand, "--production"}, route.Argv()) | 55 | + | require.Equal(t, []string{"vev", ports.BrokerMuxStdioCommand, "--production"}, route.Argv()) |
| 56 | identity, err := registry.BindAuthenticatedIdentity(ctx, ports.BrokerIdentityBindingRequest{Fence: target.Fence, Policy: policy, Identity: "daemon-test"}) | 56 | identity, err := registry.BindAuthenticatedIdentity(ctx, ports.BrokerIdentityBindingRequest{Fence: target.Fence, Policy: policy, Identity: "daemon-test"}) | ||
| 57 | require.NoError(t, err) | 57 | require.NoError(t, err) | ||
| 58 | _, err = routes.AuthorizeRemoteDial(ctx, target) | 58 | _, err = routes.AuthorizeRemoteDial(ctx, target) | ||
M internal/app/cli.go +5 -4
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "strings" | |
| 6 | 6 | ||
| 7 | 7 | "github.com/bnema/vev/internal/domain" | |
| 8 | + | "github.com/bnema/vev/internal/ports" | |
| 8 | 9 | "github.com/bnema/vev/internal/protocol" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| @@ -172,10 +173,10 @@ func parseSubcommand(args []string) (command, error) { | |||
| 172 | 173 | case brokerReadyCommand: | |
| 173 | 174 | options, err := parseBrokerReadyArgs(args[1:]) | |
| 174 | 175 | return command{kind: kindBrokerReady, brokerReady: options}, err | |
| 175 | - | case brokerMuxStdioCommand: | |
| 176 | - | return parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, args[1:]) | |
| 177 | - | case brokerMuxQUICBootstrapCommand: | |
| 178 | - | return parseBrokerMuxArgs(brokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap, args[1:]) | |
| 176 | + | case ports.BrokerMuxStdioCommand: | |
| 177 | + | return parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, args[1:]) | |
| 178 | + | case ports.BrokerMuxQUICBootstrapCommand: | |
| 179 | + | return parseBrokerMuxArgs(ports.BrokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap, args[1:]) | |
| 179 | 180 | case brokerMuxQUICProxyCommand: | |
| 180 | 181 | return parseBrokerMuxArgs(brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy, args[1:]) | |
| 181 | 182 | case uiRemoteCleanupCommand: | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "strings" | 5 | "strings" | ||
| 6 | 6 | ||||
| 7 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| 8 | + | "github.com/bnema/vev/internal/ports" | |||
| 8 | "github.com/bnema/vev/internal/protocol" | 9 | "github.com/bnema/vev/internal/protocol" | ||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| @@ -172,10 +173,10 @@ func parseSubcommand(args []string) (command, error) { | |||||
| 172 | case brokerReadyCommand: | 173 | case brokerReadyCommand: | ||
| 173 | options, err := parseBrokerReadyArgs(args[1:]) | 174 | options, err := parseBrokerReadyArgs(args[1:]) | ||
| 174 | return command{kind: kindBrokerReady, brokerReady: options}, err | 175 | return command{kind: kindBrokerReady, brokerReady: options}, err | ||
| 175 | - | case brokerMuxStdioCommand: | 176 | + | case ports.BrokerMuxStdioCommand: |
| 176 | - | return parseBrokerMuxArgs(brokerMuxStdioCommand, kindBrokerMuxStdio, args[1:]) | 177 | + | return parseBrokerMuxArgs(ports.BrokerMuxStdioCommand, kindBrokerMuxStdio, args[1:]) |
| 177 | - | case brokerMuxQUICBootstrapCommand: | 178 | + | case ports.BrokerMuxQUICBootstrapCommand: |
| 178 | - | return parseBrokerMuxArgs(brokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap, args[1:]) | 179 | + | return parseBrokerMuxArgs(ports.BrokerMuxQUICBootstrapCommand, kindBrokerMuxQUICBootstrap, args[1:]) |
| 179 | case brokerMuxQUICProxyCommand: | 180 | case brokerMuxQUICProxyCommand: | ||
| 180 | return parseBrokerMuxArgs(brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy, args[1:]) | 181 | return parseBrokerMuxArgs(brokerMuxQUICProxyCommand, kindBrokerMuxQUICProxy, args[1:]) | ||
| 181 | case uiRemoteCleanupCommand: | 182 | case uiRemoteCleanupCommand: | ||
M internal/app/spawn_test.go +3 -1
| @@ -12,6 +12,8 @@ import ( | |||
| 12 | 12 | "syscall" | |
| 13 | 13 | "testing" | |
| 14 | 14 | "time" | |
| 15 | + | ||
| 16 | + | "github.com/bnema/vev/internal/ports" | |
| 15 | 17 | ) | |
| 16 | 18 | ||
| 17 | 19 | const ( | |
| @@ -28,7 +30,7 @@ func TestMain(m *testing.M) { | |||
| 28 | 30 | // runs. | |
| 29 | 31 | if len(os.Args) >= 2 && os.Getenv(brokerHelperEnv) == "1" { | |
| 30 | 32 | switch os.Args[1] { | |
| 31 | - | case brokerMuxStdioCommand, brokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand, | |
| 33 | + | case ports.BrokerMuxStdioCommand, ports.BrokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand, | |
| 32 | 34 | brokerReadyCommand, | |
| 33 | 35 | productionBrokerServeCommand, productionBrokerLauncherCommand, "--daemon": | |
| 34 | 36 | recordBrokerHelperProcess(os.Args[1]) | |
| @@ -12,6 +12,8 @@ import ( | |||||
| 12 | "syscall" | 12 | "syscall" | ||
| 13 | "testing" | 13 | "testing" | ||
| 14 | "time" | 14 | "time" | ||
| 15 | + | ||||
| 16 | + | "github.com/bnema/vev/internal/ports" | |||
| 15 | ) | 17 | ) | ||
| 16 | 18 | ||||
| 17 | const ( | 19 | const ( | ||
| @@ -28,7 +30,7 @@ func TestMain(m *testing.M) { | |||||
| 28 | // runs. | 30 | // runs. | ||
| 29 | if len(os.Args) >= 2 && os.Getenv(brokerHelperEnv) == "1" { | 31 | if len(os.Args) >= 2 && os.Getenv(brokerHelperEnv) == "1" { | ||
| 30 | switch os.Args[1] { | 32 | switch os.Args[1] { | ||
| 31 | - | case brokerMuxStdioCommand, brokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand, | 33 | + | case ports.BrokerMuxStdioCommand, ports.BrokerMuxQUICBootstrapCommand, brokerMuxQUICProxyCommand, |
| 32 | brokerReadyCommand, | 34 | brokerReadyCommand, | ||
| 33 | productionBrokerServeCommand, productionBrokerLauncherCommand, "--daemon": | 35 | productionBrokerServeCommand, productionBrokerLauncherCommand, "--daemon": | ||
| 34 | recordBrokerHelperProcess(os.Args[1]) | 36 | recordBrokerHelperProcess(os.Args[1]) | ||
M internal/snapshotcodec/object_compress.go +26 -10
| @@ -58,18 +58,10 @@ func inflateObjectPayload(compressed []byte, size uint32) ([]byte, error) { | |||
| 58 | 58 | return nil, invalid(err) | |
| 59 | 59 | } | |
| 60 | 60 | defer objectDecompressorPool.Put(r) | |
| 61 | - | out := bytes.NewBuffer(make([]byte, 0, min(int(size), inflateInitialCap))) | |
| 62 | - | if _, err := io.Copy(out, io.LimitReader(r, int64(size))); err != nil { | |
| 61 | + | payload, err := readExact(r, int(size)) | |
| 62 | + | if err != nil { | |
| 63 | 63 | return nil, invalid(err) | |
| 64 | 64 | } | |
| 65 | - | if out.Len() != int(size) { | |
| 66 | - | return nil, invalid(io.ErrUnexpectedEOF) | |
| 67 | - | } | |
| 68 | - | payload := out.Bytes() | |
| 69 | - | if cap(payload) > len(payload)+len(payload)/4 { | |
| 70 | - | // Restored payloads are retained; drop the growth slack. | |
| 71 | - | payload = bytes.Clone(payload) | |
| 72 | - | } | |
| 73 | 65 | var extra [1]byte | |
| 74 | 66 | if n, err := r.Read(extra[:]); n != 0 || err != io.EOF { | |
| 75 | 67 | if err == nil || err == io.EOF { | |
| @@ -83,6 +75,30 @@ func inflateObjectPayload(compressed []byte, size uint32) ([]byte, error) { | |||
| 83 | 75 | return payload, nil | |
| 84 | 76 | } | |
| 85 | 77 | ||
| 78 | + | // readExact reads exactly size bytes. It doubles the buffer as data arrives, | |
| 79 | + | // starting at inflateInitialCap and capping every step at size, so a corrupt | |
| 80 | + | // declared length allocates at most about four times what the stream produces | |
| 81 | + | // (about three times live at peak) and the returned payload has no slack. | |
| 82 | + | func readExact(r io.Reader, size int) ([]byte, error) { | |
| 83 | + | buf := make([]byte, min(size, inflateInitialCap)) | |
| 84 | + | for n := 0; ; { | |
| 85 | + | m, err := io.ReadFull(r, buf[n:]) | |
| 86 | + | n += m | |
| 87 | + | if err != nil { | |
| 88 | + | if err == io.EOF { | |
| 89 | + | err = io.ErrUnexpectedEOF | |
| 90 | + | } | |
| 91 | + | return nil, err | |
| 92 | + | } | |
| 93 | + | if n == size { | |
| 94 | + | return buf, nil | |
| 95 | + | } | |
| 96 | + | grown := make([]byte, min(2*len(buf), size)) | |
| 97 | + | copy(grown, buf) | |
| 98 | + | buf = grown | |
| 99 | + | } | |
| 100 | + | } | |
| 101 | + | ||
| 86 | 102 | func objectDecompressor(source io.Reader) (io.ReadCloser, error) { | |
| 87 | 103 | if pooled, ok := objectDecompressorPool.Get().(io.ReadCloser); ok { | |
| 88 | 104 | if err := pooled.(zlib.Resetter).Reset(source, nil); err != nil { | |
| @@ -58,18 +58,10 @@ func inflateObjectPayload(compressed []byte, size uint32) ([]byte, error) { | |||||
| 58 | return nil, invalid(err) | 58 | return nil, invalid(err) | ||
| 59 | } | 59 | } | ||
| 60 | defer objectDecompressorPool.Put(r) | 60 | defer objectDecompressorPool.Put(r) | ||
| 61 | - | out := bytes.NewBuffer(make([]byte, 0, min(int(size), inflateInitialCap))) | 61 | + | payload, err := readExact(r, int(size)) |
| 62 | - | if _, err := io.Copy(out, io.LimitReader(r, int64(size))); err != nil { | 62 | + | if err != nil { |
| 63 | return nil, invalid(err) | 63 | return nil, invalid(err) | ||
| 64 | } | 64 | } | ||
| 65 | - | if out.Len() != int(size) { | |||
| 66 | - | return nil, invalid(io.ErrUnexpectedEOF) | |||
| 67 | - | } | |||
| 68 | - | payload := out.Bytes() | |||
| 69 | - | if cap(payload) > len(payload)+len(payload)/4 { | |||
| 70 | - | // Restored payloads are retained; drop the growth slack. | |||
| 71 | - | payload = bytes.Clone(payload) | |||
| 72 | - | } | |||
| 73 | var extra [1]byte | 65 | var extra [1]byte | ||
| 74 | if n, err := r.Read(extra[:]); n != 0 || err != io.EOF { | 66 | if n, err := r.Read(extra[:]); n != 0 || err != io.EOF { | ||
| 75 | if err == nil || err == io.EOF { | 67 | if err == nil || err == io.EOF { | ||
| @@ -83,6 +75,30 @@ func inflateObjectPayload(compressed []byte, size uint32) ([]byte, error) { | |||||
| 83 | return payload, nil | 75 | return payload, nil | ||
| 84 | } | 76 | } | ||
| 85 | 77 | ||||
| 78 | + | // readExact reads exactly size bytes. It doubles the buffer as data arrives, | |||
| 79 | + | // starting at inflateInitialCap and capping every step at size, so a corrupt | |||
| 80 | + | // declared length allocates at most about four times what the stream produces | |||
| 81 | + | // (about three times live at peak) and the returned payload has no slack. | |||
| 82 | + | func readExact(r io.Reader, size int) ([]byte, error) { | |||
| 83 | + | buf := make([]byte, min(size, inflateInitialCap)) | |||
| 84 | + | for n := 0; ; { | |||
| 85 | + | m, err := io.ReadFull(r, buf[n:]) | |||
| 86 | + | n += m | |||
| 87 | + | if err != nil { | |||
| 88 | + | if err == io.EOF { | |||
| 89 | + | err = io.ErrUnexpectedEOF | |||
| 90 | + | } | |||
| 91 | + | return nil, err | |||
| 92 | + | } | |||
| 93 | + | if n == size { | |||
| 94 | + | return buf, nil | |||
| 95 | + | } | |||
| 96 | + | grown := make([]byte, min(2*len(buf), size)) | |||
| 97 | + | copy(grown, buf) | |||
| 98 | + | buf = grown | |||
| 99 | + | } | |||
| 100 | + | } | |||
| 101 | + | ||||
| 86 | func objectDecompressor(source io.Reader) (io.ReadCloser, error) { | 102 | func objectDecompressor(source io.Reader) (io.ReadCloser, error) { | ||
| 87 | if pooled, ok := objectDecompressorPool.Get().(io.ReadCloser); ok { | 103 | if pooled, ok := objectDecompressorPool.Get().(io.ReadCloser); ok { | ||
| 88 | if err := pooled.(zlib.Resetter).Reset(source, nil); err != nil { | 104 | if err := pooled.(zlib.Resetter).Reset(source, nil); err != nil { | ||
M internal/snapshotcodec/object_compress_test.go +41 -0
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "encoding/binary" | |
| 6 | 6 | "errors" | |
| 7 | 7 | "hash/crc32" | |
| 8 | + | "runtime" | |
| 8 | 9 | "testing" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| @@ -65,3 +66,43 @@ func TestObjectCompressionRejectsMalformedPayloads(t *testing.T) { | |||
| 65 | 66 | }) | |
| 66 | 67 | } | |
| 67 | 68 | } | |
| 69 | + | ||
| 70 | + | // TestInflateObjectPayloadAllocation pins the inflate memory bound: a payload | |
| 71 | + | // costs about its own size, and a corrupt maximum declared length on a small | |
| 72 | + | // stream never allocates the declared size. | |
| 73 | + | func TestInflateObjectPayloadAllocation(t *testing.T) { | |
| 74 | + | for _, tc := range []struct { | |
| 75 | + | name string | |
| 76 | + | size int | |
| 77 | + | declared uint32 | |
| 78 | + | maxBytes int64 | |
| 79 | + | }{ | |
| 80 | + | {name: "small payload", size: 64 << 10, maxBytes: 64<<10 + 64<<10}, | |
| 81 | + | {name: "payload at the initial cap", size: inflateInitialCap, maxBytes: inflateInitialCap + 64<<10}, | |
| 82 | + | {name: "payload above the initial cap", size: 3 << 20, maxBytes: 2*(3<<20) + 64<<10}, | |
| 83 | + | {name: "declared maximum on a small stream", size: 64 << 10, declared: maxObjectPayloadSize, maxBytes: 2*inflateInitialCap + 64<<10}, | |
| 84 | + | } { | |
| 85 | + | t.Run(tc.name, func(t *testing.T) { | |
| 86 | + | payload := bytes.Repeat([]byte("history row "), tc.size/12+1)[:tc.size] | |
| 87 | + | compressed, err := compressObjectPayload(nil, payload) | |
| 88 | + | if err != nil { | |
| 89 | + | t.Fatal(err) | |
| 90 | + | } | |
| 91 | + | declared := uint32(tc.size) | |
| 92 | + | if tc.declared != 0 { | |
| 93 | + | declared = tc.declared | |
| 94 | + | } | |
| 95 | + | _, _ = inflateObjectPayload(compressed, declared) // warm the decompressor pool | |
| 96 | + | const runs = 8 | |
| 97 | + | var before, after runtime.MemStats | |
| 98 | + | runtime.ReadMemStats(&before) | |
| 99 | + | for range runs { | |
| 100 | + | _, _ = inflateObjectPayload(compressed, declared) | |
| 101 | + | } | |
| 102 | + | runtime.ReadMemStats(&after) | |
| 103 | + | if got := int64(after.TotalAlloc-before.TotalAlloc) / runs; got > tc.maxBytes { | |
| 104 | + | t.Fatalf("inflate allocated %d bytes/op, want at most %d", got, tc.maxBytes) | |
| 105 | + | } | |
| 106 | + | }) | |
| 107 | + | } | |
| 108 | + | } | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "encoding/binary" | 5 | "encoding/binary" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "hash/crc32" | 7 | "hash/crc32" | ||
| 8 | + | "runtime" | |||
| 8 | "testing" | 9 | "testing" | ||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| @@ -65,3 +66,43 @@ func TestObjectCompressionRejectsMalformedPayloads(t *testing.T) { | |||||
| 65 | }) | 66 | }) | ||
| 66 | } | 67 | } | ||
| 67 | } | 68 | } | ||
| 69 | + | ||||
| 70 | + | // TestInflateObjectPayloadAllocation pins the inflate memory bound: a payload | |||
| 71 | + | // costs about its own size, and a corrupt maximum declared length on a small | |||
| 72 | + | // stream never allocates the declared size. | |||
| 73 | + | func TestInflateObjectPayloadAllocation(t *testing.T) { | |||
| 74 | + | for _, tc := range []struct { | |||
| 75 | + | name string | |||
| 76 | + | size int | |||
| 77 | + | declared uint32 | |||
| 78 | + | maxBytes int64 | |||
| 79 | + | }{ | |||
| 80 | + | {name: "small payload", size: 64 << 10, maxBytes: 64<<10 + 64<<10}, | |||
| 81 | + | {name: "payload at the initial cap", size: inflateInitialCap, maxBytes: inflateInitialCap + 64<<10}, | |||
| 82 | + | {name: "payload above the initial cap", size: 3 << 20, maxBytes: 2*(3<<20) + 64<<10}, | |||
| 83 | + | {name: "declared maximum on a small stream", size: 64 << 10, declared: maxObjectPayloadSize, maxBytes: 2*inflateInitialCap + 64<<10}, | |||
| 84 | + | } { | |||
| 85 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 86 | + | payload := bytes.Repeat([]byte("history row "), tc.size/12+1)[:tc.size] | |||
| 87 | + | compressed, err := compressObjectPayload(nil, payload) | |||
| 88 | + | if err != nil { | |||
| 89 | + | t.Fatal(err) | |||
| 90 | + | } | |||
| 91 | + | declared := uint32(tc.size) | |||
| 92 | + | if tc.declared != 0 { | |||
| 93 | + | declared = tc.declared | |||
| 94 | + | } | |||
| 95 | + | _, _ = inflateObjectPayload(compressed, declared) // warm the decompressor pool | |||
| 96 | + | const runs = 8 | |||
| 97 | + | var before, after runtime.MemStats | |||
| 98 | + | runtime.ReadMemStats(&before) | |||
| 99 | + | for range runs { | |||
| 100 | + | _, _ = inflateObjectPayload(compressed, declared) | |||
| 101 | + | } | |||
| 102 | + | runtime.ReadMemStats(&after) | |||
| 103 | + | if got := int64(after.TotalAlloc-before.TotalAlloc) / runs; got > tc.maxBytes { | |||
| 104 | + | t.Fatalf("inflate allocated %d bytes/op, want at most %d", got, tc.maxBytes) | |||
| 105 | + | } | |||
| 106 | + | }) | |||
| 107 | + | } | |||
| 108 | + | } | |||
M internal/usecase/client/attachment_outbox_test.go +30 -0
| @@ -282,6 +282,36 @@ func TestResumeWatchHeldOverflowIsSticky(t *testing.T) { | |||
| 282 | 282 | require.Zero(t, next.heldLen(), "a later phase must not accept keys after an overflow") | |
| 283 | 283 | } | |
| 284 | 284 | ||
| 285 | + | // TestResumeWatchOverflowKeepsHeldPrefix overflows one phase and checks the | |
| 286 | + | // next phase still replays the gap-free prefix the first phase handed back. | |
| 287 | + | // TestResumeWatchHeldOverflowIsSticky covers dropping keys typed after it. | |
| 288 | + | func TestResumeWatchOverflowKeepsHeldPrefix(t *testing.T) { | |
| 289 | + | reader := &inputTestReader{chunks: make(chan []byte, 2)} | |
| 290 | + | pump := newTerminalInputPump(reader) | |
| 291 | + | pump.start() | |
| 292 | + | t.Cleanup(func() { | |
| 293 | + | pump.stop() | |
| 294 | + | close(reader.chunks) | |
| 295 | + | }) | |
| 296 | + | lifetime := &terminalInputLifetime{eof: make(chan error, 1), pump: pump} | |
| 297 | + | clock := newSupervisorTestClock() | |
| 298 | + | ||
| 299 | + | first := lifetime.watchResume(clock, nil) | |
| 300 | + | reader.chunks <- []byte("ls") | |
| 301 | + | require.Eventually(t, func() bool { return first.heldLen() == 2 }, 5*time.Second, time.Millisecond) | |
| 302 | + | first.hold(bytes.Repeat([]byte("x"), resumeHeldInputLimit)) | |
| 303 | + | require.True(t, lifetime.resumeOverflowed.Load()) | |
| 304 | + | first.release() | |
| 305 | + | ||
| 306 | + | second := lifetime.watchResume(clock, nil) | |
| 307 | + | require.Eventually(t, func() bool { return second.heldLen() == 2 }, 5*time.Second, time.Millisecond) | |
| 308 | + | second.release() | |
| 309 | + | ||
| 310 | + | pump.mu.Lock() | |
| 311 | + | defer pump.mu.Unlock() | |
| 312 | + | require.Equal(t, "ls", string(pump.residual)) | |
| 313 | + | } | |
| 314 | + | ||
| 285 | 315 | // stallingSessionStream passes writes through until stall is set; stalled | |
| 286 | 316 | // writes block until release closes, then fail with failErr when set. | |
| 287 | 317 | type stallingSessionStream struct { | |
| @@ -282,6 +282,36 @@ func TestResumeWatchHeldOverflowIsSticky(t *testing.T) { | |||||
| 282 | require.Zero(t, next.heldLen(), "a later phase must not accept keys after an overflow") | 282 | require.Zero(t, next.heldLen(), "a later phase must not accept keys after an overflow") | ||
| 283 | } | 283 | } | ||
| 284 | 284 | ||||
| 285 | + | // TestResumeWatchOverflowKeepsHeldPrefix overflows one phase and checks the | |||
| 286 | + | // next phase still replays the gap-free prefix the first phase handed back. | |||
| 287 | + | // TestResumeWatchHeldOverflowIsSticky covers dropping keys typed after it. | |||
| 288 | + | func TestResumeWatchOverflowKeepsHeldPrefix(t *testing.T) { | |||
| 289 | + | reader := &inputTestReader{chunks: make(chan []byte, 2)} | |||
| 290 | + | pump := newTerminalInputPump(reader) | |||
| 291 | + | pump.start() | |||
| 292 | + | t.Cleanup(func() { | |||
| 293 | + | pump.stop() | |||
| 294 | + | close(reader.chunks) | |||
| 295 | + | }) | |||
| 296 | + | lifetime := &terminalInputLifetime{eof: make(chan error, 1), pump: pump} | |||
| 297 | + | clock := newSupervisorTestClock() | |||
| 298 | + | ||||
| 299 | + | first := lifetime.watchResume(clock, nil) | |||
| 300 | + | reader.chunks <- []byte("ls") | |||
| 301 | + | require.Eventually(t, func() bool { return first.heldLen() == 2 }, 5*time.Second, time.Millisecond) | |||
| 302 | + | first.hold(bytes.Repeat([]byte("x"), resumeHeldInputLimit)) | |||
| 303 | + | require.True(t, lifetime.resumeOverflowed.Load()) | |||
| 304 | + | first.release() | |||
| 305 | + | ||||
| 306 | + | second := lifetime.watchResume(clock, nil) | |||
| 307 | + | require.Eventually(t, func() bool { return second.heldLen() == 2 }, 5*time.Second, time.Millisecond) | |||
| 308 | + | second.release() | |||
| 309 | + | ||||
| 310 | + | pump.mu.Lock() | |||
| 311 | + | defer pump.mu.Unlock() | |||
| 312 | + | require.Equal(t, "ls", string(pump.residual)) | |||
| 313 | + | } | |||
| 314 | + | ||||
| 285 | // stallingSessionStream passes writes through until stall is set; stalled | 315 | // stallingSessionStream passes writes through until stall is set; stalled | ||
| 286 | // writes block until release closes, then fail with failErr when set. | 316 | // writes block until release closes, then fail with failErr when set. | ||
| 287 | type stallingSessionStream struct { | 317 | type stallingSessionStream struct { | ||
M internal/usecase/client/predictive_echo.go +1 -1
| @@ -619,7 +619,7 @@ func (p *echoPredictor) needsTick() bool { | |||
| 619 | 619 | // scrolls would otherwise move a guess where nothing ever repairs it. The | |
| 620 | 620 | // guesses stay pending and render draws them again after the frame. | |
| 621 | 621 | func (p *echoPredictor) undraw() []byte { | |
| 622 | - | if len(p.drawn) == 0 { | |
| 622 | + | if len(p.drawn) == 0 && !p.cursorMove { | |
| 623 | 623 | return nil | |
| 624 | 624 | } | |
| 625 | 625 | var out bytes.Buffer | |
| @@ -619,7 +619,7 @@ func (p *echoPredictor) needsTick() bool { | |||||
| 619 | // scrolls would otherwise move a guess where nothing ever repairs it. The | 619 | // scrolls would otherwise move a guess where nothing ever repairs it. The | ||
| 620 | // guesses stay pending and render draws them again after the frame. | 620 | // guesses stay pending and render draws them again after the frame. | ||
| 621 | func (p *echoPredictor) undraw() []byte { | 621 | func (p *echoPredictor) undraw() []byte { | ||
| 622 | - | if len(p.drawn) == 0 { | 622 | + | if len(p.drawn) == 0 && !p.cursorMove { |
| 623 | return nil | 623 | return nil | ||
| 624 | } | 624 | } | ||
| 625 | var out bytes.Buffer | 625 | var out bytes.Buffer | ||
M internal/usecase/client/predictive_echo_test.go +22 -10
| @@ -141,17 +141,29 @@ func TestEchoPredictorEnterKeepsDrawnGuessesUntilAFrame(t *testing.T) { | |||
| 141 | 141 | ||
| 142 | 142 | func TestEchoPredictorUndrawRestoresMirror(t *testing.T) { | |
| 143 | 143 | t.Parallel() | |
| 144 | - | h := newEchoHarness(t, domain.EchoPredictAlways) | |
| 145 | - | h.warm(100 * time.Millisecond) | |
| 146 | - | h.confirm() | |
| 147 | - | if got := h.typed("b"); !bytes.Contains(got, []byte("b")) { | |
| 148 | - | t.Fatalf("render = %q, want the guess drawn", got) | |
| 149 | - | } | |
| 150 | - | if got, want := string(h.p.undraw()), "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"; got != want { | |
| 151 | - | t.Fatalf("undraw = %q, want %q", got, want) | |
| 144 | + | tests := []struct { | |
| 145 | + | name, typed, want string | |
| 146 | + | }{ | |
| 147 | + | {name: "drawn guess", typed: "b", want: "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"}, | |
| 148 | + | // A blank guess matches the mirror cell, so only the cursor moved. | |
| 149 | + | {name: "cursor-only guess", typed: " ", want: "\x1b[0m\x1b[1;4H"}, | |
| 152 | 150 | } | |
| 153 | - | if got := h.p.undraw(); len(got) != 0 { | |
| 154 | - | t.Fatalf("second undraw = %q, want nothing", got) | |
| 151 | + | for _, tt := range tests { | |
| 152 | + | t.Run(tt.name, func(t *testing.T) { | |
| 153 | + | t.Parallel() | |
| 154 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |
| 155 | + | h.warm(100 * time.Millisecond) | |
| 156 | + | h.confirm() | |
| 157 | + | if got := h.typed(tt.typed); len(got) == 0 { | |
| 158 | + | t.Fatal("guess rendered nothing") | |
| 159 | + | } | |
| 160 | + | if got := string(h.p.undraw()); got != tt.want { | |
| 161 | + | t.Fatalf("undraw = %q, want %q", got, tt.want) | |
| 162 | + | } | |
| 163 | + | if got := h.p.undraw(); len(got) != 0 { | |
| 164 | + | t.Fatalf("second undraw = %q, want nothing", got) | |
| 165 | + | } | |
| 166 | + | }) | |
| 155 | 167 | } | |
| 156 | 168 | } | |
| 157 | 169 | ||
| @@ -141,17 +141,29 @@ func TestEchoPredictorEnterKeepsDrawnGuessesUntilAFrame(t *testing.T) { | |||||
| 141 | 141 | ||||
| 142 | func TestEchoPredictorUndrawRestoresMirror(t *testing.T) { | 142 | func TestEchoPredictorUndrawRestoresMirror(t *testing.T) { | ||
| 143 | t.Parallel() | 143 | t.Parallel() | ||
| 144 | - | h := newEchoHarness(t, domain.EchoPredictAlways) | 144 | + | tests := []struct { |
| 145 | - | h.warm(100 * time.Millisecond) | 145 | + | name, typed, want string |
| 146 | - | h.confirm() | 146 | + | }{ |
| 147 | - | if got := h.typed("b"); !bytes.Contains(got, []byte("b")) { | 147 | + | {name: "drawn guess", typed: "b", want: "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"}, |
| 148 | - | t.Fatalf("render = %q, want the guess drawn", got) | 148 | + | // A blank guess matches the mirror cell, so only the cursor moved. |
| 149 | - | } | 149 | + | {name: "cursor-only guess", typed: " ", want: "\x1b[0m\x1b[1;4H"}, |
| 150 | - | if got, want := string(h.p.undraw()), "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"; got != want { | |||
| 151 | - | t.Fatalf("undraw = %q, want %q", got, want) | |||
| 152 | } | 150 | } | ||
| 153 | - | if got := h.p.undraw(); len(got) != 0 { | 151 | + | for _, tt := range tests { |
| 154 | - | t.Fatalf("second undraw = %q, want nothing", got) | 152 | + | t.Run(tt.name, func(t *testing.T) { |
| 153 | + | t.Parallel() | |||
| 154 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |||
| 155 | + | h.warm(100 * time.Millisecond) | |||
| 156 | + | h.confirm() | |||
| 157 | + | if got := h.typed(tt.typed); len(got) == 0 { | |||
| 158 | + | t.Fatal("guess rendered nothing") | |||
| 159 | + | } | |||
| 160 | + | if got := string(h.p.undraw()); got != tt.want { | |||
| 161 | + | t.Fatalf("undraw = %q, want %q", got, tt.want) | |||
| 162 | + | } | |||
| 163 | + | if got := h.p.undraw(); len(got) != 0 { | |||
| 164 | + | t.Fatalf("second undraw = %q, want nothing", got) | |||
| 165 | + | } | |||
| 166 | + | }) | |||
| 155 | } | 167 | } | ||
| 156 | } | 168 | } | ||
| 157 | 169 | ||||
M internal/usecase/client/supervisor.go +30 -10
| @@ -895,8 +895,8 @@ type readyOutcome struct { | |||
| 895 | 895 | // the supervisor can admit exactly the row the user committed. It opens no | |
| 896 | 896 | // stream and starts no terminal work itself. | |
| 897 | 897 | func (s *Supervisor) awaitReady(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, sub ports.BrokerSubscription) readyOutcome { | |
| 898 | - | if key := s.picker.pendingPickerKey; key != "" { | |
| 899 | - | s.picker.pendingPickerKey = "" | |
| 898 | + | if key := s.picker.pendingKey; key != "" { | |
| 899 | + | s.picker.pendingKey = "" | |
| 900 | 900 | return readyOutcome{commitKey: key} | |
| 901 | 901 | } | |
| 902 | 902 | var changed <-chan struct{} | |
| @@ -991,7 +991,7 @@ func (s *Supervisor) pickerOps() <-chan struct{} { | |||
| 991 | 991 | func (s *Supervisor) takePickerClose() bool { | |
| 992 | 992 | op, key := s.cfg.Picker.TakeOp() | |
| 993 | 993 | if op.commit && key != "" { | |
| 994 | - | s.picker.pendingPickerKey = key | |
| 994 | + | s.picker.pendingKey = key | |
| 995 | 995 | } | |
| 996 | 996 | if op.close && !op.exit { | |
| 997 | 997 | s.notifyPicker(pickerExitHint) | |
| @@ -1515,9 +1515,10 @@ type resumeWatch struct { | |||
| 1515 | 1515 | clock ports.Clock | |
| 1516 | 1516 | logger *slog.Logger | |
| 1517 | 1517 | ||
| 1518 | - | // backlog counts deliveries already waiting at claim: session input the | |
| 1519 | - | // lost attachment kept, held verbatim and never read as a cancel key. | |
| 1520 | - | backlog int | |
| 1518 | + | // backlog marks deliveries already waiting at claim: session input the | |
| 1519 | + | // lost attachment or an earlier phase kept, held verbatim and never read | |
| 1520 | + | // as a cancel key. | |
| 1521 | + | backlog claimBacklog | |
| 1521 | 1522 | releaseOnce sync.Once | |
| 1522 | 1523 | ||
| 1523 | 1524 | mu sync.Mutex | |
| @@ -1531,12 +1532,24 @@ func (w *resumeWatch) heldLen() int { | |||
| 1531 | 1532 | return len(w.held) | |
| 1532 | 1533 | } | |
| 1533 | 1534 | ||
| 1534 | - | // hold keeps data within the bound. | |
| 1535 | + | // hold keeps newly typed data within the bound. After an overflow in this | |
| 1536 | + | // outage it drops every new key, so the session never sees a gap. | |
| 1535 | 1537 | func (w *resumeWatch) hold(data []byte) { | |
| 1538 | + | w.holdInput(data, false) | |
| 1539 | + | } | |
| 1540 | + | ||
| 1541 | + | // holdResidual keeps the residual an earlier owner handed back. It was kept | |
| 1542 | + | // before any overflow, so it precedes the dropped keys and the sticky | |
| 1543 | + | // overflow must not discard it. | |
| 1544 | + | func (w *resumeWatch) holdResidual(data []byte) { | |
| 1545 | + | w.holdInput(data, true) | |
| 1546 | + | } | |
| 1547 | + | ||
| 1548 | + | func (w *resumeWatch) holdInput(data []byte, residual bool) { | |
| 1536 | 1549 | w.mu.Lock() | |
| 1537 | 1550 | defer w.mu.Unlock() | |
| 1538 | 1551 | overflowed := &w.lifetime.resumeOverflowed | |
| 1539 | - | if overflowed.Load() || len(w.held)+len(data) > resumeHeldInputLimit { | |
| 1552 | + | if (!residual && overflowed.Load()) || len(w.held)+len(data) > resumeHeldInputLimit { | |
| 1540 | 1553 | if len(data) != 0 { | |
| 1541 | 1554 | overflowed.Store(true) | |
| 1542 | 1555 | } | |
| @@ -1615,8 +1628,15 @@ func (w *resumeWatch) run() { | |||
| 1615 | 1628 | return | |
| 1616 | 1629 | } | |
| 1617 | 1630 | pump.ack(w.id) | |
| 1618 | - | if w.backlog > 0 { | |
| 1619 | - | w.backlog-- | |
| 1631 | + | if w.backlog.residual { | |
| 1632 | + | w.backlog.residual = false | |
| 1633 | + | w.holdResidual(result.data) | |
| 1634 | + | continue | |
| 1635 | + | } | |
| 1636 | + | if w.backlog.pending { | |
| 1637 | + | // Read after the earlier owner stopped: newly typed, so an | |
| 1638 | + | // overflow drops it, but it is still never a cancel key. | |
| 1639 | + | w.backlog.pending = false | |
| 1620 | 1640 | w.hold(result.data) | |
| 1621 | 1641 | continue | |
| 1622 | 1642 | } | |
| @@ -895,8 +895,8 @@ type readyOutcome struct { | |||||
| 895 | // the supervisor can admit exactly the row the user committed. It opens no | 895 | // the supervisor can admit exactly the row the user committed. It opens no | ||
| 896 | // stream and starts no terminal work itself. | 896 | // stream and starts no terminal work itself. | ||
| 897 | func (s *Supervisor) awaitReady(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, sub ports.BrokerSubscription) readyOutcome { | 897 | func (s *Supervisor) awaitReady(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, sub ports.BrokerSubscription) readyOutcome { | ||
| 898 | - | if key := s.picker.pendingPickerKey; key != "" { | 898 | + | if key := s.picker.pendingKey; key != "" { |
| 899 | - | s.picker.pendingPickerKey = "" | 899 | + | s.picker.pendingKey = "" |
| 900 | return readyOutcome{commitKey: key} | 900 | return readyOutcome{commitKey: key} | ||
| 901 | } | 901 | } | ||
| 902 | var changed <-chan struct{} | 902 | var changed <-chan struct{} | ||
| @@ -991,7 +991,7 @@ func (s *Supervisor) pickerOps() <-chan struct{} { | |||||
| 991 | func (s *Supervisor) takePickerClose() bool { | 991 | func (s *Supervisor) takePickerClose() bool { | ||
| 992 | op, key := s.cfg.Picker.TakeOp() | 992 | op, key := s.cfg.Picker.TakeOp() | ||
| 993 | if op.commit && key != "" { | 993 | if op.commit && key != "" { | ||
| 994 | - | s.picker.pendingPickerKey = key | 994 | + | s.picker.pendingKey = key |
| 995 | } | 995 | } | ||
| 996 | if op.close && !op.exit { | 996 | if op.close && !op.exit { | ||
| 997 | s.notifyPicker(pickerExitHint) | 997 | s.notifyPicker(pickerExitHint) | ||
| @@ -1515,9 +1515,10 @@ type resumeWatch struct { | |||||
| 1515 | clock ports.Clock | 1515 | clock ports.Clock | ||
| 1516 | logger *slog.Logger | 1516 | logger *slog.Logger | ||
| 1517 | 1517 | ||||
| 1518 | - | // backlog counts deliveries already waiting at claim: session input the | 1518 | + | // backlog marks deliveries already waiting at claim: session input the |
| 1519 | - | // lost attachment kept, held verbatim and never read as a cancel key. | 1519 | + | // lost attachment or an earlier phase kept, held verbatim and never read |
| 1520 | - | backlog int | 1520 | + | // as a cancel key. |
| 1521 | + | backlog claimBacklog | |||
| 1521 | releaseOnce sync.Once | 1522 | releaseOnce sync.Once | ||
| 1522 | 1523 | ||||
| 1523 | mu sync.Mutex | 1524 | mu sync.Mutex | ||
| @@ -1531,12 +1532,24 @@ func (w *resumeWatch) heldLen() int { | |||||
| 1531 | return len(w.held) | 1532 | return len(w.held) | ||
| 1532 | } | 1533 | } | ||
| 1533 | 1534 | ||||
| 1534 | - | // hold keeps data within the bound. | 1535 | + | // hold keeps newly typed data within the bound. After an overflow in this |
| 1536 | + | // outage it drops every new key, so the session never sees a gap. | |||
| 1535 | func (w *resumeWatch) hold(data []byte) { | 1537 | func (w *resumeWatch) hold(data []byte) { | ||
| 1538 | + | w.holdInput(data, false) | |||
| 1539 | + | } | |||
| 1540 | + | ||||
| 1541 | + | // holdResidual keeps the residual an earlier owner handed back. It was kept | |||
| 1542 | + | // before any overflow, so it precedes the dropped keys and the sticky | |||
| 1543 | + | // overflow must not discard it. | |||
| 1544 | + | func (w *resumeWatch) holdResidual(data []byte) { | |||
| 1545 | + | w.holdInput(data, true) | |||
| 1546 | + | } | |||
| 1547 | + | ||||
| 1548 | + | func (w *resumeWatch) holdInput(data []byte, residual bool) { | |||
| 1536 | w.mu.Lock() | 1549 | w.mu.Lock() | ||
| 1537 | defer w.mu.Unlock() | 1550 | defer w.mu.Unlock() | ||
| 1538 | overflowed := &w.lifetime.resumeOverflowed | 1551 | overflowed := &w.lifetime.resumeOverflowed | ||
| 1539 | - | if overflowed.Load() || len(w.held)+len(data) > resumeHeldInputLimit { | 1552 | + | if (!residual && overflowed.Load()) || len(w.held)+len(data) > resumeHeldInputLimit { |
| 1540 | if len(data) != 0 { | 1553 | if len(data) != 0 { | ||
| 1541 | overflowed.Store(true) | 1554 | overflowed.Store(true) | ||
| 1542 | } | 1555 | } | ||
| @@ -1615,8 +1628,15 @@ func (w *resumeWatch) run() { | |||||
| 1615 | return | 1628 | return | ||
| 1616 | } | 1629 | } | ||
| 1617 | pump.ack(w.id) | 1630 | pump.ack(w.id) | ||
| 1618 | - | if w.backlog > 0 { | 1631 | + | if w.backlog.residual { |
| 1619 | - | w.backlog-- | 1632 | + | w.backlog.residual = false |
| 1633 | + | w.holdResidual(result.data) | |||
| 1634 | + | continue | |||
| 1635 | + | } | |||
| 1636 | + | if w.backlog.pending { | |||
| 1637 | + | // Read after the earlier owner stopped: newly typed, so an | |||
| 1638 | + | // overflow drops it, but it is still never a cancel key. | |||
| 1639 | + | w.backlog.pending = false | |||
| 1620 | w.hold(result.data) | 1640 | w.hold(result.data) | ||
| 1621 | continue | 1641 | continue | ||
| 1622 | } | 1642 | } | ||
M internal/usecase/client/supervisor_attach.go +1 -1
| @@ -551,7 +551,7 @@ func (s *Supervisor) runInitialNavigation(ctx context.Context, input *terminalIn | |||
| 551 | 551 | if s.cfg.Picker != nil { | |
| 552 | 552 | s.cfg.Picker.TakeOp() | |
| 553 | 553 | } | |
| 554 | - | s.picker.pendingPickerKey = "" | |
| 554 | + | s.picker.pendingKey = "" | |
| 555 | 555 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | |
| 556 | 556 | } | |
| 557 | 557 | ||
| @@ -551,7 +551,7 @@ func (s *Supervisor) runInitialNavigation(ctx context.Context, input *terminalIn | |||||
| 551 | if s.cfg.Picker != nil { | 551 | if s.cfg.Picker != nil { | ||
| 552 | s.cfg.Picker.TakeOp() | 552 | s.cfg.Picker.TakeOp() | ||
| 553 | } | 553 | } | ||
| 554 | - | s.picker.pendingPickerKey = "" | 554 | + | s.picker.pendingKey = "" |
| 555 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | 555 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | ||
| 556 | } | 556 | } | ||
| 557 | 557 | ||||
M internal/usecase/client/supervisor_picker_session.go +3 -3
| @@ -7,10 +7,10 @@ package client | |||
| 7 | 7 | // goroutine's kills.wg/kills.done handoff. Construct with preview.clock set; | |
| 8 | 8 | // other fields are ready at zero. | |
| 9 | 9 | type pickerSession struct { | |
| 10 | - | // pendingPickerKey retains a commit observed while connecting. It is | |
| 10 | + | // pendingKey retains a commit observed while connecting. It is | |
| 11 | 11 | // revalidated against the adopted service just like a ready-phase commit. | |
| 12 | - | pendingPickerKey string | |
| 13 | - | preview previewManager | |
| 12 | + | pendingKey string | |
| 13 | + | preview previewManager | |
| 14 | 14 | // kills runs the picker's `x` operations off the run goroutine. | |
| 15 | 15 | kills pickerKills | |
| 16 | 16 | } | |
| @@ -7,10 +7,10 @@ package client | |||||
| 7 | // goroutine's kills.wg/kills.done handoff. Construct with preview.clock set; | 7 | // goroutine's kills.wg/kills.done handoff. Construct with preview.clock set; | ||
| 8 | // other fields are ready at zero. | 8 | // other fields are ready at zero. | ||
| 9 | type pickerSession struct { | 9 | type pickerSession struct { | ||
| 10 | - | // pendingPickerKey retains a commit observed while connecting. It is | 10 | + | // pendingKey retains a commit observed while connecting. It is |
| 11 | // revalidated against the adopted service just like a ready-phase commit. | 11 | // revalidated against the adopted service just like a ready-phase commit. | ||
| 12 | - | pendingPickerKey string | 12 | + | pendingKey string |
| 13 | - | preview previewManager | 13 | + | preview previewManager |
| 14 | // kills runs the picker's `x` operations off the run goroutine. | 14 | // kills runs the picker's `x` operations off the run goroutine. | ||
| 15 | kills pickerKills | 15 | kills pickerKills | ||
| 16 | } | 16 | } | ||
M internal/usecase/client/terminal_input_pump.go +12 -12
| @@ -109,22 +109,22 @@ func (p *terminalInputPump) tryClaim() (uint64, bool) { | |||
| 109 | 109 | return consumer, ok | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | - | // claimWithBacklog claims the pump and reports, atomically with the claim, how | |
| 113 | - | // many deliveries were already waiting: the residual, then the pending read. | |
| 114 | - | // They were read for an earlier owner, not typed for this one. | |
| 115 | - | func (p *terminalInputPump) claimWithBacklog() (uint64, int, bool) { | |
| 112 | + | // claimBacklog describes deliveries already waiting at claim. They are taken | |
| 113 | + | // in this order: the residual first, then the pending read. | |
| 114 | + | type claimBacklog struct { | |
| 115 | + | residual, pending bool | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | // claimWithBacklog claims the pump and reports, atomically with the claim, | |
| 119 | + | // which deliveries were already waiting. They were read for an earlier owner, | |
| 120 | + | // not typed for this one. | |
| 121 | + | func (p *terminalInputPump) claimWithBacklog() (uint64, claimBacklog, bool) { | |
| 116 | 122 | p.mu.Lock() | |
| 117 | 123 | if p.consumer != 0 { | |
| 118 | 124 | p.mu.Unlock() | |
| 119 | - | return 0, 0, false | |
| 120 | - | } | |
| 121 | - | backlog := 0 | |
| 122 | - | if len(p.residual) != 0 { | |
| 123 | - | backlog++ | |
| 124 | - | } | |
| 125 | - | if p.pending != nil { | |
| 126 | - | backlog++ | |
| 125 | + | return 0, claimBacklog{}, false | |
| 127 | 126 | } | |
| 127 | + | backlog := claimBacklog{residual: len(p.residual) != 0, pending: p.pending != nil} | |
| 128 | 128 | p.nextID++ | |
| 129 | 129 | p.consumer = p.nextID | |
| 130 | 130 | consumer := p.consumer | |
| @@ -109,22 +109,22 @@ func (p *terminalInputPump) tryClaim() (uint64, bool) { | |||||
| 109 | return consumer, ok | 109 | return consumer, ok | ||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | - | // claimWithBacklog claims the pump and reports, atomically with the claim, how | 112 | + | // claimBacklog describes deliveries already waiting at claim. They are taken |
| 113 | - | // many deliveries were already waiting: the residual, then the pending read. | 113 | + | // in this order: the residual first, then the pending read. |
| 114 | - | // They were read for an earlier owner, not typed for this one. | 114 | + | type claimBacklog struct { |
| 115 | - | func (p *terminalInputPump) claimWithBacklog() (uint64, int, bool) { | 115 | + | residual, pending bool |
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | // claimWithBacklog claims the pump and reports, atomically with the claim, | |||
| 119 | + | // which deliveries were already waiting. They were read for an earlier owner, | |||
| 120 | + | // not typed for this one. | |||
| 121 | + | func (p *terminalInputPump) claimWithBacklog() (uint64, claimBacklog, bool) { | |||
| 116 | p.mu.Lock() | 122 | p.mu.Lock() | ||
| 117 | if p.consumer != 0 { | 123 | if p.consumer != 0 { | ||
| 118 | p.mu.Unlock() | 124 | p.mu.Unlock() | ||
| 119 | - | return 0, 0, false | 125 | + | return 0, claimBacklog{}, false |
| 120 | - | } | |||
| 121 | - | backlog := 0 | |||
| 122 | - | if len(p.residual) != 0 { | |||
| 123 | - | backlog++ | |||
| 124 | - | } | |||
| 125 | - | if p.pending != nil { | |||
| 126 | - | backlog++ | |||
| 127 | } | 126 | } | ||
| 127 | + | backlog := claimBacklog{residual: len(p.residual) != 0, pending: p.pending != nil} | |||
| 128 | p.nextID++ | 128 | p.nextID++ | ||
| 129 | p.consumer = p.nextID | 129 | p.consumer = p.nextID | ||
| 130 | consumer := p.consumer | 130 | consumer := p.consumer | ||
M internal/usecase/daemon/bar_refresh_test.go +2 -2
| @@ -96,10 +96,10 @@ func TestBarScriptContextUsesFirstOrderedPaneAndClientCols(t *testing.T) { | |||
| 96 | 96 | active := sess.tabs[0].focusedPane() | |
| 97 | 97 | active.stableID = "pane-default" | |
| 98 | 98 | active.pty = newScriptPTY(nil) | |
| 99 | - | WithCwdReader(func(pid int) (string, error) { | |
| 99 | + | WithProcessInspector(&processInspectorFake{cwd: func(pid int) (string, error) { | |
| 100 | 100 | require.Equal(t, 4242, pid) | |
| 101 | 101 | return "/pane-repo", nil | |
| 102 | - | })(d) | |
| 102 | + | }})(d) | |
| 103 | 103 | ||
| 104 | 104 | require.True(t, d.refreshBarScriptsIfDue(sess, time.Unix(0, 0), true)) | |
| 105 | 105 | waitBarRefreshIdle(t, d) | |
| @@ -96,10 +96,10 @@ func TestBarScriptContextUsesFirstOrderedPaneAndClientCols(t *testing.T) { | |||||
| 96 | active := sess.tabs[0].focusedPane() | 96 | active := sess.tabs[0].focusedPane() | ||
| 97 | active.stableID = "pane-default" | 97 | active.stableID = "pane-default" | ||
| 98 | active.pty = newScriptPTY(nil) | 98 | active.pty = newScriptPTY(nil) | ||
| 99 | - | WithCwdReader(func(pid int) (string, error) { | 99 | + | WithProcessInspector(&processInspectorFake{cwd: func(pid int) (string, error) { |
| 100 | require.Equal(t, 4242, pid) | 100 | require.Equal(t, 4242, pid) | ||
| 101 | return "/pane-repo", nil | 101 | return "/pane-repo", nil | ||
| 102 | - | })(d) | 102 | + | }})(d) |
| 103 | 103 | ||||
| 104 | require.True(t, d.refreshBarScriptsIfDue(sess, time.Unix(0, 0), true)) | 104 | require.True(t, d.refreshBarScriptsIfDue(sess, time.Unix(0, 0), true)) | ||
| 105 | waitBarRefreshIdle(t, d) | 105 | waitBarRefreshIdle(t, d) | ||
M internal/usecase/daemon/client.go +1 -0
| @@ -238,6 +238,7 @@ func (ac *attachedClient) geometrySnapshot() domain.Geometry { | |||
| 238 | 238 | if ac.geometry.Valid() { | |
| 239 | 239 | return ac.geometry | |
| 240 | 240 | } | |
| 241 | + | // Pixels only describe a valid cell size; never hand them out without one. | |
| 241 | 242 | return domain.Geometry{Size: ac.geometry.Size} | |
| 242 | 243 | } | |
| 243 | 244 | ||
| @@ -238,6 +238,7 @@ func (ac *attachedClient) geometrySnapshot() domain.Geometry { | |||||
| 238 | if ac.geometry.Valid() { | 238 | if ac.geometry.Valid() { | ||
| 239 | return ac.geometry | 239 | return ac.geometry | ||
| 240 | } | 240 | } | ||
| 241 | + | // Pixels only describe a valid cell size; never hand them out without one. | |||
| 241 | return domain.Geometry{Size: ac.geometry.Size} | 242 | return domain.Geometry{Size: ac.geometry.Size} | ||
| 242 | } | 243 | } | ||
| 243 | 244 | ||||
M internal/usecase/daemon/config_test.go +34 -82
| @@ -1,7 +1,6 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "sync" | |
| 5 | 4 | "testing" | |
| 6 | 5 | ||
| 7 | 6 | "github.com/stretchr/testify/require" | |
| @@ -172,95 +171,48 @@ func updateRuntimeConfig(d *Daemon, mutate func(*runtimeConfig)) { | |||
| 172 | 171 | d.config.Store(&next) | |
| 173 | 172 | } | |
| 174 | 173 | ||
| 175 | - | func storeThemeForTest(d *Daemon, cfg domain.Config) { | |
| 176 | - | updateRuntimeConfig(d, func(rc *runtimeConfig) { | |
| 177 | - | rc.theme = themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette, accent: cfg.ThemeAccent} | |
| 178 | - | }) | |
| 179 | - | } | |
| 180 | - | ||
| 181 | - | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | |
| 182 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 183 | - | ||
| 184 | - | // A nil atomic pointer is the zero/default snapshot: auto theme with | |
| 185 | - | // terminal palette inheritance enabled. | |
| 186 | - | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | |
| 174 | + | // TestApplyConfigPublishesOneSnapshot checks ApplyConfig publishes every | |
| 175 | + | // section of one config in a single snapshot, and that the snapshot in force | |
| 176 | + | // before any reload equals the one built from vev's defaults. | |
| 177 | + | func TestApplyConfigPublishesOneSnapshot(t *testing.T) { | |
| 178 | + | custom := domain.Defaults() | |
| 179 | + | custom.Theme = domain.ThemeDark | |
| 180 | + | custom.ThemePalette = false | |
| 181 | + | custom.Codes = map[string]string{"new-tab": "nt"} | |
| 182 | + | custom.Snapshot = domain.SnapshotConfig{RestoreProcesses: []string{"htop"}, RestoreProcessesSet: true} | |
| 183 | + | custom.Copy.WordSeparators = "-" | |
| 184 | + | custom.Palette = domain.PaletteConfig{Anchor: custom.Palette.Anchor, AnchorSet: true} | |
| 185 | + | custom.Floating.Width++ | |
| 186 | + | custom.Nav.OverflowTabs = !custom.Nav.OverflowTabs | |
| 187 | + | custom.Tabs.TerminalTitle = !custom.Tabs.TerminalTitle | |
| 188 | + | custom.Ephemeral.CloseOnExit = !custom.Ephemeral.CloseOnExit | |
| 189 | + | custom.Scrollback = domain.ScrollbackConfig{Megabytes: 3, Lines: 300} | |
| 187 | 190 | ||
| 188 | 191 | for _, tt := range []struct { | |
| 189 | 192 | name string | |
| 190 | - | mode domain.ThemeMode | |
| 191 | - | gate bool | |
| 193 | + | cfg domain.Config | |
| 194 | + | want *runtimeConfig | |
| 192 | 195 | }{ | |
| 193 | - | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | |
| 194 | - | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | |
| 195 | - | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | |
| 196 | - | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | |
| 197 | - | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | |
| 198 | - | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | |
| 196 | + | {name: "defaults match the pre-reload snapshot", cfg: domain.Defaults(), want: defaultRuntimeConfig()}, | |
| 197 | + | {name: "every section comes from one config", cfg: custom, want: &runtimeConfig{ | |
| 198 | + | codeOverrides: map[string]string{"new-tab": "NT"}, | |
| 199 | + | restoreProcessAllowlist: map[string]struct{}{"htop": {}}, | |
| 200 | + | floating: custom.Floating, | |
| 201 | + | copy: custom.Copy, | |
| 202 | + | palette: custom.Palette, | |
| 203 | + | nav: custom.Nav, | |
| 204 | + | tabs: custom.Tabs, | |
| 205 | + | ephemeral: custom.Ephemeral, | |
| 206 | + | scrollback: custom.Scrollback, | |
| 207 | + | theme: themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: true, accent: custom.ThemeAccent}, | |
| 208 | + | }}, | |
| 199 | 209 | } { | |
| 200 | 210 | t.Run(tt.name, func(t *testing.T) { | |
| 201 | - | storeThemeForTest(d, domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | |
| 202 | - | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | |
| 211 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 212 | + | d.ApplyConfig(tt.cfg) | |
| 213 | + | require.Equal(t, tt.want, d.config.Load()) | |
| 203 | 214 | }) | |
| 204 | 215 | } | |
| 205 | - | ||
| 206 | - | // Publish complementary configurations concurrently. Every load must be a | |
| 207 | - | // complete published snapshot, never mode from one update and gate from | |
| 208 | - | // another. Coordination establishes concurrent readers/writers without | |
| 209 | - | // timing sleeps. | |
| 210 | - | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |
| 211 | - | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |
| 212 | - | allowed := map[themeConfigSnapshot]struct{}{ | |
| 213 | - | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |
| 214 | - | {mode: domain.ThemeDark, paletteOff: false}: {}, | |
| 215 | - | } | |
| 216 | - | firstPublished := make(chan struct{}) | |
| 217 | - | firstObserved := make(chan struct{}) | |
| 218 | - | secondPublished := make(chan struct{}) | |
| 219 | - | secondObserved := make(chan struct{}) | |
| 220 | - | observed := make(chan themeConfigSnapshot, 2) | |
| 221 | - | invalid := make(chan themeConfigSnapshot, 1) | |
| 222 | - | var wg sync.WaitGroup | |
| 223 | - | wg.Add(2) | |
| 224 | - | go func() { | |
| 225 | - | defer wg.Done() | |
| 226 | - | storeThemeForTest(d, first) | |
| 227 | - | close(firstPublished) | |
| 228 | - | <-firstObserved | |
| 229 | - | storeThemeForTest(d, second) | |
| 230 | - | close(secondPublished) | |
| 231 | - | <-secondObserved | |
| 232 | - | for range 10_000 { | |
| 233 | - | storeThemeForTest(d, second) | |
| 234 | - | storeThemeForTest(d, first) | |
| 235 | - | } | |
| 236 | - | }() | |
| 237 | - | go func() { | |
| 238 | - | defer wg.Done() | |
| 239 | - | <-firstPublished | |
| 240 | - | observed <- d.currentThemeConfig() | |
| 241 | - | close(firstObserved) | |
| 242 | - | <-secondPublished | |
| 243 | - | observed <- d.currentThemeConfig() | |
| 244 | - | close(secondObserved) | |
| 245 | - | for range 20_000 { | |
| 246 | - | snapshot := d.currentThemeConfig() | |
| 247 | - | if _, ok := allowed[snapshot]; !ok { | |
| 248 | - | select { | |
| 249 | - | case invalid <- snapshot: | |
| 250 | - | default: | |
| 251 | - | } | |
| 252 | - | return | |
| 253 | - | } | |
| 254 | - | } | |
| 255 | - | }() | |
| 256 | - | wg.Wait() | |
| 257 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeAuto, paletteOff: true}, <-observed) | |
| 258 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: false}, <-observed) | |
| 259 | - | select { | |
| 260 | - | case snapshot := <-invalid: | |
| 261 | - | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |
| 262 | - | default: | |
| 263 | - | } | |
| 264 | 216 | } | |
| 265 | 217 | ||
| 266 | 218 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | |
| @@ -1,7 +1,6 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "sync" | |||
| 5 | "testing" | 4 | "testing" | ||
| 6 | 5 | ||||
| 7 | "github.com/stretchr/testify/require" | 6 | "github.com/stretchr/testify/require" | ||
| @@ -172,95 +171,48 @@ func updateRuntimeConfig(d *Daemon, mutate func(*runtimeConfig)) { | |||||
| 172 | d.config.Store(&next) | 171 | d.config.Store(&next) | ||
| 173 | } | 172 | } | ||
| 174 | 173 | ||||
| 175 | - | func storeThemeForTest(d *Daemon, cfg domain.Config) { | 174 | + | // TestApplyConfigPublishesOneSnapshot checks ApplyConfig publishes every |
| 176 | - | updateRuntimeConfig(d, func(rc *runtimeConfig) { | 175 | + | // section of one config in a single snapshot, and that the snapshot in force |
| 177 | - | rc.theme = themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette, accent: cfg.ThemeAccent} | 176 | + | // before any reload equals the one built from vev's defaults. |
| 178 | - | }) | 177 | + | func TestApplyConfigPublishesOneSnapshot(t *testing.T) { |
| 179 | - | } | 178 | + | custom := domain.Defaults() |
| 180 | - | 179 | + | custom.Theme = domain.ThemeDark | |
| 181 | - | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | 180 | + | custom.ThemePalette = false |
| 182 | - | d := newTestDaemon(t, nil, stubClock{}) | 181 | + | custom.Codes = map[string]string{"new-tab": "nt"} |
| 183 | - | 182 | + | custom.Snapshot = domain.SnapshotConfig{RestoreProcesses: []string{"htop"}, RestoreProcessesSet: true} | |
| 184 | - | // A nil atomic pointer is the zero/default snapshot: auto theme with | 183 | + | custom.Copy.WordSeparators = "-" |
| 185 | - | // terminal palette inheritance enabled. | 184 | + | custom.Palette = domain.PaletteConfig{Anchor: custom.Palette.Anchor, AnchorSet: true} |
| 186 | - | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | 185 | + | custom.Floating.Width++ |
| 186 | + | custom.Nav.OverflowTabs = !custom.Nav.OverflowTabs | |||
| 187 | + | custom.Tabs.TerminalTitle = !custom.Tabs.TerminalTitle | |||
| 188 | + | custom.Ephemeral.CloseOnExit = !custom.Ephemeral.CloseOnExit | |||
| 189 | + | custom.Scrollback = domain.ScrollbackConfig{Megabytes: 3, Lines: 300} | |||
| 187 | 190 | ||||
| 188 | for _, tt := range []struct { | 191 | for _, tt := range []struct { | ||
| 189 | name string | 192 | name string | ||
| 190 | - | mode domain.ThemeMode | 193 | + | cfg domain.Config |
| 191 | - | gate bool | 194 | + | want *runtimeConfig |
| 192 | }{ | 195 | }{ | ||
| 193 | - | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | 196 | + | {name: "defaults match the pre-reload snapshot", cfg: domain.Defaults(), want: defaultRuntimeConfig()}, |
| 194 | - | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | 197 | + | {name: "every section comes from one config", cfg: custom, want: &runtimeConfig{ |
| 195 | - | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | 198 | + | codeOverrides: map[string]string{"new-tab": "NT"}, |
| 196 | - | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | 199 | + | restoreProcessAllowlist: map[string]struct{}{"htop": {}}, |
| 197 | - | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | 200 | + | floating: custom.Floating, |
| 198 | - | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | 201 | + | copy: custom.Copy, |
| 202 | + | palette: custom.Palette, | |||
| 203 | + | nav: custom.Nav, | |||
| 204 | + | tabs: custom.Tabs, | |||
| 205 | + | ephemeral: custom.Ephemeral, | |||
| 206 | + | scrollback: custom.Scrollback, | |||
| 207 | + | theme: themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: true, accent: custom.ThemeAccent}, | |||
| 208 | + | }}, | |||
| 199 | } { | 209 | } { | ||
| 200 | t.Run(tt.name, func(t *testing.T) { | 210 | t.Run(tt.name, func(t *testing.T) { | ||
| 201 | - | storeThemeForTest(d, domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | 211 | + | d := newTestDaemon(t, nil, stubClock{}) |
| 202 | - | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | 212 | + | d.ApplyConfig(tt.cfg) |
| 213 | + | require.Equal(t, tt.want, d.config.Load()) | |||
| 203 | }) | 214 | }) | ||
| 204 | } | 215 | } | ||
| 205 | - | ||||
| 206 | - | // Publish complementary configurations concurrently. Every load must be a | |||
| 207 | - | // complete published snapshot, never mode from one update and gate from | |||
| 208 | - | // another. Coordination establishes concurrent readers/writers without | |||
| 209 | - | // timing sleeps. | |||
| 210 | - | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |||
| 211 | - | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |||
| 212 | - | allowed := map[themeConfigSnapshot]struct{}{ | |||
| 213 | - | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |||
| 214 | - | {mode: domain.ThemeDark, paletteOff: false}: {}, | |||
| 215 | - | } | |||
| 216 | - | firstPublished := make(chan struct{}) | |||
| 217 | - | firstObserved := make(chan struct{}) | |||
| 218 | - | secondPublished := make(chan struct{}) | |||
| 219 | - | secondObserved := make(chan struct{}) | |||
| 220 | - | observed := make(chan themeConfigSnapshot, 2) | |||
| 221 | - | invalid := make(chan themeConfigSnapshot, 1) | |||
| 222 | - | var wg sync.WaitGroup | |||
| 223 | - | wg.Add(2) | |||
| 224 | - | go func() { | |||
| 225 | - | defer wg.Done() | |||
| 226 | - | storeThemeForTest(d, first) | |||
| 227 | - | close(firstPublished) | |||
| 228 | - | <-firstObserved | |||
| 229 | - | storeThemeForTest(d, second) | |||
| 230 | - | close(secondPublished) | |||
| 231 | - | <-secondObserved | |||
| 232 | - | for range 10_000 { | |||
| 233 | - | storeThemeForTest(d, second) | |||
| 234 | - | storeThemeForTest(d, first) | |||
| 235 | - | } | |||
| 236 | - | }() | |||
| 237 | - | go func() { | |||
| 238 | - | defer wg.Done() | |||
| 239 | - | <-firstPublished | |||
| 240 | - | observed <- d.currentThemeConfig() | |||
| 241 | - | close(firstObserved) | |||
| 242 | - | <-secondPublished | |||
| 243 | - | observed <- d.currentThemeConfig() | |||
| 244 | - | close(secondObserved) | |||
| 245 | - | for range 20_000 { | |||
| 246 | - | snapshot := d.currentThemeConfig() | |||
| 247 | - | if _, ok := allowed[snapshot]; !ok { | |||
| 248 | - | select { | |||
| 249 | - | case invalid <- snapshot: | |||
| 250 | - | default: | |||
| 251 | - | } | |||
| 252 | - | return | |||
| 253 | - | } | |||
| 254 | - | } | |||
| 255 | - | }() | |||
| 256 | - | wg.Wait() | |||
| 257 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeAuto, paletteOff: true}, <-observed) | |||
| 258 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: false}, <-observed) | |||
| 259 | - | select { | |||
| 260 | - | case snapshot := <-invalid: | |||
| 261 | - | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |||
| 262 | - | default: | |||
| 263 | - | } | |||
| 264 | } | 216 | } | ||
| 265 | 217 | ||||
| 266 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | 218 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | ||
M internal/usecase/daemon/copy_mouse_input.go +3 -2
| @@ -59,8 +59,9 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||
| 59 | 59 | if routed == nil { | |
| 60 | 60 | return false | |
| 61 | 61 | } | |
| 62 | + | cfg := d.currentRuntimeConfig() | |
| 62 | 63 | tb.mu.Lock() | |
| 63 | - | geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), ev.Col, ev.Row) | |
| 64 | + | geometry, ok := hitTestCopyMouseGeometryLocked(tb, cfg.floating, ev.Col, ev.Row) | |
| 64 | 65 | tb.mu.Unlock() | |
| 65 | 66 | if !ok || geometry.pane != routed { | |
| 66 | 67 | ac.overlays.copyMu.Lock() | |
| @@ -76,7 +77,7 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||
| 76 | 77 | if mouseMode != 0 || altScreen { | |
| 77 | 78 | return false // child forwarding retains its existing raw-byte path. | |
| 78 | 79 | } | |
| 79 | - | document := scopy.NewDocument(snapshot, d.currentCopyConfig().WordSeparators) | |
| 80 | + | document := scopy.NewDocument(snapshot, cfg.copy.WordSeparators) | |
| 80 | 81 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | |
| 81 | 82 | if !ok { | |
| 82 | 83 | ac.overlays.copyMu.Lock() | |
| @@ -59,8 +59,9 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||||
| 59 | if routed == nil { | 59 | if routed == nil { | ||
| 60 | return false | 60 | return false | ||
| 61 | } | 61 | } | ||
| 62 | + | cfg := d.currentRuntimeConfig() | |||
| 62 | tb.mu.Lock() | 63 | tb.mu.Lock() | ||
| 63 | - | geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), ev.Col, ev.Row) | 64 | + | geometry, ok := hitTestCopyMouseGeometryLocked(tb, cfg.floating, ev.Col, ev.Row) |
| 64 | tb.mu.Unlock() | 65 | tb.mu.Unlock() | ||
| 65 | if !ok || geometry.pane != routed { | 66 | if !ok || geometry.pane != routed { | ||
| 66 | ac.overlays.copyMu.Lock() | 67 | ac.overlays.copyMu.Lock() | ||
| @@ -76,7 +77,7 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||||
| 76 | if mouseMode != 0 || altScreen { | 77 | if mouseMode != 0 || altScreen { | ||
| 77 | return false // child forwarding retains its existing raw-byte path. | 78 | return false // child forwarding retains its existing raw-byte path. | ||
| 78 | } | 79 | } | ||
| 79 | - | document := scopy.NewDocument(snapshot, d.currentCopyConfig().WordSeparators) | 80 | + | document := scopy.NewDocument(snapshot, cfg.copy.WordSeparators) |
| 80 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | 81 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | ||
| 81 | if !ok { | 82 | if !ok { | ||
| 82 | ac.overlays.copyMu.Lock() | 83 | ac.overlays.copyMu.Lock() | ||
M internal/usecase/daemon/daemon.go +1 -12
| @@ -187,7 +187,7 @@ type Daemon struct { | |||
| 187 | 187 | restoreDone chan struct{} | |
| 188 | 188 | restoreOnce sync.Once | |
| 189 | 189 | // proc inspects pane processes (cwd, foreground command, argv). Nil means | |
| 190 | - | // no inspection is available; see WithProcessInspector and WithCwdReader. | |
| 190 | + | // no inspection is available; see WithProcessInspector. | |
| 191 | 191 | proc ports.ProcessInspector | |
| 192 | 192 | dirOrHome func(string) string | |
| 193 | 193 | // bindings is read by keys.Router through its own atomic pointer, so it is | |
| @@ -527,17 +527,6 @@ func WithSnapshotGarbageCollection() Option { | |||
| 527 | 527 | return func(d *Daemon) { d.snapshotGarbageCollection = true } | |
| 528 | 528 | } | |
| 529 | 529 | ||
| 530 | - | // WithCwdReader overrides the process cwd reader used for persistence tests. | |
| 531 | - | // Every other inspection keeps using the installed process inspector, or stays | |
| 532 | - | // unavailable when none is installed. | |
| 533 | - | func WithCwdReader(fn func(int) (string, error)) Option { | |
| 534 | - | return func(d *Daemon) { | |
| 535 | - | if fn != nil { | |
| 536 | - | d.proc = cwdOverrideInspector{base: unwrapCwdOverride(d.proc), cwd: fn} | |
| 537 | - | } | |
| 538 | - | } | |
| 539 | - | } | |
| 540 | - | ||
| 541 | 530 | // WithProcessInspector installs the platform process-inspection implementation. | |
| 542 | 531 | func WithProcessInspector(ins ports.ProcessInspector) Option { | |
| 543 | 532 | return func(d *Daemon) { | |
| @@ -187,7 +187,7 @@ type Daemon struct { | |||||
| 187 | restoreDone chan struct{} | 187 | restoreDone chan struct{} | ||
| 188 | restoreOnce sync.Once | 188 | restoreOnce sync.Once | ||
| 189 | // proc inspects pane processes (cwd, foreground command, argv). Nil means | 189 | // proc inspects pane processes (cwd, foreground command, argv). Nil means | ||
| 190 | - | // no inspection is available; see WithProcessInspector and WithCwdReader. | 190 | + | // no inspection is available; see WithProcessInspector. |
| 191 | proc ports.ProcessInspector | 191 | proc ports.ProcessInspector | ||
| 192 | dirOrHome func(string) string | 192 | dirOrHome func(string) string | ||
| 193 | // bindings is read by keys.Router through its own atomic pointer, so it is | 193 | // bindings is read by keys.Router through its own atomic pointer, so it is | ||
| @@ -527,17 +527,6 @@ func WithSnapshotGarbageCollection() Option { | |||||
| 527 | return func(d *Daemon) { d.snapshotGarbageCollection = true } | 527 | return func(d *Daemon) { d.snapshotGarbageCollection = true } | ||
| 528 | } | 528 | } | ||
| 529 | 529 | ||||
| 530 | - | // WithCwdReader overrides the process cwd reader used for persistence tests. | |||
| 531 | - | // Every other inspection keeps using the installed process inspector, or stays | |||
| 532 | - | // unavailable when none is installed. | |||
| 533 | - | func WithCwdReader(fn func(int) (string, error)) Option { | |||
| 534 | - | return func(d *Daemon) { | |||
| 535 | - | if fn != nil { | |||
| 536 | - | d.proc = cwdOverrideInspector{base: unwrapCwdOverride(d.proc), cwd: fn} | |||
| 537 | - | } | |||
| 538 | - | } | |||
| 539 | - | } | |||
| 540 | - | ||||
| 541 | // WithProcessInspector installs the platform process-inspection implementation. | 530 | // WithProcessInspector installs the platform process-inspection implementation. | ||
| 542 | func WithProcessInspector(ins ports.ProcessInspector) Option { | 531 | func WithProcessInspector(ins ports.ProcessInspector) Option { | ||
| 543 | return func(d *Daemon) { | 532 | return func(d *Daemon) { | ||
M internal/usecase/daemon/floating_spare.go +16 -3
| @@ -85,9 +85,11 @@ func (d *Daemon) ensureFloatingSpare(sess *session, tb *tab) { | |||
| 85 | 85 | // starts with the session theme, as a new tab does. | |
| 86 | 86 | d.applySessionThemeToPane(sess, p) | |
| 87 | 87 | } | |
| 88 | + | var published *pane | |
| 88 | 89 | sess.floatingLaunchMu.Lock() | |
| 89 | 90 | sess.floatingSpareStarting = false | |
| 90 | 91 | if p != nil && !sess.floatingLaunchStopping { | |
| 92 | + | published = p | |
| 91 | 93 | sess.floatingSpare = &floatingSpare{pane: p, key: spareKey(spec)} | |
| 92 | 94 | p.prestarted = true | |
| 93 | 95 | spare := p | |
| @@ -108,6 +110,11 @@ func (d *Daemon) ensureFloatingSpare(sess *session, tb *tab) { | |||
| 108 | 110 | if p != nil { | |
| 109 | 111 | closeFloatingPane(p) | |
| 110 | 112 | } | |
| 113 | + | if published != nil { | |
| 114 | + | // A theme change between the first apply and publication could | |
| 115 | + | // not see the spare; any later one does, so apply once more. | |
| 116 | + | d.applySessionThemeToPane(sess, published) | |
| 117 | + | } | |
| 111 | 118 | }) | |
| 112 | 119 | } | |
| 113 | 120 | ||
| @@ -153,10 +160,16 @@ func (d *Daemon) applySessionThemeToPane(sess *session, p *pane) { | |||
| 153 | 160 | if len(attachments) == 0 { | |
| 154 | 161 | return | |
| 155 | 162 | } | |
| 156 | - | t := d.effectiveTheme(attachments[0].getClientTheme()) | |
| 163 | + | applyClientThemeToPane(d, attachments[0], p) | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | // applyClientThemeToPane reads ac's theme while holding p.mu, so a concurrent | |
| 167 | + | // theme application either is seen here or writes the pane after it; a stale | |
| 168 | + | // theme can never overwrite a newer one. ac.themeMu is a leaf lock. | |
| 169 | + | func applyClientThemeToPane(d *Daemon, ac *attachedClient, p *pane) { | |
| 157 | 170 | p.mu.Lock() | |
| 158 | - | applyPaneThemeLocked(p, t, false) | |
| 159 | - | p.mu.Unlock() | |
| 171 | + | defer p.mu.Unlock() | |
| 172 | + | applyPaneThemeLocked(p, d.effectiveTheme(ac.getClientTheme()), false) | |
| 160 | 173 | } | |
| 161 | 174 | ||
| 162 | 175 | // floatingSparePane returns the session's unclaimed spare pane, if any. | |
| @@ -85,9 +85,11 @@ func (d *Daemon) ensureFloatingSpare(sess *session, tb *tab) { | |||||
| 85 | // starts with the session theme, as a new tab does. | 85 | // starts with the session theme, as a new tab does. | ||
| 86 | d.applySessionThemeToPane(sess, p) | 86 | d.applySessionThemeToPane(sess, p) | ||
| 87 | } | 87 | } | ||
| 88 | + | var published *pane | |||
| 88 | sess.floatingLaunchMu.Lock() | 89 | sess.floatingLaunchMu.Lock() | ||
| 89 | sess.floatingSpareStarting = false | 90 | sess.floatingSpareStarting = false | ||
| 90 | if p != nil && !sess.floatingLaunchStopping { | 91 | if p != nil && !sess.floatingLaunchStopping { | ||
| 92 | + | published = p | |||
| 91 | sess.floatingSpare = &floatingSpare{pane: p, key: spareKey(spec)} | 93 | sess.floatingSpare = &floatingSpare{pane: p, key: spareKey(spec)} | ||
| 92 | p.prestarted = true | 94 | p.prestarted = true | ||
| 93 | spare := p | 95 | spare := p | ||
| @@ -108,6 +110,11 @@ func (d *Daemon) ensureFloatingSpare(sess *session, tb *tab) { | |||||
| 108 | if p != nil { | 110 | if p != nil { | ||
| 109 | closeFloatingPane(p) | 111 | closeFloatingPane(p) | ||
| 110 | } | 112 | } | ||
| 113 | + | if published != nil { | |||
| 114 | + | // A theme change between the first apply and publication could | |||
| 115 | + | // not see the spare; any later one does, so apply once more. | |||
| 116 | + | d.applySessionThemeToPane(sess, published) | |||
| 117 | + | } | |||
| 111 | }) | 118 | }) | ||
| 112 | } | 119 | } | ||
| 113 | 120 | ||||
| @@ -153,10 +160,16 @@ func (d *Daemon) applySessionThemeToPane(sess *session, p *pane) { | |||||
| 153 | if len(attachments) == 0 { | 160 | if len(attachments) == 0 { | ||
| 154 | return | 161 | return | ||
| 155 | } | 162 | } | ||
| 156 | - | t := d.effectiveTheme(attachments[0].getClientTheme()) | 163 | + | applyClientThemeToPane(d, attachments[0], p) |
| 164 | + | } | |||
| 165 | + | ||||
| 166 | + | // applyClientThemeToPane reads ac's theme while holding p.mu, so a concurrent | |||
| 167 | + | // theme application either is seen here or writes the pane after it; a stale | |||
| 168 | + | // theme can never overwrite a newer one. ac.themeMu is a leaf lock. | |||
| 169 | + | func applyClientThemeToPane(d *Daemon, ac *attachedClient, p *pane) { | |||
| 157 | p.mu.Lock() | 170 | p.mu.Lock() | ||
| 158 | - | applyPaneThemeLocked(p, t, false) | 171 | + | defer p.mu.Unlock() |
| 159 | - | p.mu.Unlock() | 172 | + | applyPaneThemeLocked(p, d.effectiveTheme(ac.getClientTheme()), false) |
| 160 | } | 173 | } | ||
| 161 | 174 | ||||
| 162 | // floatingSparePane returns the session's unclaimed spare pane, if any. | 175 | // floatingSparePane returns the session's unclaimed spare pane, if any. | ||
M internal/usecase/daemon/floating_spare_test.go +1 -1
| @@ -124,7 +124,7 @@ func TestFloatingSpareIsSharedPerSession(t *testing.T) { | |||
| 124 | 124 | d := newTestDaemon(t, factory, stubClock{}) | |
| 125 | 125 | d.ApplyConfig(domain.Config{Floating: cfg}) | |
| 126 | 126 | cwds := map[int]string{1: "/work", 2: tt.claimCwd} | |
| 127 | - | WithCwdReader(func(pid int) (string, error) { return cwds[pid], nil })(d) | |
| 127 | + | WithProcessInspector(&processInspectorFake{cwd: func(pid int) (string, error) { return cwds[pid], nil }})(d) | |
| 128 | 128 | d.dirOrHome = func(cwd string) string { return cwd } | |
| 129 | 129 | ||
| 130 | 130 | first := newTab(&pidPTY{pid: 1}, domain.Size{Cols: 120, Rows: 40}) | |
| @@ -124,7 +124,7 @@ func TestFloatingSpareIsSharedPerSession(t *testing.T) { | |||||
| 124 | d := newTestDaemon(t, factory, stubClock{}) | 124 | d := newTestDaemon(t, factory, stubClock{}) | ||
| 125 | d.ApplyConfig(domain.Config{Floating: cfg}) | 125 | d.ApplyConfig(domain.Config{Floating: cfg}) | ||
| 126 | cwds := map[int]string{1: "/work", 2: tt.claimCwd} | 126 | cwds := map[int]string{1: "/work", 2: tt.claimCwd} | ||
| 127 | - | WithCwdReader(func(pid int) (string, error) { return cwds[pid], nil })(d) | 127 | + | WithProcessInspector(&processInspectorFake{cwd: func(pid int) (string, error) { return cwds[pid], nil }})(d) |
| 128 | d.dirOrHome = func(cwd string) string { return cwd } | 128 | d.dirOrHome = func(cwd string) string { return cwd } | ||
| 129 | 129 | ||||
| 130 | first := newTab(&pidPTY{pid: 1}, domain.Size{Cols: 120, Rows: 40}) | 130 | first := newTab(&pidPTY{pid: 1}, domain.Size{Cols: 120, Rows: 40}) | ||
M internal/usecase/daemon/floating_test.go +1 -1
| @@ -627,7 +627,7 @@ func TestFloatingLaunchUsesLiveOrValidatedSessionCwd(t *testing.T) { | |||
| 627 | 627 | return cwd | |
| 628 | 628 | } | |
| 629 | 629 | if tt.liveCwd != "" { | |
| 630 | - | WithCwdReader(func(int) (string, error) { return tt.liveCwd, nil })(d) | |
| 630 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return tt.liveCwd, nil }})(d) | |
| 631 | 631 | } | |
| 632 | 632 | tb := newFloatingTestTab(t) | |
| 633 | 633 | sess := &session{sessionCore: sessionCore{name: "work"}, cwd: tt.sessionCwd, tabs: []*tab{tb}, ctx: t.Context()} | |
| @@ -627,7 +627,7 @@ func TestFloatingLaunchUsesLiveOrValidatedSessionCwd(t *testing.T) { | |||||
| 627 | return cwd | 627 | return cwd | ||
| 628 | } | 628 | } | ||
| 629 | if tt.liveCwd != "" { | 629 | if tt.liveCwd != "" { | ||
| 630 | - | WithCwdReader(func(int) (string, error) { return tt.liveCwd, nil })(d) | 630 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return tt.liveCwd, nil }})(d) |
| 631 | } | 631 | } | ||
| 632 | tb := newFloatingTestTab(t) | 632 | tb := newFloatingTestTab(t) | ||
| 633 | sess := &session{sessionCore: sessionCore{name: "work"}, cwd: tt.sessionCwd, tabs: []*tab{tb}, ctx: t.Context()} | 633 | sess := &session{sessionCore: sessionCore{name: "work"}, cwd: tt.sessionCwd, tabs: []*tab{tb}, ctx: t.Context()} | ||
M internal/usecase/daemon/graphics_namespace_pool.go +0 -14
| @@ -70,20 +70,6 @@ func (p *graphicsNamespacePool) reserveLease(key string) (uint64, uint64) { | |||
| 70 | 70 | return 0, 0 | |
| 71 | 71 | } | |
| 72 | 72 | ||
| 73 | - | // releaseBase is the legacy base-only release. A quarantined block is never | |
| 74 | - | // released by it. | |
| 75 | - | func (p *graphicsNamespacePool) releaseBase(base uint64) { | |
| 76 | - | if base == 0 || base%graphicsIDNamespaceSize != 1 { | |
| 77 | - | return | |
| 78 | - | } | |
| 79 | - | block := (base - 1) / graphicsIDNamespaceSize | |
| 80 | - | if _, quarantined := p.quarantines[block]; quarantined { | |
| 81 | - | return | |
| 82 | - | } | |
| 83 | - | delete(p.reserved, block) | |
| 84 | - | delete(p.fences, block) | |
| 85 | - | } | |
| 86 | - | ||
| 87 | 73 | func (p *graphicsNamespacePool) releaseLease(state *graphicsOutputState) { | |
| 88 | 74 | if state == nil || state.namespaceBase == 0 || state.namespaceBase%graphicsIDNamespaceSize != 1 { | |
| 89 | 75 | return | |
| @@ -70,20 +70,6 @@ func (p *graphicsNamespacePool) reserveLease(key string) (uint64, uint64) { | |||||
| 70 | return 0, 0 | 70 | return 0, 0 | ||
| 71 | } | 71 | } | ||
| 72 | 72 | ||||
| 73 | - | // releaseBase is the legacy base-only release. A quarantined block is never | |||
| 74 | - | // released by it. | |||
| 75 | - | func (p *graphicsNamespacePool) releaseBase(base uint64) { | |||
| 76 | - | if base == 0 || base%graphicsIDNamespaceSize != 1 { | |||
| 77 | - | return | |||
| 78 | - | } | |||
| 79 | - | block := (base - 1) / graphicsIDNamespaceSize | |||
| 80 | - | if _, quarantined := p.quarantines[block]; quarantined { | |||
| 81 | - | return | |||
| 82 | - | } | |||
| 83 | - | delete(p.reserved, block) | |||
| 84 | - | delete(p.fences, block) | |||
| 85 | - | } | |||
| 86 | - | ||||
| 87 | func (p *graphicsNamespacePool) releaseLease(state *graphicsOutputState) { | 73 | func (p *graphicsNamespacePool) releaseLease(state *graphicsOutputState) { | ||
| 88 | if state == nil || state.namespaceBase == 0 || state.namespaceBase%graphicsIDNamespaceSize != 1 { | 74 | if state == nil || state.namespaceBase == 0 || state.namespaceBase%graphicsIDNamespaceSize != 1 { | ||
| 89 | return | 75 | return | ||
M internal/usecase/daemon/graphics_output.go +4 -27
| @@ -149,14 +149,10 @@ func graphicsNamespaceKey(sess *session, clientID [16]byte) string { | |||
| 149 | 149 | return fmt.Sprintf("session:%s:%x:%x", sess.id, sess.incarnation, clientID) | |
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | - | // reserveGraphicsNamespaceLocked chooses a deterministic preferred block and | |
| 153 | - | // linearly probes the bounded namespace table for a free block. Caller holds | |
| 154 | - | // d.mu; probing makes equal session/client identities collision-safe too. | |
| 155 | - | func (d *Daemon) reserveGraphicsNamespaceLocked(key string) uint64 { | |
| 156 | - | base, _ := d.reserveGraphicsNamespaceLeaseLocked(key) | |
| 157 | - | return base | |
| 158 | - | } | |
| 159 | - | ||
| 152 | + | // reserveGraphicsNamespaceLeaseLocked chooses a deterministic preferred block | |
| 153 | + | // and linearly probes the bounded namespace table for a free block, returning | |
| 154 | + | // its base and fence. Caller holds d.mu; probing makes equal session/client | |
| 155 | + | // identities collision-safe too. | |
| 160 | 156 | func (d *Daemon) reserveGraphicsNamespaceLeaseLocked(key string) (uint64, uint64) { | |
| 161 | 157 | if d == nil { | |
| 162 | 158 | return 0, 0 | |
| @@ -164,25 +160,6 @@ func (d *Daemon) reserveGraphicsNamespaceLeaseLocked(key string) (uint64, uint64 | |||
| 164 | 160 | return d.graphics.reserveLease(key) | |
| 165 | 161 | } | |
| 166 | 162 | ||
| 167 | - | func (d *Daemon) releaseGraphicsNamespace(base uint64) { | |
| 168 | - | if d == nil || base == 0 || base%graphicsIDNamespaceSize != 1 { | |
| 169 | - | return | |
| 170 | - | } | |
| 171 | - | d.mu.Lock() | |
| 172 | - | d.releaseGraphicsNamespaceLocked(base) | |
| 173 | - | d.mu.Unlock() | |
| 174 | - | } | |
| 175 | - | ||
| 176 | - | // releaseGraphicsNamespaceLocked is used by resume publication while d.mu is | |
| 177 | - | // already held. A quarantined block is intentionally not released by this | |
| 178 | - | // legacy base-only helper; only its fenced cleanup lifecycle may retire it. | |
| 179 | - | func (d *Daemon) releaseGraphicsNamespaceLocked(base uint64) { | |
| 180 | - | if d == nil { | |
| 181 | - | return | |
| 182 | - | } | |
| 183 | - | d.graphics.releaseBase(base) | |
| 184 | - | } | |
| 185 | - | ||
| 186 | 163 | func (d *Daemon) releaseGraphicsNamespaceLeaseLocked(state *graphicsOutputState) { | |
| 187 | 164 | if d == nil { | |
| 188 | 165 | return | |
| @@ -149,14 +149,10 @@ func graphicsNamespaceKey(sess *session, clientID [16]byte) string { | |||||
| 149 | return fmt.Sprintf("session:%s:%x:%x", sess.id, sess.incarnation, clientID) | 149 | return fmt.Sprintf("session:%s:%x:%x", sess.id, sess.incarnation, clientID) | ||
| 150 | } | 150 | } | ||
| 151 | 151 | ||||
| 152 | - | // reserveGraphicsNamespaceLocked chooses a deterministic preferred block and | 152 | + | // reserveGraphicsNamespaceLeaseLocked chooses a deterministic preferred block |
| 153 | - | // linearly probes the bounded namespace table for a free block. Caller holds | 153 | + | // and linearly probes the bounded namespace table for a free block, returning |
| 154 | - | // d.mu; probing makes equal session/client identities collision-safe too. | 154 | + | // its base and fence. Caller holds d.mu; probing makes equal session/client |
| 155 | - | func (d *Daemon) reserveGraphicsNamespaceLocked(key string) uint64 { | 155 | + | // identities collision-safe too. |
| 156 | - | base, _ := d.reserveGraphicsNamespaceLeaseLocked(key) | |||
| 157 | - | return base | |||
| 158 | - | } | |||
| 159 | - | ||||
| 160 | func (d *Daemon) reserveGraphicsNamespaceLeaseLocked(key string) (uint64, uint64) { | 156 | func (d *Daemon) reserveGraphicsNamespaceLeaseLocked(key string) (uint64, uint64) { | ||
| 161 | if d == nil { | 157 | if d == nil { | ||
| 162 | return 0, 0 | 158 | return 0, 0 | ||
| @@ -164,25 +160,6 @@ func (d *Daemon) reserveGraphicsNamespaceLeaseLocked(key string) (uint64, uint64 | |||||
| 164 | return d.graphics.reserveLease(key) | 160 | return d.graphics.reserveLease(key) | ||
| 165 | } | 161 | } | ||
| 166 | 162 | ||||
| 167 | - | func (d *Daemon) releaseGraphicsNamespace(base uint64) { | |||
| 168 | - | if d == nil || base == 0 || base%graphicsIDNamespaceSize != 1 { | |||
| 169 | - | return | |||
| 170 | - | } | |||
| 171 | - | d.mu.Lock() | |||
| 172 | - | d.releaseGraphicsNamespaceLocked(base) | |||
| 173 | - | d.mu.Unlock() | |||
| 174 | - | } | |||
| 175 | - | ||||
| 176 | - | // releaseGraphicsNamespaceLocked is used by resume publication while d.mu is | |||
| 177 | - | // already held. A quarantined block is intentionally not released by this | |||
| 178 | - | // legacy base-only helper; only its fenced cleanup lifecycle may retire it. | |||
| 179 | - | func (d *Daemon) releaseGraphicsNamespaceLocked(base uint64) { | |||
| 180 | - | if d == nil { | |||
| 181 | - | return | |||
| 182 | - | } | |||
| 183 | - | d.graphics.releaseBase(base) | |||
| 184 | - | } | |||
| 185 | - | ||||
| 186 | func (d *Daemon) releaseGraphicsNamespaceLeaseLocked(state *graphicsOutputState) { | 163 | func (d *Daemon) releaseGraphicsNamespaceLeaseLocked(state *graphicsOutputState) { | ||
| 187 | if d == nil { | 164 | if d == nil { | ||
| 188 | return | 165 | return | ||
M internal/usecase/daemon/graphics_output_test.go +5 -7
| @@ -353,16 +353,14 @@ func TestUnsupportedGraphicsWarningIsBoundedForLateSceneCreation(t *testing.T) { | |||
| 353 | 353 | func TestGraphicsNamespacesAreDeterministicAndCollisionSafe(t *testing.T) { | |
| 354 | 354 | d := &Daemon{} | |
| 355 | 355 | d.mu.Lock() | |
| 356 | - | first := d.reserveGraphicsNamespaceLocked("session:work:attachment") | |
| 357 | - | second := d.reserveGraphicsNamespaceLocked("session:work:attachment") | |
| 358 | - | d.mu.Unlock() | |
| 356 | + | defer d.mu.Unlock() | |
| 357 | + | first, firstFence := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") | |
| 358 | + | second, _ := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") | |
| 359 | 359 | require.NotZero(t, first) | |
| 360 | 360 | require.NotZero(t, second) | |
| 361 | 361 | require.NotEqual(t, first, second, "equal attachment/session keys still need a collision-safe namespace") | |
| 362 | - | d.releaseGraphicsNamespace(first) | |
| 363 | - | d.mu.Lock() | |
| 364 | - | reused := d.reserveGraphicsNamespaceLocked("session:work:attachment") | |
| 365 | - | d.mu.Unlock() | |
| 362 | + | d.releaseGraphicsNamespaceLeaseLocked(&graphicsOutputState{namespaceBase: first, namespaceFence: firstFence}) | |
| 363 | + | reused, _ := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") | |
| 366 | 364 | require.Equal(t, first, reused, "released namespaces should deterministically return to their preferred block") | |
| 367 | 365 | } | |
| 368 | 366 | ||
| @@ -353,16 +353,14 @@ func TestUnsupportedGraphicsWarningIsBoundedForLateSceneCreation(t *testing.T) { | |||||
| 353 | func TestGraphicsNamespacesAreDeterministicAndCollisionSafe(t *testing.T) { | 353 | func TestGraphicsNamespacesAreDeterministicAndCollisionSafe(t *testing.T) { | ||
| 354 | d := &Daemon{} | 354 | d := &Daemon{} | ||
| 355 | d.mu.Lock() | 355 | d.mu.Lock() | ||
| 356 | - | first := d.reserveGraphicsNamespaceLocked("session:work:attachment") | 356 | + | defer d.mu.Unlock() |
| 357 | - | second := d.reserveGraphicsNamespaceLocked("session:work:attachment") | 357 | + | first, firstFence := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") |
| 358 | - | d.mu.Unlock() | 358 | + | second, _ := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") |
| 359 | require.NotZero(t, first) | 359 | require.NotZero(t, first) | ||
| 360 | require.NotZero(t, second) | 360 | require.NotZero(t, second) | ||
| 361 | require.NotEqual(t, first, second, "equal attachment/session keys still need a collision-safe namespace") | 361 | require.NotEqual(t, first, second, "equal attachment/session keys still need a collision-safe namespace") | ||
| 362 | - | d.releaseGraphicsNamespace(first) | 362 | + | d.releaseGraphicsNamespaceLeaseLocked(&graphicsOutputState{namespaceBase: first, namespaceFence: firstFence}) |
| 363 | - | d.mu.Lock() | 363 | + | reused, _ := d.reserveGraphicsNamespaceLeaseLocked("session:work:attachment") |
| 364 | - | reused := d.reserveGraphicsNamespaceLocked("session:work:attachment") | |||
| 365 | - | d.mu.Unlock() | |||
| 366 | require.Equal(t, first, reused, "released namespaces should deterministically return to their preferred block") | 364 | require.Equal(t, first, reused, "released namespaces should deterministically return to their preferred block") | ||
| 367 | } | 365 | } | ||
| 368 | 366 | ||||
M internal/usecase/daemon/palette.go +22 -59
| @@ -388,21 +388,8 @@ type paletteDecision struct { | |||
| 388 | 388 | // decidePaletteInput takes paletteMu, applies data to the attachment's palette, | |
| 389 | 389 | // and returns the action to run after unlocking. | |
| 390 | 390 | func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | |
| 391 | - | var cmd command.Command | |
| 392 | - | var sessionTarget palette.Result | |
| 393 | - | var hasSessionTarget bool | |
| 394 | - | var importedSourceKey, importedEntryKey string | |
| 395 | - | var hasImportedTarget bool | |
| 396 | - | var routeTarget protocol.RouteNavigationAction | |
| 397 | - | var hasRouteTarget bool | |
| 398 | - | var createDestination palette.Result | |
| 399 | - | var hasCreateDestination bool | |
| 400 | - | var args []string | |
| 401 | - | var routeSnapshot protocol.RecentRouteSnapshot | |
| 402 | - | var generation uint64 | |
| 403 | - | var rawQuery string | |
| 404 | - | var cancelInventoryInteraction uint64 | |
| 405 | - | var hasCancelInventory bool | |
| 391 | + | // decision holds the executed selection; execute reports one was made. | |
| 392 | + | var decision paletteDecision | |
| 406 | 393 | changed, cancel, execute, chooseDestination := false, false, false, false | |
| 407 | 394 | ||
| 408 | 395 | ac.overlays.paletteMu.Lock() | |
| @@ -456,40 +443,35 @@ func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | |||
| 456 | 443 | chooseDestination = true | |
| 457 | 444 | return | |
| 458 | 445 | } | |
| 459 | - | rawQuery = ac.overlays.palette.Query() | |
| 460 | - | if selectedCommand, ok := selected.Command(); ok { | |
| 461 | - | cmd = selectedCommand | |
| 446 | + | next := paletteDecision{rawQuery: ac.overlays.palette.Query(), generation: ac.overlays.paletteGeneration} | |
| 447 | + | if cmd, ok := selected.Command(); ok { | |
| 448 | + | next.kind, next.cmd = paletteDecisionCommand, cmd | |
| 462 | 449 | if cmd.Arguments != command.ArgumentsNone { | |
| 463 | - | action, valid := palette.ParseAction([]palette.Result{selected}, rawQuery) | |
| 450 | + | action, valid := palette.ParseAction([]palette.Result{selected}, next.rawQuery) | |
| 464 | 451 | if valid { | |
| 465 | - | args = action.Args | |
| 452 | + | next.args = action.Args | |
| 466 | 453 | } else if cmd.Arguments == command.ArgumentsRequired { | |
| 467 | 454 | changed = true | |
| 468 | 455 | return | |
| 469 | 456 | } | |
| 470 | 457 | } | |
| 471 | 458 | if cmd.ContextHint == command.ContextHintRecentSessions { | |
| 472 | - | routeSnapshot = ac.overlays.paletteRouteSnapshot | |
| 473 | - | routeSnapshot.Entries = append([]protocol.RecentRouteEntry(nil), routeSnapshot.Entries...) | |
| 459 | + | next.routeSnapshot = ac.overlays.paletteRouteSnapshot | |
| 460 | + | next.routeSnapshot.Entries = append([]protocol.RecentRouteEntry(nil), next.routeSnapshot.Entries...) | |
| 474 | 461 | } | |
| 475 | 462 | } else if sourceKey, entryKey, ok := selected.ImportedSessionKey(); ok { | |
| 476 | - | importedSourceKey = sourceKey | |
| 477 | - | importedEntryKey = entryKey | |
| 478 | - | hasImportedTarget = true | |
| 463 | + | next.kind, next.importedSourceKey, next.importedEntryKey = paletteDecisionImportedSession, sourceKey, entryKey | |
| 479 | 464 | } else if action, ok := selected.RouteNavigationAction(); ok { | |
| 480 | - | routeTarget = action | |
| 481 | - | hasRouteTarget = true | |
| 465 | + | next.kind, next.routeTarget = paletteDecisionRouteNavigation, action | |
| 482 | 466 | } else if _, _, _, _, _, ok := selected.CreateSessionDestination(); ok { | |
| 483 | - | createDestination = selected | |
| 484 | - | hasCreateDestination = true | |
| 467 | + | next.kind, next.createDestination = paletteDecisionCreateDestination, selected | |
| 485 | 468 | } else if _, ok := selected.SessionTarget(); ok { | |
| 486 | - | sessionTarget = selected | |
| 487 | - | hasSessionTarget = true | |
| 469 | + | next.kind, next.sessionTarget = paletteDecisionSessionTarget, selected | |
| 488 | 470 | } else { | |
| 489 | 471 | changed = true | |
| 490 | 472 | return | |
| 491 | 473 | } | |
| 492 | - | generation = ac.overlays.paletteGeneration | |
| 474 | + | decision = next | |
| 493 | 475 | execute = true | |
| 494 | 476 | }, | |
| 495 | 477 | }) | |
| @@ -515,41 +497,22 @@ func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | |||
| 515 | 497 | } | |
| 516 | 498 | } | |
| 517 | 499 | if cancel { | |
| 518 | - | cancelInventoryInteraction, hasCancelInventory = takePaletteInventoryClose(ac.overlays) | |
| 500 | + | cancelled := paletteDecision{kind: paletteDecisionCancel} | |
| 501 | + | cancelled.cancelInventoryInteraction, cancelled.hasCancelInventory = takePaletteInventoryClose(ac.overlays) | |
| 519 | 502 | ac.clearPaletteLocked() | |
| 503 | + | ac.overlays.paletteMu.Unlock() | |
| 504 | + | return cancelled | |
| 520 | 505 | } | |
| 521 | 506 | ac.overlays.paletteMu.Unlock() | |
| 522 | 507 | ||
| 523 | 508 | switch { | |
| 524 | - | case cancel: | |
| 525 | - | return paletteDecision{kind: paletteDecisionCancel, cancelInventoryInteraction: cancelInventoryInteraction, hasCancelInventory: hasCancelInventory} | |
| 526 | - | case !execute && changed: | |
| 509 | + | case execute: | |
| 510 | + | return decision | |
| 511 | + | case changed: | |
| 527 | 512 | return paletteDecision{kind: paletteDecisionRedraw} | |
| 528 | - | case !execute: | |
| 529 | - | return paletteDecision{} | |
| 530 | - | } | |
| 531 | - | decision := paletteDecision{generation: generation, rawQuery: rawQuery} | |
| 532 | - | switch { | |
| 533 | - | case hasCreateDestination: | |
| 534 | - | decision.kind = paletteDecisionCreateDestination | |
| 535 | - | decision.createDestination = createDestination | |
| 536 | - | case hasImportedTarget: | |
| 537 | - | decision.kind = paletteDecisionImportedSession | |
| 538 | - | decision.importedSourceKey = importedSourceKey | |
| 539 | - | decision.importedEntryKey = importedEntryKey | |
| 540 | - | case hasRouteTarget: | |
| 541 | - | decision.kind = paletteDecisionRouteNavigation | |
| 542 | - | decision.routeTarget = routeTarget | |
| 543 | - | case hasSessionTarget: | |
| 544 | - | decision.kind = paletteDecisionSessionTarget | |
| 545 | - | decision.sessionTarget = sessionTarget | |
| 546 | 513 | default: | |
| 547 | - | decision.kind = paletteDecisionCommand | |
| 548 | - | decision.cmd = cmd | |
| 549 | - | decision.args = args | |
| 550 | - | decision.routeSnapshot = routeSnapshot | |
| 514 | + | return paletteDecision{} | |
| 551 | 515 | } | |
| 552 | - | return decision | |
| 553 | 516 | } | |
| 554 | 517 | ||
| 555 | 518 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ...*attachmentEffect) { | |
| @@ -388,21 +388,8 @@ type paletteDecision struct { | |||||
| 388 | // decidePaletteInput takes paletteMu, applies data to the attachment's palette, | 388 | // decidePaletteInput takes paletteMu, applies data to the attachment's palette, | ||
| 389 | // and returns the action to run after unlocking. | 389 | // and returns the action to run after unlocking. | ||
| 390 | func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | 390 | func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | ||
| 391 | - | var cmd command.Command | 391 | + | // decision holds the executed selection; execute reports one was made. |
| 392 | - | var sessionTarget palette.Result | 392 | + | var decision paletteDecision |
| 393 | - | var hasSessionTarget bool | |||
| 394 | - | var importedSourceKey, importedEntryKey string | |||
| 395 | - | var hasImportedTarget bool | |||
| 396 | - | var routeTarget protocol.RouteNavigationAction | |||
| 397 | - | var hasRouteTarget bool | |||
| 398 | - | var createDestination palette.Result | |||
| 399 | - | var hasCreateDestination bool | |||
| 400 | - | var args []string | |||
| 401 | - | var routeSnapshot protocol.RecentRouteSnapshot | |||
| 402 | - | var generation uint64 | |||
| 403 | - | var rawQuery string | |||
| 404 | - | var cancelInventoryInteraction uint64 | |||
| 405 | - | var hasCancelInventory bool | |||
| 406 | changed, cancel, execute, chooseDestination := false, false, false, false | 393 | changed, cancel, execute, chooseDestination := false, false, false, false | ||
| 407 | 394 | ||||
| 408 | ac.overlays.paletteMu.Lock() | 395 | ac.overlays.paletteMu.Lock() | ||
| @@ -456,40 +443,35 @@ func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | |||||
| 456 | chooseDestination = true | 443 | chooseDestination = true | ||
| 457 | return | 444 | return | ||
| 458 | } | 445 | } | ||
| 459 | - | rawQuery = ac.overlays.palette.Query() | 446 | + | next := paletteDecision{rawQuery: ac.overlays.palette.Query(), generation: ac.overlays.paletteGeneration} |
| 460 | - | if selectedCommand, ok := selected.Command(); ok { | 447 | + | if cmd, ok := selected.Command(); ok { |
| 461 | - | cmd = selectedCommand | 448 | + | next.kind, next.cmd = paletteDecisionCommand, cmd |
| 462 | if cmd.Arguments != command.ArgumentsNone { | 449 | if cmd.Arguments != command.ArgumentsNone { | ||
| 463 | - | action, valid := palette.ParseAction([]palette.Result{selected}, rawQuery) | 450 | + | action, valid := palette.ParseAction([]palette.Result{selected}, next.rawQuery) |
| 464 | if valid { | 451 | if valid { | ||
| 465 | - | args = action.Args | 452 | + | next.args = action.Args |
| 466 | } else if cmd.Arguments == command.ArgumentsRequired { | 453 | } else if cmd.Arguments == command.ArgumentsRequired { | ||
| 467 | changed = true | 454 | changed = true | ||
| 468 | return | 455 | return | ||
| 469 | } | 456 | } | ||
| 470 | } | 457 | } | ||
| 471 | if cmd.ContextHint == command.ContextHintRecentSessions { | 458 | if cmd.ContextHint == command.ContextHintRecentSessions { | ||
| 472 | - | routeSnapshot = ac.overlays.paletteRouteSnapshot | 459 | + | next.routeSnapshot = ac.overlays.paletteRouteSnapshot |
| 473 | - | routeSnapshot.Entries = append([]protocol.RecentRouteEntry(nil), routeSnapshot.Entries...) | 460 | + | next.routeSnapshot.Entries = append([]protocol.RecentRouteEntry(nil), next.routeSnapshot.Entries...) |
| 474 | } | 461 | } | ||
| 475 | } else if sourceKey, entryKey, ok := selected.ImportedSessionKey(); ok { | 462 | } else if sourceKey, entryKey, ok := selected.ImportedSessionKey(); ok { | ||
| 476 | - | importedSourceKey = sourceKey | 463 | + | next.kind, next.importedSourceKey, next.importedEntryKey = paletteDecisionImportedSession, sourceKey, entryKey |
| 477 | - | importedEntryKey = entryKey | |||
| 478 | - | hasImportedTarget = true | |||
| 479 | } else if action, ok := selected.RouteNavigationAction(); ok { | 464 | } else if action, ok := selected.RouteNavigationAction(); ok { | ||
| 480 | - | routeTarget = action | 465 | + | next.kind, next.routeTarget = paletteDecisionRouteNavigation, action |
| 481 | - | hasRouteTarget = true | |||
| 482 | } else if _, _, _, _, _, ok := selected.CreateSessionDestination(); ok { | 466 | } else if _, _, _, _, _, ok := selected.CreateSessionDestination(); ok { | ||
| 483 | - | createDestination = selected | 467 | + | next.kind, next.createDestination = paletteDecisionCreateDestination, selected |
| 484 | - | hasCreateDestination = true | |||
| 485 | } else if _, ok := selected.SessionTarget(); ok { | 468 | } else if _, ok := selected.SessionTarget(); ok { | ||
| 486 | - | sessionTarget = selected | 469 | + | next.kind, next.sessionTarget = paletteDecisionSessionTarget, selected |
| 487 | - | hasSessionTarget = true | |||
| 488 | } else { | 470 | } else { | ||
| 489 | changed = true | 471 | changed = true | ||
| 490 | return | 472 | return | ||
| 491 | } | 473 | } | ||
| 492 | - | generation = ac.overlays.paletteGeneration | 474 | + | decision = next |
| 493 | execute = true | 475 | execute = true | ||
| 494 | }, | 476 | }, | ||
| 495 | }) | 477 | }) | ||
| @@ -515,41 +497,22 @@ func decidePaletteInput(ac *attachedClient, data []byte) paletteDecision { | |||||
| 515 | } | 497 | } | ||
| 516 | } | 498 | } | ||
| 517 | if cancel { | 499 | if cancel { | ||
| 518 | - | cancelInventoryInteraction, hasCancelInventory = takePaletteInventoryClose(ac.overlays) | 500 | + | cancelled := paletteDecision{kind: paletteDecisionCancel} |
| 501 | + | cancelled.cancelInventoryInteraction, cancelled.hasCancelInventory = takePaletteInventoryClose(ac.overlays) | |||
| 519 | ac.clearPaletteLocked() | 502 | ac.clearPaletteLocked() | ||
| 503 | + | ac.overlays.paletteMu.Unlock() | |||
| 504 | + | return cancelled | |||
| 520 | } | 505 | } | ||
| 521 | ac.overlays.paletteMu.Unlock() | 506 | ac.overlays.paletteMu.Unlock() | ||
| 522 | 507 | ||||
| 523 | switch { | 508 | switch { | ||
| 524 | - | case cancel: | 509 | + | case execute: |
| 525 | - | return paletteDecision{kind: paletteDecisionCancel, cancelInventoryInteraction: cancelInventoryInteraction, hasCancelInventory: hasCancelInventory} | 510 | + | return decision |
| 526 | - | case !execute && changed: | 511 | + | case changed: |
| 527 | return paletteDecision{kind: paletteDecisionRedraw} | 512 | return paletteDecision{kind: paletteDecisionRedraw} | ||
| 528 | - | case !execute: | |||
| 529 | - | return paletteDecision{} | |||
| 530 | - | } | |||
| 531 | - | decision := paletteDecision{generation: generation, rawQuery: rawQuery} | |||
| 532 | - | switch { | |||
| 533 | - | case hasCreateDestination: | |||
| 534 | - | decision.kind = paletteDecisionCreateDestination | |||
| 535 | - | decision.createDestination = createDestination | |||
| 536 | - | case hasImportedTarget: | |||
| 537 | - | decision.kind = paletteDecisionImportedSession | |||
| 538 | - | decision.importedSourceKey = importedSourceKey | |||
| 539 | - | decision.importedEntryKey = importedEntryKey | |||
| 540 | - | case hasRouteTarget: | |||
| 541 | - | decision.kind = paletteDecisionRouteNavigation | |||
| 542 | - | decision.routeTarget = routeTarget | |||
| 543 | - | case hasSessionTarget: | |||
| 544 | - | decision.kind = paletteDecisionSessionTarget | |||
| 545 | - | decision.sessionTarget = sessionTarget | |||
| 546 | default: | 513 | default: | ||
| 547 | - | decision.kind = paletteDecisionCommand | 514 | + | return paletteDecision{} |
| 548 | - | decision.cmd = cmd | |||
| 549 | - | decision.args = args | |||
| 550 | - | decision.routeSnapshot = routeSnapshot | |||
| 551 | } | 515 | } | ||
| 552 | - | return decision | |||
| 553 | } | 516 | } | ||
| 554 | 517 | ||||
| 555 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ...*attachmentEffect) { | 518 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ...*attachmentEffect) { | ||
M internal/usecase/daemon/pane_titles.go +1 -1
| @@ -187,7 +187,7 @@ func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string | |||
| 187 | 187 | // distinct from the display fallback: an OSC title must then stand alone, | |
| 188 | 188 | // and future floating panes can supply their command fallback at format time. | |
| 189 | 189 | processName := "" | |
| 190 | - | if d.hasProcessDetails() && p.pty != nil { | |
| 190 | + | if d.proc != nil && p.pty != nil { | |
| 191 | 191 | if pgid, err := p.pty.ForegroundPgid(); err == nil && pgid > 0 { | |
| 192 | 192 | if comm, err := d.proc.Comm(pgid); err == nil && strings.TrimSpace(comm) != "" { | |
| 193 | 193 | processName = strings.TrimSpace(comm) | |
| @@ -187,7 +187,7 @@ func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string | |||||
| 187 | // distinct from the display fallback: an OSC title must then stand alone, | 187 | // distinct from the display fallback: an OSC title must then stand alone, | ||
| 188 | // and future floating panes can supply their command fallback at format time. | 188 | // and future floating panes can supply their command fallback at format time. | ||
| 189 | processName := "" | 189 | processName := "" | ||
| 190 | - | if d.hasProcessDetails() && p.pty != nil { | 190 | + | if d.proc != nil && p.pty != nil { |
| 191 | if pgid, err := p.pty.ForegroundPgid(); err == nil && pgid > 0 { | 191 | if pgid, err := p.pty.ForegroundPgid(); err == nil && pgid > 0 { | ||
| 192 | if comm, err := d.proc.Comm(pgid); err == nil && strings.TrimSpace(comm) != "" { | 192 | if comm, err := d.proc.Comm(pgid); err == nil && strings.TrimSpace(comm) != "" { | ||
| 193 | processName = strings.TrimSpace(comm) | 193 | processName = strings.TrimSpace(comm) | ||
M internal/usecase/daemon/pane_titles_test.go +2 -0
| @@ -205,6 +205,8 @@ func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | |||
| 205 | 205 | require.Equal(t, "sh", d.refreshPaneDisplayTitle(sess, p, true), "pointer refresh must retain its pane-owned fallback") | |
| 206 | 206 | } | |
| 207 | 207 | ||
| 208 | + | var errProcessInspectionUnavailable = errors.New("process inspection unavailable") | |
| 209 | + | ||
| 208 | 210 | // processInspectorFake is a hand-written ports.ProcessInspector whose | |
| 209 | 211 | // behavior is set per test through func fields; an unset func reports an | |
| 210 | 212 | // error, the same as an inspector that cannot read the process. | |
| @@ -205,6 +205,8 @@ func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | |||||
| 205 | require.Equal(t, "sh", d.refreshPaneDisplayTitle(sess, p, true), "pointer refresh must retain its pane-owned fallback") | 205 | require.Equal(t, "sh", d.refreshPaneDisplayTitle(sess, p, true), "pointer refresh must retain its pane-owned fallback") | ||
| 206 | } | 206 | } | ||
| 207 | 207 | ||||
| 208 | + | var errProcessInspectionUnavailable = errors.New("process inspection unavailable") | |||
| 209 | + | ||||
| 208 | // processInspectorFake is a hand-written ports.ProcessInspector whose | 210 | // processInspectorFake is a hand-written ports.ProcessInspector whose | ||
| 209 | // behavior is set per test through func fields; an unset func reports an | 211 | // behavior is set per test through func fields; an unset func reports an | ||
| 210 | // error, the same as an inspector that cannot read the process. | 212 | // error, the same as an inspector that cannot read the process. | ||
D internal/usecase/daemon/process_inspector.go +0 -58
| @@ -1,58 +0,0 @@ | |||
| 1 | - | package daemon | |
| 2 | - | ||
| 3 | - | import ( | |
| 4 | - | "errors" | |
| 5 | - | ||
| 6 | - | "github.com/bnema/vev/internal/ports" | |
| 7 | - | ) | |
| 8 | - | ||
| 9 | - | var errProcessInspectionUnavailable = errors.New("process inspection unavailable") | |
| 10 | - | ||
| 11 | - | // cwdOverrideInspector replaces only Cwd on top of an optional base inspector. | |
| 12 | - | // With a nil base, Comm, Argv, and GroupArgv report the inspection as | |
| 13 | - | // unavailable, matching a daemon that was given only a cwd reader. | |
| 14 | - | type cwdOverrideInspector struct { | |
| 15 | - | base ports.ProcessInspector | |
| 16 | - | cwd func(int) (string, error) | |
| 17 | - | } | |
| 18 | - | ||
| 19 | - | func (i cwdOverrideInspector) Cwd(pid int) (string, error) { return i.cwd(pid) } | |
| 20 | - | ||
| 21 | - | func (i cwdOverrideInspector) Comm(pid int) (string, error) { | |
| 22 | - | if i.base == nil { | |
| 23 | - | return "", errProcessInspectionUnavailable | |
| 24 | - | } | |
| 25 | - | return i.base.Comm(pid) | |
| 26 | - | } | |
| 27 | - | ||
| 28 | - | func (i cwdOverrideInspector) Argv(pid int) ([]string, error) { | |
| 29 | - | if i.base == nil { | |
| 30 | - | return nil, errProcessInspectionUnavailable | |
| 31 | - | } | |
| 32 | - | return i.base.Argv(pid) | |
| 33 | - | } | |
| 34 | - | ||
| 35 | - | func (i cwdOverrideInspector) GroupArgv(pgid, shellPid int) ([]string, error) { | |
| 36 | - | if i.base == nil { | |
| 37 | - | return nil, errProcessInspectionUnavailable | |
| 38 | - | } | |
| 39 | - | return i.base.GroupArgv(pgid, shellPid) | |
| 40 | - | } | |
| 41 | - | ||
| 42 | - | // unwrapCwdOverride returns the inspector beneath a previous cwd override so | |
| 43 | - | // repeated overrides do not stack. | |
| 44 | - | func unwrapCwdOverride(ins ports.ProcessInspector) ports.ProcessInspector { | |
| 45 | - | if o, ok := ins.(cwdOverrideInspector); ok { | |
| 46 | - | return o.base | |
| 47 | - | } | |
| 48 | - | return ins | |
| 49 | - | } | |
| 50 | - | ||
| 51 | - | // hasProcessDetails reports whether Comm, Argv, and GroupArgv are backed by a | |
| 52 | - | // real inspector rather than a cwd-only override. | |
| 53 | - | func (d *Daemon) hasProcessDetails() bool { | |
| 54 | - | if o, ok := d.proc.(cwdOverrideInspector); ok { | |
| 55 | - | return o.base != nil | |
| 56 | - | } | |
| 57 | - | return d.proc != nil | |
| 58 | - | } | |
| @@ -1,58 +0,0 @@ | |||||
| 1 | - | package daemon | |||
| 2 | - | ||||
| 3 | - | import ( | |||
| 4 | - | "errors" | |||
| 5 | - | ||||
| 6 | - | "github.com/bnema/vev/internal/ports" | |||
| 7 | - | ) | |||
| 8 | - | ||||
| 9 | - | var errProcessInspectionUnavailable = errors.New("process inspection unavailable") | |||
| 10 | - | ||||
| 11 | - | // cwdOverrideInspector replaces only Cwd on top of an optional base inspector. | |||
| 12 | - | // With a nil base, Comm, Argv, and GroupArgv report the inspection as | |||
| 13 | - | // unavailable, matching a daemon that was given only a cwd reader. | |||
| 14 | - | type cwdOverrideInspector struct { | |||
| 15 | - | base ports.ProcessInspector | |||
| 16 | - | cwd func(int) (string, error) | |||
| 17 | - | } | |||
| 18 | - | ||||
| 19 | - | func (i cwdOverrideInspector) Cwd(pid int) (string, error) { return i.cwd(pid) } | |||
| 20 | - | ||||
| 21 | - | func (i cwdOverrideInspector) Comm(pid int) (string, error) { | |||
| 22 | - | if i.base == nil { | |||
| 23 | - | return "", errProcessInspectionUnavailable | |||
| 24 | - | } | |||
| 25 | - | return i.base.Comm(pid) | |||
| 26 | - | } | |||
| 27 | - | ||||
| 28 | - | func (i cwdOverrideInspector) Argv(pid int) ([]string, error) { | |||
| 29 | - | if i.base == nil { | |||
| 30 | - | return nil, errProcessInspectionUnavailable | |||
| 31 | - | } | |||
| 32 | - | return i.base.Argv(pid) | |||
| 33 | - | } | |||
| 34 | - | ||||
| 35 | - | func (i cwdOverrideInspector) GroupArgv(pgid, shellPid int) ([]string, error) { | |||
| 36 | - | if i.base == nil { | |||
| 37 | - | return nil, errProcessInspectionUnavailable | |||
| 38 | - | } | |||
| 39 | - | return i.base.GroupArgv(pgid, shellPid) | |||
| 40 | - | } | |||
| 41 | - | ||||
| 42 | - | // unwrapCwdOverride returns the inspector beneath a previous cwd override so | |||
| 43 | - | // repeated overrides do not stack. | |||
| 44 | - | func unwrapCwdOverride(ins ports.ProcessInspector) ports.ProcessInspector { | |||
| 45 | - | if o, ok := ins.(cwdOverrideInspector); ok { | |||
| 46 | - | return o.base | |||
| 47 | - | } | |||
| 48 | - | return ins | |||
| 49 | - | } | |||
| 50 | - | ||||
| 51 | - | // hasProcessDetails reports whether Comm, Argv, and GroupArgv are backed by a | |||
| 52 | - | // real inspector rather than a cwd-only override. | |||
| 53 | - | func (d *Daemon) hasProcessDetails() bool { | |||
| 54 | - | if o, ok := d.proc.(cwdOverrideInspector); ok { | |||
| 55 | - | return o.base != nil | |||
| 56 | - | } | |||
| 57 | - | return d.proc != nil | |||
| 58 | - | } | |||
D internal/usecase/daemon/process_inspector_test.go +0 -83
| @@ -1,83 +0,0 @@ | |||
| 1 | - | package daemon | |
| 2 | - | ||
| 3 | - | import ( | |
| 4 | - | "testing" | |
| 5 | - | ||
| 6 | - | "github.com/stretchr/testify/require" | |
| 7 | - | ) | |
| 8 | - | ||
| 9 | - | func TestProcessInspectorOptionsComposition(t *testing.T) { | |
| 10 | - | cwd := func(int) (string, error) { return "/override", nil } | |
| 11 | - | base := &processInspectorFake{ | |
| 12 | - | cwd: func(int) (string, error) { return "/base", nil }, | |
| 13 | - | comm: func(int) (string, error) { return "vim", nil }, | |
| 14 | - | } | |
| 15 | - | ||
| 16 | - | tests := []struct { | |
| 17 | - | name string | |
| 18 | - | opts []Option | |
| 19 | - | wantDetails bool | |
| 20 | - | wantCwd string | |
| 21 | - | wantComm string | |
| 22 | - | wantCommErr error | |
| 23 | - | }{ | |
| 24 | - | {name: "none", wantDetails: false}, | |
| 25 | - | { | |
| 26 | - | name: "cwd reader only", | |
| 27 | - | opts: []Option{WithCwdReader(cwd)}, | |
| 28 | - | wantDetails: false, | |
| 29 | - | wantCwd: "/override", | |
| 30 | - | wantCommErr: errProcessInspectionUnavailable, | |
| 31 | - | }, | |
| 32 | - | { | |
| 33 | - | name: "inspector only", | |
| 34 | - | opts: []Option{WithProcessInspector(base)}, | |
| 35 | - | wantDetails: true, | |
| 36 | - | wantCwd: "/base", | |
| 37 | - | wantComm: "vim", | |
| 38 | - | }, | |
| 39 | - | { | |
| 40 | - | name: "inspector then cwd reader keeps base details", | |
| 41 | - | opts: []Option{WithProcessInspector(base), WithCwdReader(cwd)}, | |
| 42 | - | wantDetails: true, | |
| 43 | - | wantCwd: "/override", | |
| 44 | - | wantComm: "vim", | |
| 45 | - | }, | |
| 46 | - | { | |
| 47 | - | name: "cwd reader then inspector replaces override", | |
| 48 | - | opts: []Option{WithCwdReader(cwd), WithProcessInspector(base)}, | |
| 49 | - | wantDetails: true, | |
| 50 | - | wantCwd: "/base", | |
| 51 | - | wantComm: "vim", | |
| 52 | - | }, | |
| 53 | - | { | |
| 54 | - | name: "repeated cwd readers do not stack", | |
| 55 | - | opts: []Option{WithProcessInspector(base), WithCwdReader(cwd), WithCwdReader(cwd)}, | |
| 56 | - | wantDetails: true, | |
| 57 | - | wantCwd: "/override", | |
| 58 | - | wantComm: "vim", | |
| 59 | - | }, | |
| 60 | - | } | |
| 61 | - | for _, tt := range tests { | |
| 62 | - | t.Run(tt.name, func(t *testing.T) { | |
| 63 | - | d := &Daemon{} | |
| 64 | - | for _, opt := range tt.opts { | |
| 65 | - | opt(d) | |
| 66 | - | } | |
| 67 | - | require.Equal(t, tt.wantDetails, d.hasProcessDetails()) | |
| 68 | - | if d.proc == nil { | |
| 69 | - | return | |
| 70 | - | } | |
| 71 | - | got, err := d.proc.Cwd(1) | |
| 72 | - | require.NoError(t, err) | |
| 73 | - | require.Equal(t, tt.wantCwd, got) | |
| 74 | - | comm, err := d.proc.Comm(1) | |
| 75 | - | require.ErrorIs(t, err, tt.wantCommErr) | |
| 76 | - | require.Equal(t, tt.wantComm, comm) | |
| 77 | - | if o, ok := d.proc.(cwdOverrideInspector); ok { | |
| 78 | - | _, nested := o.base.(cwdOverrideInspector) | |
| 79 | - | require.False(t, nested) | |
| 80 | - | } | |
| 81 | - | }) | |
| 82 | - | } | |
| 83 | - | } | |
| @@ -1,83 +0,0 @@ | |||||
| 1 | - | package daemon | |||
| 2 | - | ||||
| 3 | - | import ( | |||
| 4 | - | "testing" | |||
| 5 | - | ||||
| 6 | - | "github.com/stretchr/testify/require" | |||
| 7 | - | ) | |||
| 8 | - | ||||
| 9 | - | func TestProcessInspectorOptionsComposition(t *testing.T) { | |||
| 10 | - | cwd := func(int) (string, error) { return "/override", nil } | |||
| 11 | - | base := &processInspectorFake{ | |||
| 12 | - | cwd: func(int) (string, error) { return "/base", nil }, | |||
| 13 | - | comm: func(int) (string, error) { return "vim", nil }, | |||
| 14 | - | } | |||
| 15 | - | ||||
| 16 | - | tests := []struct { | |||
| 17 | - | name string | |||
| 18 | - | opts []Option | |||
| 19 | - | wantDetails bool | |||
| 20 | - | wantCwd string | |||
| 21 | - | wantComm string | |||
| 22 | - | wantCommErr error | |||
| 23 | - | }{ | |||
| 24 | - | {name: "none", wantDetails: false}, | |||
| 25 | - | { | |||
| 26 | - | name: "cwd reader only", | |||
| 27 | - | opts: []Option{WithCwdReader(cwd)}, | |||
| 28 | - | wantDetails: false, | |||
| 29 | - | wantCwd: "/override", | |||
| 30 | - | wantCommErr: errProcessInspectionUnavailable, | |||
| 31 | - | }, | |||
| 32 | - | { | |||
| 33 | - | name: "inspector only", | |||
| 34 | - | opts: []Option{WithProcessInspector(base)}, | |||
| 35 | - | wantDetails: true, | |||
| 36 | - | wantCwd: "/base", | |||
| 37 | - | wantComm: "vim", | |||
| 38 | - | }, | |||
| 39 | - | { | |||
| 40 | - | name: "inspector then cwd reader keeps base details", | |||
| 41 | - | opts: []Option{WithProcessInspector(base), WithCwdReader(cwd)}, | |||
| 42 | - | wantDetails: true, | |||
| 43 | - | wantCwd: "/override", | |||
| 44 | - | wantComm: "vim", | |||
| 45 | - | }, | |||
| 46 | - | { | |||
| 47 | - | name: "cwd reader then inspector replaces override", | |||
| 48 | - | opts: []Option{WithCwdReader(cwd), WithProcessInspector(base)}, | |||
| 49 | - | wantDetails: true, | |||
| 50 | - | wantCwd: "/base", | |||
| 51 | - | wantComm: "vim", | |||
| 52 | - | }, | |||
| 53 | - | { | |||
| 54 | - | name: "repeated cwd readers do not stack", | |||
| 55 | - | opts: []Option{WithProcessInspector(base), WithCwdReader(cwd), WithCwdReader(cwd)}, | |||
| 56 | - | wantDetails: true, | |||
| 57 | - | wantCwd: "/override", | |||
| 58 | - | wantComm: "vim", | |||
| 59 | - | }, | |||
| 60 | - | } | |||
| 61 | - | for _, tt := range tests { | |||
| 62 | - | t.Run(tt.name, func(t *testing.T) { | |||
| 63 | - | d := &Daemon{} | |||
| 64 | - | for _, opt := range tt.opts { | |||
| 65 | - | opt(d) | |||
| 66 | - | } | |||
| 67 | - | require.Equal(t, tt.wantDetails, d.hasProcessDetails()) | |||
| 68 | - | if d.proc == nil { | |||
| 69 | - | return | |||
| 70 | - | } | |||
| 71 | - | got, err := d.proc.Cwd(1) | |||
| 72 | - | require.NoError(t, err) | |||
| 73 | - | require.Equal(t, tt.wantCwd, got) | |||
| 74 | - | comm, err := d.proc.Comm(1) | |||
| 75 | - | require.ErrorIs(t, err, tt.wantCommErr) | |||
| 76 | - | require.Equal(t, tt.wantComm, comm) | |||
| 77 | - | if o, ok := d.proc.(cwdOverrideInspector); ok { | |||
| 78 | - | _, nested := o.base.(cwdOverrideInspector) | |||
| 79 | - | require.False(t, nested) | |||
| 80 | - | } | |||
| 81 | - | }) | |||
| 82 | - | } | |||
| 83 | - | } | |||
M internal/usecase/daemon/render.go +3 -2
| @@ -445,8 +445,9 @@ func (d *Daemon) paintLocked(entry *session, ac *attachedClient, tb *tab, reset | |||
| 445 | 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | |
| 446 | 446 | } | |
| 447 | 447 | ||
| 448 | - | paletteCfg := d.currentPaletteConfig() | |
| 449 | - | floatingCfg := d.currentFloatingConfig() | |
| 448 | + | // One snapshot keeps palette and floating settings from the same reload. | |
| 449 | + | cfg := d.currentRuntimeConfig() | |
| 450 | + | paletteCfg, floatingCfg := cfg.palette, cfg.floating | |
| 450 | 451 | // Title refresh may inspect process state, so it remains before the capture | |
| 451 | 452 | // boundary and outside tab/pane ownership locks. | |
| 452 | 453 | if local { | |
| @@ -445,8 +445,9 @@ func (d *Daemon) paintLocked(entry *session, ac *attachedClient, tb *tab, reset | |||||
| 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | ||
| 446 | } | 446 | } | ||
| 447 | 447 | ||||
| 448 | - | paletteCfg := d.currentPaletteConfig() | 448 | + | // One snapshot keeps palette and floating settings from the same reload. |
| 449 | - | floatingCfg := d.currentFloatingConfig() | 449 | + | cfg := d.currentRuntimeConfig() |
| 450 | + | paletteCfg, floatingCfg := cfg.palette, cfg.floating | |||
| 450 | // Title refresh may inspect process state, so it remains before the capture | 451 | // Title refresh may inspect process state, so it remains before the capture | ||
| 451 | // boundary and outside tab/pane ownership locks. | 452 | // boundary and outside tab/pane ownership locks. | ||
| 452 | if local { | 453 | if local { | ||
M internal/usecase/daemon/render_pipeline.go +2 -1
| @@ -59,7 +59,8 @@ type composeCacheInput struct { | |||
| 59 | 59 | // the same commit/spare rotation as the cached frame, so a page is only ever | |
| 60 | 60 | // written by a composition whose input scratch owns it. Neither slot may hold | |
| 61 | 61 | // a page shared with a cached frame (frame, copyViewport.frame) or with another | |
| 62 | - | // slot; they are filled only by copyFrameInto, which allocates a fresh page. | |
| 62 | + | // slot; they are filled only by copyFrameInto, whose first fill clones, so a | |
| 63 | + | // slot never shares a page with a cached frame. | |
| 63 | 64 | type transientFrames struct { | |
| 64 | 65 | // overlay receives the toast-decorated or modal-decorated copy of the base | |
| 65 | 66 | // frame. Toasts and modals never both clone, so they share the slot. | |
| @@ -59,7 +59,8 @@ type composeCacheInput struct { | |||||
| 59 | // the same commit/spare rotation as the cached frame, so a page is only ever | 59 | // the same commit/spare rotation as the cached frame, so a page is only ever | ||
| 60 | // written by a composition whose input scratch owns it. Neither slot may hold | 60 | // written by a composition whose input scratch owns it. Neither slot may hold | ||
| 61 | // a page shared with a cached frame (frame, copyViewport.frame) or with another | 61 | // a page shared with a cached frame (frame, copyViewport.frame) or with another | ||
| 62 | - | // slot; they are filled only by copyFrameInto, which allocates a fresh page. | 62 | + | // slot; they are filled only by copyFrameInto, whose first fill clones, so a |
| 63 | + | // slot never shares a page with a cached frame. | |||
| 63 | type transientFrames struct { | 64 | type transientFrames struct { | ||
| 64 | // overlay receives the toast-decorated or modal-decorated copy of the base | 65 | // overlay receives the toast-decorated or modal-decorated copy of the base | ||
| 65 | // frame. Toasts and modals never both clone, so they share the slot. | 66 | // frame. Toasts and modals never both clone, so they share the slot. | ||
M internal/usecase/daemon/same_peer_switch.go +8 -7
| @@ -69,17 +69,13 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||
| 69 | 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | |
| 70 | 70 | return | |
| 71 | 71 | } | |
| 72 | - | if !effect.ac.consumeSamePeerOffer(request.Target) { | |
| 73 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 74 | - | return | |
| 75 | - | } | |
| 76 | - | ||
| 77 | 72 | // The switch moves an attachment between sessions, so it must not run | |
| 78 | - | // inside the exact set a KillAll purge has captured. | |
| 73 | + | // inside the exact set a KillAll purge has captured. A purge is retry | |
| 74 | + | // state, so it is checked first and keeps any pending daemon offer. | |
| 79 | 75 | d.mu.Lock() | |
| 80 | 76 | if d.purgeAdmissionClosedLocked() { | |
| 81 | 77 | d.mu.Unlock() | |
| 82 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 78 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchUnavailable) | |
| 83 | 79 | return | |
| 84 | 80 | } | |
| 85 | 81 | d.purgeAdmissionActive++ | |
| @@ -90,6 +86,11 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||
| 90 | 86 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | |
| 91 | 87 | defer release() | |
| 92 | 88 | ||
| 89 | + | if !effect.ac.consumeSamePeerOffer(request.Target) { | |
| 90 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 91 | + | return | |
| 92 | + | } | |
| 93 | + | ||
| 93 | 94 | target, targetTabIndex, ok := d.samePeerTarget(request) | |
| 94 | 95 | if !ok || target == effect.sess { | |
| 95 | 96 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| @@ -69,17 +69,13 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||||
| 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | ||
| 70 | return | 70 | return | ||
| 71 | } | 71 | } | ||
| 72 | - | if !effect.ac.consumeSamePeerOffer(request.Target) { | |||
| 73 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |||
| 74 | - | return | |||
| 75 | - | } | |||
| 76 | - | ||||
| 77 | // The switch moves an attachment between sessions, so it must not run | 72 | // The switch moves an attachment between sessions, so it must not run | ||
| 78 | - | // inside the exact set a KillAll purge has captured. | 73 | + | // inside the exact set a KillAll purge has captured. A purge is retry |
| 74 | + | // state, so it is checked first and keeps any pending daemon offer. | |||
| 79 | d.mu.Lock() | 75 | d.mu.Lock() | ||
| 80 | if d.purgeAdmissionClosedLocked() { | 76 | if d.purgeAdmissionClosedLocked() { | ||
| 81 | d.mu.Unlock() | 77 | d.mu.Unlock() | ||
| 82 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | 78 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchUnavailable) |
| 83 | return | 79 | return | ||
| 84 | } | 80 | } | ||
| 85 | d.purgeAdmissionActive++ | 81 | d.purgeAdmissionActive++ | ||
| @@ -90,6 +86,11 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||||
| 90 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | 86 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | ||
| 91 | defer release() | 87 | defer release() | ||
| 92 | 88 | ||||
| 89 | + | if !effect.ac.consumeSamePeerOffer(request.Target) { | |||
| 90 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |||
| 91 | + | return | |||
| 92 | + | } | |||
| 93 | + | ||||
| 93 | target, targetTabIndex, ok := d.samePeerTarget(request) | 94 | target, targetTabIndex, ok := d.samePeerTarget(request) | ||
| 94 | if !ok || target == effect.sess { | 95 | if !ok || target == effect.sess { | ||
| 95 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | 96 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | ||
M internal/usecase/daemon/same_peer_switch_test.go +10 -3
| @@ -133,11 +133,13 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||
| 133 | 133 | offer *protocol.ExactSessionTarget | |
| 134 | 134 | purging bool | |
| 135 | 135 | switched bool | |
| 136 | + | wantCode protocol.SamePeerSwitchFailureCode | |
| 136 | 137 | }{ | |
| 137 | 138 | {name: "no pending offer switches", switched: true}, | |
| 138 | 139 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | |
| 139 | - | {name: "another pending offer refuses", offer: &otherOffer}, | |
| 140 | - | {name: "a running purge refuses", purging: true}, | |
| 140 | + | {name: "another pending offer refuses", offer: &otherOffer, wantCode: protocol.SamePeerSwitchStaleTarget}, | |
| 141 | + | {name: "a running purge refuses", purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |
| 142 | + | {name: "a running purge keeps the pending offer", offer: &requestTarget, purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |
| 141 | 143 | } | |
| 142 | 144 | for _, tt := range tests { | |
| 143 | 145 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -179,7 +181,12 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||
| 179 | 181 | } | |
| 180 | 182 | require.Same(t, source, ac.currentAttachmentSession()) | |
| 181 | 183 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | |
| 182 | - | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | |
| 184 | + | require.Equal(t, tt.wantCode, failure.Code) | |
| 185 | + | if tt.offer != nil { | |
| 186 | + | ac.samePeerOfferMu.Lock() | |
| 187 | + | defer ac.samePeerOfferMu.Unlock() | |
| 188 | + | require.Equal(t, tt.offer, ac.samePeerOffer, "a refused switch keeps the pending offer") | |
| 189 | + | } | |
| 183 | 190 | }) | |
| 184 | 191 | } | |
| 185 | 192 | } | |
| @@ -133,11 +133,13 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||||
| 133 | offer *protocol.ExactSessionTarget | 133 | offer *protocol.ExactSessionTarget | ||
| 134 | purging bool | 134 | purging bool | ||
| 135 | switched bool | 135 | switched bool | ||
| 136 | + | wantCode protocol.SamePeerSwitchFailureCode | |||
| 136 | }{ | 137 | }{ | ||
| 137 | {name: "no pending offer switches", switched: true}, | 138 | {name: "no pending offer switches", switched: true}, | ||
| 138 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | 139 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | ||
| 139 | - | {name: "another pending offer refuses", offer: &otherOffer}, | 140 | + | {name: "another pending offer refuses", offer: &otherOffer, wantCode: protocol.SamePeerSwitchStaleTarget}, |
| 140 | - | {name: "a running purge refuses", purging: true}, | 141 | + | {name: "a running purge refuses", purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, |
| 142 | + | {name: "a running purge keeps the pending offer", offer: &requestTarget, purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |||
| 141 | } | 143 | } | ||
| 142 | for _, tt := range tests { | 144 | for _, tt := range tests { | ||
| 143 | t.Run(tt.name, func(t *testing.T) { | 145 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -179,7 +181,12 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||||
| 179 | } | 181 | } | ||
| 180 | require.Same(t, source, ac.currentAttachmentSession()) | 182 | require.Same(t, source, ac.currentAttachmentSession()) | ||
| 181 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | 183 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | ||
| 182 | - | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | 184 | + | require.Equal(t, tt.wantCode, failure.Code) |
| 185 | + | if tt.offer != nil { | |||
| 186 | + | ac.samePeerOfferMu.Lock() | |||
| 187 | + | defer ac.samePeerOfferMu.Unlock() | |||
| 188 | + | require.Equal(t, tt.offer, ac.samePeerOffer, "a refused switch keeps the pending offer") | |||
| 189 | + | } | |||
| 183 | }) | 190 | }) | ||
| 184 | } | 191 | } | ||
| 185 | } | 192 | } | ||
M internal/usecase/daemon/session.go +1 -4
| @@ -726,14 +726,11 @@ func (d *Daemon) createTabForAttachment(sess *session, ac *attachedClient, _ dom | |||
| 726 | 726 | themeClient = attachments[0] | |
| 727 | 727 | } | |
| 728 | 728 | if themeClient != nil { | |
| 729 | - | t := d.effectiveTheme(themeClient.getClientTheme()) | |
| 730 | 729 | tb.mu.Lock() | |
| 731 | 730 | p := tb.focusedPane() | |
| 732 | 731 | tb.mu.Unlock() | |
| 733 | 732 | if p != nil { | |
| 734 | - | p.mu.Lock() | |
| 735 | - | applyPaneThemeLocked(p, t, false) | |
| 736 | - | p.mu.Unlock() | |
| 733 | + | applyClientThemeToPane(d, themeClient, p) | |
| 737 | 734 | } | |
| 738 | 735 | } | |
| 739 | 736 | d.mu.Lock() | |
| @@ -726,14 +726,11 @@ func (d *Daemon) createTabForAttachment(sess *session, ac *attachedClient, _ dom | |||||
| 726 | themeClient = attachments[0] | 726 | themeClient = attachments[0] | ||
| 727 | } | 727 | } | ||
| 728 | if themeClient != nil { | 728 | if themeClient != nil { | ||
| 729 | - | t := d.effectiveTheme(themeClient.getClientTheme()) | |||
| 730 | tb.mu.Lock() | 729 | tb.mu.Lock() | ||
| 731 | p := tb.focusedPane() | 730 | p := tb.focusedPane() | ||
| 732 | tb.mu.Unlock() | 731 | tb.mu.Unlock() | ||
| 733 | if p != nil { | 732 | if p != nil { | ||
| 734 | - | p.mu.Lock() | 733 | + | applyClientThemeToPane(d, themeClient, p) |
| 735 | - | applyPaneThemeLocked(p, t, false) | |||
| 736 | - | p.mu.Unlock() | |||
| 737 | } | 734 | } | ||
| 738 | } | 735 | } | ||
| 739 | d.mu.Lock() | 736 | d.mu.Lock() | ||
M internal/usecase/daemon/session_test.go +2 -2
| @@ -1336,7 +1336,7 @@ func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | |||
| 1336 | 1336 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 1337 | 1337 | WithStore(t, store)(d) | |
| 1338 | 1338 | cwd := "/tmp/work" | |
| 1339 | - | WithCwdReader(func(int) (string, error) { return cwd, nil })(d) | |
| 1339 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return cwd, nil }})(d) | |
| 1340 | 1340 | ||
| 1341 | 1341 | sess, err := createSessionForTest(d, "work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 1342 | 1342 | require.NoError(t, err) | |
| @@ -1529,7 +1529,7 @@ func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | |||
| 1529 | 1529 | store, state := newMockStore(t) | |
| 1530 | 1530 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | |
| 1531 | 1531 | WithStore(t, store)(d) | |
| 1532 | - | WithCwdReader(func(int) (string, error) { return "/tmp/latest", nil })(d) | |
| 1532 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return "/tmp/latest", nil }})(d) | |
| 1533 | 1533 | ||
| 1534 | 1534 | natural, err := createSessionForTest(d, "natural", false, "/tmp/old", sz, terminalEnv{}, d.baseEnv) | |
| 1535 | 1535 | require.NoError(t, err) | |
| @@ -1336,7 +1336,7 @@ func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | |||||
| 1336 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | 1336 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | ||
| 1337 | WithStore(t, store)(d) | 1337 | WithStore(t, store)(d) | ||
| 1338 | cwd := "/tmp/work" | 1338 | cwd := "/tmp/work" | ||
| 1339 | - | WithCwdReader(func(int) (string, error) { return cwd, nil })(d) | 1339 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return cwd, nil }})(d) |
| 1340 | 1340 | ||||
| 1341 | sess, err := createSessionForTest(d, "work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | 1341 | sess, err := createSessionForTest(d, "work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | ||
| 1342 | require.NoError(t, err) | 1342 | require.NoError(t, err) | ||
| @@ -1529,7 +1529,7 @@ func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | |||||
| 1529 | store, state := newMockStore(t) | 1529 | store, state := newMockStore(t) | ||
| 1530 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | 1530 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | ||
| 1531 | WithStore(t, store)(d) | 1531 | WithStore(t, store)(d) | ||
| 1532 | - | WithCwdReader(func(int) (string, error) { return "/tmp/latest", nil })(d) | 1532 | + | WithProcessInspector(&processInspectorFake{cwd: func(int) (string, error) { return "/tmp/latest", nil }})(d) |
| 1533 | 1533 | ||||
| 1534 | natural, err := createSessionForTest(d, "natural", false, "/tmp/old", sz, terminalEnv{}, d.baseEnv) | 1534 | natural, err := createSessionForTest(d, "natural", false, "/tmp/old", sz, terminalEnv{}, d.baseEnv) | ||
| 1535 | require.NoError(t, err) | 1535 | require.NoError(t, err) | ||
M internal/usecase/daemon/snapshot_capture.go +1 -1
| @@ -82,7 +82,7 @@ func (d *Daemon) captureSession(sess *session) bool { | |||
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | 84 | func (d *Daemon) capturePaneProcess(pty interface{ ForegroundPgid() (int, error) }, shellPid int) *snapcodec.Process { | |
| 85 | - | if d == nil || pty == nil || shellPid <= 0 || !d.hasProcessDetails() { | |
| 85 | + | if d == nil || pty == nil || shellPid <= 0 || d.proc == nil { | |
| 86 | 86 | return nil | |
| 87 | 87 | } | |
| 88 | 88 | pgid, err := pty.ForegroundPgid() | |
| @@ -82,7 +82,7 @@ func (d *Daemon) captureSession(sess *session) bool { | |||||
| 82 | } | 82 | } | ||
| 83 | 83 | ||||
| 84 | func (d *Daemon) capturePaneProcess(pty interface{ ForegroundPgid() (int, error) }, shellPid int) *snapcodec.Process { | 84 | func (d *Daemon) capturePaneProcess(pty interface{ ForegroundPgid() (int, error) }, shellPid int) *snapcodec.Process { | ||
| 85 | - | if d == nil || pty == nil || shellPid <= 0 || !d.hasProcessDetails() { | 85 | + | if d == nil || pty == nil || shellPid <= 0 || d.proc == nil { |
| 86 | return nil | 86 | return nil | ||
| 87 | } | 87 | } | ||
| 88 | pgid, err := pty.ForegroundPgid() | 88 | pgid, err := pty.ForegroundPgid() | ||
M internal/usecase/daemon/snapshot_restore_incremental.go +8 -3
| @@ -387,6 +387,14 @@ func (d *Daemon) logRejectedRestoreCandidate(record domain.CatalogueRecord, cand | |||
| 387 | 387 | } | |
| 388 | 388 | ||
| 389 | 389 | func sessionFromGeneration(generation ports.SnapshotGeneration) (snapcodec.Session, error) { | |
| 390 | + | // Objects are compressed, so the on-disk read budget no longer bounds the | |
| 391 | + | // inflated generation; budget its payloads as one uncompressed checkpoint. | |
| 392 | + | return sessionFromGenerationBudget(generation, snapcodec.MaxGenerationPayloadBytes) | |
| 393 | + | } | |
| 394 | + | ||
| 395 | + | // sessionFromGenerationBudget decodes a generation whose inflated object | |
| 396 | + | // payloads total at most budget bytes. | |
| 397 | + | func sessionFromGenerationBudget(generation ports.SnapshotGeneration, budget int) (snapcodec.Session, error) { | |
| 390 | 398 | manifest, err := snapcodec.UnmarshalManifest(generation.Manifest) | |
| 391 | 399 | if err != nil { | |
| 392 | 400 | return snapcodec.Session{}, err | |
| @@ -395,9 +403,6 @@ func sessionFromGeneration(generation ports.SnapshotGeneration) (snapcodec.Sessi | |||
| 395 | 403 | return snapcodec.Session{}, fmt.Errorf("snapshot: generation identity mismatch") | |
| 396 | 404 | } | |
| 397 | 405 | result := snapcodec.Session{Name: manifest.Name, CreatedAt: manifest.CreatedAt, Active: manifest.Active, Tabs: make([]snapcodec.Tab, 0, len(manifest.Tabs))} | |
| 398 | - | // Objects are compressed, so the on-disk read budget no longer bounds the | |
| 399 | - | // inflated generation; budget its payloads as one uncompressed checkpoint. | |
| 400 | - | budget := snapcodec.MaxGenerationPayloadBytes | |
| 401 | 406 | object := func(ref snapcodec.ObjectRef, kind snapcodec.ObjectKind) ([]byte, error) { | |
| 402 | 407 | data, err := generationObject(generation, ref, kind) | |
| 403 | 408 | if err != nil { | |
| @@ -387,6 +387,14 @@ func (d *Daemon) logRejectedRestoreCandidate(record domain.CatalogueRecord, cand | |||||
| 387 | } | 387 | } | ||
| 388 | 388 | ||||
| 389 | func sessionFromGeneration(generation ports.SnapshotGeneration) (snapcodec.Session, error) { | 389 | func sessionFromGeneration(generation ports.SnapshotGeneration) (snapcodec.Session, error) { | ||
| 390 | + | // Objects are compressed, so the on-disk read budget no longer bounds the | |||
| 391 | + | // inflated generation; budget its payloads as one uncompressed checkpoint. | |||
| 392 | + | return sessionFromGenerationBudget(generation, snapcodec.MaxGenerationPayloadBytes) | |||
| 393 | + | } | |||
| 394 | + | ||||
| 395 | + | // sessionFromGenerationBudget decodes a generation whose inflated object | |||
| 396 | + | // payloads total at most budget bytes. | |||
| 397 | + | func sessionFromGenerationBudget(generation ports.SnapshotGeneration, budget int) (snapcodec.Session, error) { | |||
| 390 | manifest, err := snapcodec.UnmarshalManifest(generation.Manifest) | 398 | manifest, err := snapcodec.UnmarshalManifest(generation.Manifest) | ||
| 391 | if err != nil { | 399 | if err != nil { | ||
| 392 | return snapcodec.Session{}, err | 400 | return snapcodec.Session{}, err | ||
| @@ -395,9 +403,6 @@ func sessionFromGeneration(generation ports.SnapshotGeneration) (snapcodec.Sessi | |||||
| 395 | return snapcodec.Session{}, fmt.Errorf("snapshot: generation identity mismatch") | 403 | return snapcodec.Session{}, fmt.Errorf("snapshot: generation identity mismatch") | ||
| 396 | } | 404 | } | ||
| 397 | result := snapcodec.Session{Name: manifest.Name, CreatedAt: manifest.CreatedAt, Active: manifest.Active, Tabs: make([]snapcodec.Tab, 0, len(manifest.Tabs))} | 405 | result := snapcodec.Session{Name: manifest.Name, CreatedAt: manifest.CreatedAt, Active: manifest.Active, Tabs: make([]snapcodec.Tab, 0, len(manifest.Tabs))} | ||
| 398 | - | // Objects are compressed, so the on-disk read budget no longer bounds the | |||
| 399 | - | // inflated generation; budget its payloads as one uncompressed checkpoint. | |||
| 400 | - | budget := snapcodec.MaxGenerationPayloadBytes | |||
| 401 | object := func(ref snapcodec.ObjectRef, kind snapcodec.ObjectKind) ([]byte, error) { | 406 | object := func(ref snapcodec.ObjectRef, kind snapcodec.ObjectKind) ([]byte, error) { | ||
| 402 | data, err := generationObject(generation, ref, kind) | 407 | data, err := generationObject(generation, ref, kind) | ||
| 403 | if err != nil { | 408 | if err != nil { | ||
M internal/usecase/daemon/snapshot_restore_test.go +36 -0
| @@ -623,3 +623,39 @@ func TestRestoreIncrementalFallbackAndInvalidObjectMappings(t *testing.T) { | |||
| 623 | 623 | }) | |
| 624 | 624 | } | |
| 625 | 625 | } | |
| 626 | + | ||
| 627 | + | // TestRestoreIncrementalPayloadBudget bounds the inflated payloads of one | |
| 628 | + | // generation: compressed objects must not restore past the budget. | |
| 629 | + | func TestRestoreIncrementalPayloadBudget(t *testing.T) { | |
| 630 | + | generation := acceptanceGeneration(t, restoreAcceptanceSession(t, "budget"), 3) | |
| 631 | + | decoded, err := sessionFromGeneration(generation) | |
| 632 | + | require.NoError(t, err) | |
| 633 | + | total := 0 | |
| 634 | + | for _, tab := range decoded.Tabs { | |
| 635 | + | for _, pane := range tab.Panes { | |
| 636 | + | for _, chunk := range pane.SealedChunks { | |
| 637 | + | total += len(chunk) | |
| 638 | + | } | |
| 639 | + | total += len(pane.Tail) + len(pane.Transcript) | |
| 640 | + | } | |
| 641 | + | } | |
| 642 | + | require.Positive(t, total) | |
| 643 | + | ||
| 644 | + | for _, tt := range []struct { | |
| 645 | + | name string | |
| 646 | + | budget int | |
| 647 | + | wantErr bool | |
| 648 | + | }{ | |
| 649 | + | {name: "exact budget", budget: total}, | |
| 650 | + | {name: "one byte short", budget: total - 1, wantErr: true}, | |
| 651 | + | } { | |
| 652 | + | t.Run(tt.name, func(t *testing.T) { | |
| 653 | + | _, err := sessionFromGenerationBudget(cloneAcceptanceGeneration(generation), tt.budget) | |
| 654 | + | if tt.wantErr { | |
| 655 | + | require.ErrorContains(t, err, "generation payload too large") | |
| 656 | + | return | |
| 657 | + | } | |
| 658 | + | require.NoError(t, err) | |
| 659 | + | }) | |
| 660 | + | } | |
| 661 | + | } | |
| @@ -623,3 +623,39 @@ func TestRestoreIncrementalFallbackAndInvalidObjectMappings(t *testing.T) { | |||||
| 623 | }) | 623 | }) | ||
| 624 | } | 624 | } | ||
| 625 | } | 625 | } | ||
| 626 | + | ||||
| 627 | + | // TestRestoreIncrementalPayloadBudget bounds the inflated payloads of one | |||
| 628 | + | // generation: compressed objects must not restore past the budget. | |||
| 629 | + | func TestRestoreIncrementalPayloadBudget(t *testing.T) { | |||
| 630 | + | generation := acceptanceGeneration(t, restoreAcceptanceSession(t, "budget"), 3) | |||
| 631 | + | decoded, err := sessionFromGeneration(generation) | |||
| 632 | + | require.NoError(t, err) | |||
| 633 | + | total := 0 | |||
| 634 | + | for _, tab := range decoded.Tabs { | |||
| 635 | + | for _, pane := range tab.Panes { | |||
| 636 | + | for _, chunk := range pane.SealedChunks { | |||
| 637 | + | total += len(chunk) | |||
| 638 | + | } | |||
| 639 | + | total += len(pane.Tail) + len(pane.Transcript) | |||
| 640 | + | } | |||
| 641 | + | } | |||
| 642 | + | require.Positive(t, total) | |||
| 643 | + | ||||
| 644 | + | for _, tt := range []struct { | |||
| 645 | + | name string | |||
| 646 | + | budget int | |||
| 647 | + | wantErr bool | |||
| 648 | + | }{ | |||
| 649 | + | {name: "exact budget", budget: total}, | |||
| 650 | + | {name: "one byte short", budget: total - 1, wantErr: true}, | |||
| 651 | + | } { | |||
| 652 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 653 | + | _, err := sessionFromGenerationBudget(cloneAcceptanceGeneration(generation), tt.budget) | |||
| 654 | + | if tt.wantErr { | |||
| 655 | + | require.ErrorContains(t, err, "generation payload too large") | |||
| 656 | + | return | |||
| 657 | + | } | |||
| 658 | + | require.NoError(t, err) | |||
| 659 | + | }) | |||
| 660 | + | } | |||
| 661 | + | } | |||
M internal/usecase/daemon/snapshot_worker_state.go +6 -4
| @@ -9,10 +9,12 @@ import ( | |||
| 9 | 9 | // other durable writers joined with it (maintenance and restoration), so | |
| 10 | 10 | // WaitDurableWriters can read all three completion signals under one lock. | |
| 11 | 11 | // | |
| 12 | - | // Lock position: mu is a leaf with respect to Daemon.mu. No code path acquires | |
| 13 | - | // Daemon.mu, a session lock, or a pane lock while holding it, and capture, | |
| 14 | - | // queue, and worker code never takes Daemon.mu. noticeMu is likewise a leaf and | |
| 15 | - | // is never held together with mu. | |
| 12 | + | // Lock position: mu is a leaf with respect to Daemon.mu and the session (mu), | |
| 13 | + | // tab, and pane locks: no path acquires any of them while holding it, and | |
| 14 | + | // capture, queue, and worker code never takes Daemon.mu. The one nested lock is | |
| 15 | + | // a session's snapshotMu, taken under mu by enqueueFinalSnapshotCapture; see | |
| 16 | + | // the lock-ordering notes in client.go. noticeMu is a leaf and is never held | |
| 17 | + | // together with mu. | |
| 16 | 18 | // | |
| 17 | 19 | // The zero value is usable for tests that build &Daemon{}: nil maps are | |
| 18 | 20 | // allocated lazily by the enqueue paths, and a nil jobs channel never becomes | |
| @@ -9,10 +9,12 @@ import ( | |||||
| 9 | // other durable writers joined with it (maintenance and restoration), so | 9 | // other durable writers joined with it (maintenance and restoration), so | ||
| 10 | // WaitDurableWriters can read all three completion signals under one lock. | 10 | // WaitDurableWriters can read all three completion signals under one lock. | ||
| 11 | // | 11 | // | ||
| 12 | - | // Lock position: mu is a leaf with respect to Daemon.mu. No code path acquires | 12 | + | // Lock position: mu is a leaf with respect to Daemon.mu and the session (mu), |
| 13 | - | // Daemon.mu, a session lock, or a pane lock while holding it, and capture, | 13 | + | // tab, and pane locks: no path acquires any of them while holding it, and |
| 14 | - | // queue, and worker code never takes Daemon.mu. noticeMu is likewise a leaf and | 14 | + | // capture, queue, and worker code never takes Daemon.mu. The one nested lock is |
| 15 | - | // is never held together with mu. | 15 | + | // a session's snapshotMu, taken under mu by enqueueFinalSnapshotCapture; see |
| 16 | + | // the lock-ordering notes in client.go. noticeMu is a leaf and is never held | |||
| 17 | + | // together with mu. | |||
| 16 | // | 18 | // | ||
| 17 | // The zero value is usable for tests that build &Daemon{}: nil maps are | 19 | // The zero value is usable for tests that build &Daemon{}: nil maps are | ||
| 18 | // allocated lazily by the enqueue paths, and a nil jobs channel never becomes | 20 | // allocated lazily by the enqueue paths, and a nil jobs channel never becomes | ||
M internal/usecase/fuzzy/fuzzy.go +8 -7
| @@ -9,6 +9,7 @@ import "strings" | |||
| 9 | 9 | // Kind is how a field matched the query. | |
| 10 | 10 | type Kind uint8 | |
| 11 | 11 | ||
| 12 | + | // Kinds are ordered best first, so a caller may use the Kind itself as a rank. | |
| 12 | 13 | const ( | |
| 13 | 14 | Exact Kind = iota | |
| 14 | 15 | Prefix | |
| @@ -33,11 +34,11 @@ func Match(text, query string, needle []rune) (Score, bool) { | |||
| 33 | 34 | var score Score | |
| 34 | 35 | switch { | |
| 35 | 36 | case text == query: | |
| 36 | - | score = Score{Kind: Exact, Positions: Range(len(needle))} | |
| 37 | + | score = Score{Kind: Exact, Positions: rangePositions(len(needle))} | |
| 37 | 38 | case strings.HasPrefix(text, query): | |
| 38 | - | score = Score{Kind: Prefix, Positions: Range(len(needle))} | |
| 39 | + | score = Score{Kind: Prefix, Positions: rangePositions(len(needle))} | |
| 39 | 40 | default: | |
| 40 | - | positions, ok := SubsequencePositions([]rune(text), needle) | |
| 41 | + | positions, ok := subsequencePositions([]rune(text), needle) | |
| 41 | 42 | if !ok { | |
| 42 | 43 | return Score{}, false | |
| 43 | 44 | } | |
| @@ -60,8 +61,8 @@ func Less(leftRank int, left Score, rightRank int, right Score) bool { | |||
| 60 | 61 | return left.First < right.First | |
| 61 | 62 | } | |
| 62 | 63 | ||
| 63 | - | // Range returns the positions 0..n-1. | |
| 64 | - | func Range(n int) []int { | |
| 64 | + | // rangePositions returns the positions 0..n-1. | |
| 65 | + | func rangePositions(n int) []int { | |
| 65 | 66 | positions := make([]int, n) | |
| 66 | 67 | for i := range positions { | |
| 67 | 68 | positions[i] = i | |
| @@ -69,9 +70,9 @@ func Range(n int) []int { | |||
| 69 | 70 | return positions | |
| 70 | 71 | } | |
| 71 | 72 | ||
| 72 | - | // SubsequencePositions returns the first in-order positions of needle in | |
| 73 | + | // subsequencePositions returns the first in-order positions of needle in | |
| 73 | 74 | // haystack. | |
| 74 | - | func SubsequencePositions(haystack, needle []rune) ([]int, bool) { | |
| 75 | + | func subsequencePositions(haystack, needle []rune) ([]int, bool) { | |
| 75 | 76 | if len(needle) == 0 { | |
| 76 | 77 | return nil, true | |
| 77 | 78 | } | |
| @@ -9,6 +9,7 @@ import "strings" | |||||
| 9 | // Kind is how a field matched the query. | 9 | // Kind is how a field matched the query. | ||
| 10 | type Kind uint8 | 10 | type Kind uint8 | ||
| 11 | 11 | ||||
| 12 | + | // Kinds are ordered best first, so a caller may use the Kind itself as a rank. | |||
| 12 | const ( | 13 | const ( | ||
| 13 | Exact Kind = iota | 14 | Exact Kind = iota | ||
| 14 | Prefix | 15 | Prefix | ||
| @@ -33,11 +34,11 @@ func Match(text, query string, needle []rune) (Score, bool) { | |||||
| 33 | var score Score | 34 | var score Score | ||
| 34 | switch { | 35 | switch { | ||
| 35 | case text == query: | 36 | case text == query: | ||
| 36 | - | score = Score{Kind: Exact, Positions: Range(len(needle))} | 37 | + | score = Score{Kind: Exact, Positions: rangePositions(len(needle))} |
| 37 | case strings.HasPrefix(text, query): | 38 | case strings.HasPrefix(text, query): | ||
| 38 | - | score = Score{Kind: Prefix, Positions: Range(len(needle))} | 39 | + | score = Score{Kind: Prefix, Positions: rangePositions(len(needle))} |
| 39 | default: | 40 | default: | ||
| 40 | - | positions, ok := SubsequencePositions([]rune(text), needle) | 41 | + | positions, ok := subsequencePositions([]rune(text), needle) |
| 41 | if !ok { | 42 | if !ok { | ||
| 42 | return Score{}, false | 43 | return Score{}, false | ||
| 43 | } | 44 | } | ||
| @@ -60,8 +61,8 @@ func Less(leftRank int, left Score, rightRank int, right Score) bool { | |||||
| 60 | return left.First < right.First | 61 | return left.First < right.First | ||
| 61 | } | 62 | } | ||
| 62 | 63 | ||||
| 63 | - | // Range returns the positions 0..n-1. | 64 | + | // rangePositions returns the positions 0..n-1. |
| 64 | - | func Range(n int) []int { | 65 | + | func rangePositions(n int) []int { |
| 65 | positions := make([]int, n) | 66 | positions := make([]int, n) | ||
| 66 | for i := range positions { | 67 | for i := range positions { | ||
| 67 | positions[i] = i | 68 | positions[i] = i | ||
| @@ -69,9 +70,9 @@ func Range(n int) []int { | |||||
| 69 | return positions | 70 | return positions | ||
| 70 | } | 71 | } | ||
| 71 | 72 | ||||
| 72 | - | // SubsequencePositions returns the first in-order positions of needle in | 73 | + | // subsequencePositions returns the first in-order positions of needle in |
| 73 | // haystack. | 74 | // haystack. | ||
| 74 | - | func SubsequencePositions(haystack, needle []rune) ([]int, bool) { | 75 | + | func subsequencePositions(haystack, needle []rune) ([]int, bool) { |
| 75 | if len(needle) == 0 { | 76 | if len(needle) == 0 { | ||
| 76 | return nil, true | 77 | return nil, true | ||
| 77 | } | 78 | } | ||
M internal/usecase/fuzzy/fuzzy_test.go +8 -0
| @@ -19,6 +19,7 @@ func TestMatch(t *testing.T) { | |||
| 19 | 19 | {name: "prefix", text: "workspace", query: "wor", want: Score{Kind: Prefix, Positions: []int{0, 1, 2}, Span: 3}, ok: true}, | |
| 20 | 20 | {name: "subsequence", text: "a-b-c", query: "bc", want: Score{Kind: Subsequence, Positions: []int{2, 4}, Span: 3, First: 2}, ok: true}, | |
| 21 | 21 | {name: "runes not bytes", text: "été-x", query: "x", want: Score{Kind: Subsequence, Positions: []int{4}, Span: 1, First: 4}, ok: true}, | |
| 22 | + | {name: "multi-byte prefix counts runes", text: "été", query: "ét", want: Score{Kind: Prefix, Positions: []int{0, 1}, Span: 2}, ok: true}, | |
| 22 | 23 | {name: "no match", text: "work", query: "z", ok: false}, | |
| 23 | 24 | {name: "out of order", text: "ab", query: "ba", ok: false}, | |
| 24 | 25 | } | |
| @@ -31,6 +32,13 @@ func TestMatch(t *testing.T) { | |||
| 31 | 32 | } | |
| 32 | 33 | } | |
| 33 | 34 | ||
| 35 | + | // TestKindOrder pins the best-first order callers rely on to use a Kind as a | |
| 36 | + | // rank. | |
| 37 | + | func TestKindOrder(t *testing.T) { | |
| 38 | + | require.Less(t, Exact, Prefix) | |
| 39 | + | require.Less(t, Prefix, Subsequence) | |
| 40 | + | } | |
| 41 | + | ||
| 34 | 42 | func TestLess(t *testing.T) { | |
| 35 | 43 | tests := []struct { | |
| 36 | 44 | name string | |
| @@ -19,6 +19,7 @@ func TestMatch(t *testing.T) { | |||||
| 19 | {name: "prefix", text: "workspace", query: "wor", want: Score{Kind: Prefix, Positions: []int{0, 1, 2}, Span: 3}, ok: true}, | 19 | {name: "prefix", text: "workspace", query: "wor", want: Score{Kind: Prefix, Positions: []int{0, 1, 2}, Span: 3}, ok: true}, | ||
| 20 | {name: "subsequence", text: "a-b-c", query: "bc", want: Score{Kind: Subsequence, Positions: []int{2, 4}, Span: 3, First: 2}, ok: true}, | 20 | {name: "subsequence", text: "a-b-c", query: "bc", want: Score{Kind: Subsequence, Positions: []int{2, 4}, Span: 3, First: 2}, ok: true}, | ||
| 21 | {name: "runes not bytes", text: "été-x", query: "x", want: Score{Kind: Subsequence, Positions: []int{4}, Span: 1, First: 4}, ok: true}, | 21 | {name: "runes not bytes", text: "été-x", query: "x", want: Score{Kind: Subsequence, Positions: []int{4}, Span: 1, First: 4}, ok: true}, | ||
| 22 | + | {name: "multi-byte prefix counts runes", text: "été", query: "ét", want: Score{Kind: Prefix, Positions: []int{0, 1}, Span: 2}, ok: true}, | |||
| 22 | {name: "no match", text: "work", query: "z", ok: false}, | 23 | {name: "no match", text: "work", query: "z", ok: false}, | ||
| 23 | {name: "out of order", text: "ab", query: "ba", ok: false}, | 24 | {name: "out of order", text: "ab", query: "ba", ok: false}, | ||
| 24 | } | 25 | } | ||
| @@ -31,6 +32,13 @@ func TestMatch(t *testing.T) { | |||||
| 31 | } | 32 | } | ||
| 32 | } | 33 | } | ||
| 33 | 34 | ||||
| 35 | + | // TestKindOrder pins the best-first order callers rely on to use a Kind as a | |||
| 36 | + | // rank. | |||
| 37 | + | func TestKindOrder(t *testing.T) { | |||
| 38 | + | require.Less(t, Exact, Prefix) | |||
| 39 | + | require.Less(t, Prefix, Subsequence) | |||
| 40 | + | } | |||
| 41 | + | ||||
| 34 | func TestLess(t *testing.T) { | 42 | func TestLess(t *testing.T) { | ||
| 35 | tests := []struct { | 43 | tests := []struct { | ||
| 36 | name string | 44 | name string | ||
M internal/usecase/palette/fuzzy.go +6 -10
| @@ -38,14 +38,8 @@ func Fuzzy(results []Result, query string) []Match { | |||
| 38 | 38 | } | |
| 39 | 39 | sort.SliceStable(out, func(i, j int) bool { | |
| 40 | 40 | a, b := out[i], out[j] | |
| 41 | - | if a.rank != b.rank { | |
| 42 | - | return a.rank < b.rank | |
| 43 | - | } | |
| 44 | - | if a.span != b.span { | |
| 45 | - | return a.span < b.span | |
| 46 | - | } | |
| 47 | - | if a.first != b.first { | |
| 48 | - | return a.first < b.first | |
| 41 | + | if a.rank != b.rank || a.span != b.span || a.first != b.first { | |
| 42 | + | return fuzzy.Less(a.rank, fuzzy.Score{Span: a.span, First: a.first}, b.rank, fuzzy.Score{Span: b.span, First: b.first}) | |
| 49 | 43 | } | |
| 50 | 44 | if a.Result.Kind() != b.Result.Kind() { | |
| 51 | 45 | return a.Result.Kind() < b.Result.Kind() | |
| @@ -86,8 +80,10 @@ func score(result Result, needle string, needleRunes []rune, order int) (Match, | |||
| 86 | 80 | return match, true | |
| 87 | 81 | } | |
| 88 | 82 | if cmd, ok := result.Command(); ok { | |
| 89 | - | if positions, ok := fuzzy.SubsequencePositions([]rune(strings.ToLower(cmd.Desc)), needleRunes); ok { | |
| 90 | - | match.rank, match.span, match.first = 5, positions[len(positions)-1]-positions[0]+1, positions[0] | |
| 83 | + | // Description matches only rank and never highlight, so Positions | |
| 84 | + | // stays empty. | |
| 85 | + | if matched, ok := fuzzy.Match(strings.ToLower(cmd.Desc), needle, needleRunes); ok { | |
| 86 | + | match.rank, match.span, match.first = 5, matched.Span, matched.First | |
| 91 | 87 | return match, true | |
| 92 | 88 | } | |
| 93 | 89 | } | |
| @@ -38,14 +38,8 @@ func Fuzzy(results []Result, query string) []Match { | |||||
| 38 | } | 38 | } | ||
| 39 | sort.SliceStable(out, func(i, j int) bool { | 39 | sort.SliceStable(out, func(i, j int) bool { | ||
| 40 | a, b := out[i], out[j] | 40 | a, b := out[i], out[j] | ||
| 41 | - | if a.rank != b.rank { | 41 | + | if a.rank != b.rank || a.span != b.span || a.first != b.first { |
| 42 | - | return a.rank < b.rank | 42 | + | return fuzzy.Less(a.rank, fuzzy.Score{Span: a.span, First: a.first}, b.rank, fuzzy.Score{Span: b.span, First: b.first}) |
| 43 | - | } | |||
| 44 | - | if a.span != b.span { | |||
| 45 | - | return a.span < b.span | |||
| 46 | - | } | |||
| 47 | - | if a.first != b.first { | |||
| 48 | - | return a.first < b.first | |||
| 49 | } | 43 | } | ||
| 50 | if a.Result.Kind() != b.Result.Kind() { | 44 | if a.Result.Kind() != b.Result.Kind() { | ||
| 51 | return a.Result.Kind() < b.Result.Kind() | 45 | return a.Result.Kind() < b.Result.Kind() | ||
| @@ -86,8 +80,10 @@ func score(result Result, needle string, needleRunes []rune, order int) (Match, | |||||
| 86 | return match, true | 80 | return match, true | ||
| 87 | } | 81 | } | ||
| 88 | if cmd, ok := result.Command(); ok { | 82 | if cmd, ok := result.Command(); ok { | ||
| 89 | - | if positions, ok := fuzzy.SubsequencePositions([]rune(strings.ToLower(cmd.Desc)), needleRunes); ok { | 83 | + | // Description matches only rank and never highlight, so Positions |
| 90 | - | match.rank, match.span, match.first = 5, positions[len(positions)-1]-positions[0]+1, positions[0] | 84 | + | // stays empty. |
| 85 | + | if matched, ok := fuzzy.Match(strings.ToLower(cmd.Desc), needle, needleRunes); ok { | |||
| 86 | + | match.rank, match.span, match.first = 5, matched.Span, matched.First | |||
| 91 | return match, true | 87 | return match, true | ||
| 92 | } | 88 | } | ||
| 93 | } | 89 | } | ||