From a15879625cad3035fcaca6989e1b767e0bad7d8a Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:12:30 -0700 Subject: [PATCH 01/11] feat(carport,config): add binary field to DriverInstance and DriverInstanceConfig proto Co-Authored-By: Claude Sonnet 4.6 --- gen/gohome/config/v1/snapshot.pb.go | 13 +++++++++++-- internal/config/evaluator.go | 2 ++ internal/config/pkl/gohome/carport.pkl | 2 +- proto/gohome/config/v1/snapshot.proto | 1 + 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/gen/gohome/config/v1/snapshot.pb.go b/gen/gohome/config/v1/snapshot.pb.go index b2ead41..5afad30 100644 --- a/gen/gohome/config/v1/snapshot.pb.go +++ b/gen/gohome/config/v1/snapshot.pb.go @@ -285,6 +285,7 @@ type DriverInstanceConfig struct { DriverName string `protobuf:"bytes,2,opt,name=driver_name,json=driverName,proto3" json:"driver_name,omitempty"` ConfigHash []byte `protobuf:"bytes,3,opt,name=config_hash,json=configHash,proto3" json:"config_hash,omitempty"` Params []byte `protobuf:"bytes,4,opt,name=params,proto3" json:"params,omitempty"` + Binary string `protobuf:"bytes,5,opt,name=binary,proto3" json:"binary,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -347,6 +348,13 @@ func (x *DriverInstanceConfig) GetParams() []byte { return nil } +func (x *DriverInstanceConfig) GetBinary() string { + if x != nil { + return x.Binary + } + return "" +} + type EntityConfig struct { state protoimpl.MessageState `protogen:"open.v1"` Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` @@ -3029,14 +3037,15 @@ const file_gohome_config_v1_snapshot_proto_rawDesc = "" + "\ascripts\x18\x11 \x03(\v2\x1e.gohome.config.v1.ScriptConfigR\ascripts\x12<\n" + "\blistener\x18\x14 \x01(\v2 .gohome.config.v1.ListenerConfigR\blistener\x12-\n" + "\x03mcp\x18\x15 \x01(\v2\x1b.gohome.config.v1.MCPConfigR\x03mcp\x12I\n" + - "\rauth_settings\x18\x16 \x01(\v2$.gohome.config.v1.AuthSettingsConfigR\fauthSettings\"\x80\x01\n" + + "\rauth_settings\x18\x16 \x01(\v2$.gohome.config.v1.AuthSettingsConfigR\fauthSettings\"\x98\x01\n" + "\x14DriverInstanceConfig\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12\x1f\n" + "\vdriver_name\x18\x02 \x01(\tR\n" + "driverName\x12\x1f\n" + "\vconfig_hash\x18\x03 \x01(\fR\n" + "configHash\x12\x16\n" + - "\x06params\x18\x04 \x01(\fR\x06params\"x\n" + + "\x06params\x18\x04 \x01(\fR\x06params\x12\x16\n" + + "\x06binary\x18\x05 \x01(\tR\x06binary\"x\n" + "\fEntityConfig\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12#\n" + "\rfriendly_name\x18\x02 \x01(\tR\ffriendlyName\x12\x1f\n" + diff --git a/internal/config/evaluator.go b/internal/config/evaluator.go index 5eb6d06..d3dd1b6 100644 --- a/internal/config/evaluator.go +++ b/internal/config/evaluator.go @@ -310,6 +310,7 @@ func parseConfigJSON(text, configDir string) (*configpb.ConfigSnapshot, error) { var base struct { ID string `json:"id"` DriverName string `json:"driverName"` + Binary string `json:"binary"` } if err := json.Unmarshal(rawInst, &base); err != nil { return nil, fmt.Errorf("parse driver instance: %w", err) @@ -318,6 +319,7 @@ func parseConfigJSON(text, configDir string) (*configpb.ConfigSnapshot, error) { snap.DriverInstances = append(snap.DriverInstances, &configpb.DriverInstanceConfig{ Id: base.ID, DriverName: base.DriverName, + Binary: base.Binary, ConfigHash: h[:], Params: rawInst, }) diff --git a/internal/config/pkl/gohome/carport.pkl b/internal/config/pkl/gohome/carport.pkl index 4102cd5..3f105a5 100644 --- a/internal/config/pkl/gohome/carport.pkl +++ b/internal/config/pkl/gohome/carport.pkl @@ -2,8 +2,8 @@ module gohome.carport // DriverInstance is the base class for all driver instance configs. // Driver authors extend this with their own typed fields. -// The pkl_module field in DriverManifest is reserved for C5+ (DR-7). abstract class DriverInstance { id: String(!isEmpty) driverName: String(!isEmpty) + binary: String(!isEmpty) } diff --git a/proto/gohome/config/v1/snapshot.proto b/proto/gohome/config/v1/snapshot.proto index ea7954e..df82862 100644 --- a/proto/gohome/config/v1/snapshot.proto +++ b/proto/gohome/config/v1/snapshot.proto @@ -30,6 +30,7 @@ message DriverInstanceConfig { string driver_name = 2; bytes config_hash = 3; bytes params = 4; + string binary = 5; } message EntityConfig { From 819b4a015000d4fd35f53585ce2c75bdd162fb13 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:15:18 -0700 Subject: [PATCH 02/11] fix(config): add binary field to test fixture after schema update --- internal/config/testdata/valid/main.pkl | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/testdata/valid/main.pkl b/internal/config/testdata/valid/main.pkl index 7faa3ed..aed3ed9 100644 --- a/internal/config/testdata/valid/main.pkl +++ b/internal/config/testdata/valid/main.pkl @@ -9,6 +9,7 @@ driverInstances { new FakeDriverInstance { id = "fake-main" driverName = "fake" + binary = "/usr/local/bin/fake-driver" } } From ec36861fcdf089a270c6dd2ba010a3da4d52cb01 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:17:00 -0700 Subject: [PATCH 03/11] feat(config): add binary param to CarportManager.RegisterInstance Co-Authored-By: Claude Sonnet 4.6 --- internal/config/manager.go | 6 +++--- internal/config/manager_test.go | 2 +- internal/daemon/daemon.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/config/manager.go b/internal/config/manager.go index ce82d86..0be6aef 100644 --- a/internal/config/manager.go +++ b/internal/config/manager.go @@ -15,7 +15,7 @@ import ( // For C4, the daemon passes a no-op implementation; dynamic carport management // will be wired when carport.Host gains RegisterInstance/UnregisterInstance methods. type CarportManager interface { - RegisterInstance(ctx context.Context, id, driverName string, params []byte) error + RegisterInstance(ctx context.Context, id, driverName, binary string, params []byte) error UnregisterInstance(ctx context.Context, id string) error } @@ -104,7 +104,7 @@ func (m *Manager) Apply(ctx context.Context, dryRun bool) error { } for _, id := range diff.DriverInstancesAdded { di := findInstance(snap, id) - if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetParams()); err != nil { + if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetBinary(), di.GetParams()); err != nil { return fmt.Errorf("register %q: %w", id, err) } } @@ -113,7 +113,7 @@ func (m *Manager) Apply(ctx context.Context, dryRun bool) error { if err := m.carportMgr.UnregisterInstance(ctx, id); err != nil { return fmt.Errorf("unregister changed %q: %w", id, err) } - if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetParams()); err != nil { + if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetBinary(), di.GetParams()); err != nil { return fmt.Errorf("re-register changed %q: %w", id, err) } } diff --git a/internal/config/manager_test.go b/internal/config/manager_test.go index 5bd6bb8..2fd1f13 100644 --- a/internal/config/manager_test.go +++ b/internal/config/manager_test.go @@ -14,7 +14,7 @@ type fakeCarport struct { unregistered []string } -func (f *fakeCarport) RegisterInstance(_ context.Context, id, _ string, _ []byte) error { +func (f *fakeCarport) RegisterInstance(_ context.Context, id, _, _ string, _ []byte) error { f.registered = append(f.registered, id) return nil } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 35daa96..6287327 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -604,7 +604,7 @@ func (a *carportAdapter) Dispatch(ctx context.Context, entityID, capability stri // RegisterInstance/UnregisterInstance methods (C5+). type nopCarportManager struct{} -func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _ string, _ []byte) error { +func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _, _ string, _ []byte) error { return nil } func (n *nopCarportManager) UnregisterInstance(_ context.Context, _ string) error { From 4cb66b10f413c4a4e46312ac9394c836709ef008 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:19:49 -0700 Subject: [PATCH 04/11] refactor(carport): extract defaultLifecycleConfig helper --- internal/carport/config.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/carport/config.go b/internal/carport/config.go index 31f23be..b8208aa 100644 --- a/internal/carport/config.go +++ b/internal/carport/config.go @@ -134,3 +134,19 @@ func intd(v, def int) int { } return v } + +// defaultLifecycleConfig returns the defaults used for dynamically registered +// instances (those coming from main.pkl rather than drivers.toml). +func defaultLifecycleConfig() LifecycleConfig { + return LifecycleConfig{ + HandshakeDeadline: 5 * time.Second, + HealthProbeInterval: 15 * time.Second, + HealthProbeTimeout: 3 * time.Second, + HealthFailuresToRestart: 3, + ShutdownGrace: 10 * time.Second, + RestartBackoffInitial: time.Second, + RestartBackoffMax: 60 * time.Second, + RestartBudgetWindow: 10 * time.Minute, + RestartBudgetMax: 10, + } +} From b77f6a622eaabe09f0ac21ee4d8555de91a05951 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:21:51 -0700 Subject: [PATCH 05/11] feat(carport): implement RegisterInstance and UnregisterInstance on Host Add ctx field to Host struct, store it in Start, and implement RegisterInstance and UnregisterInstance for dynamic driver lifecycle management. Includes TDD tests covering not-started, duplicate ID, stopped-host, and not-found error paths. Co-Authored-By: Claude Sonnet 4.6 --- internal/carport/carport.go | 53 ++++++++++++++++++++++++++++ internal/carport/dynamic_test.go | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 internal/carport/dynamic_test.go diff --git a/internal/carport/carport.go b/internal/carport/carport.go index ebdd910..dfd527d 100644 --- a/internal/carport/carport.go +++ b/internal/carport/carport.go @@ -8,6 +8,7 @@ package carport import ( "context" "database/sql" + "fmt" "log/slog" "sync" @@ -40,6 +41,8 @@ type Host struct { logger *slog.Logger metrics *observability.Metrics + ctx context.Context // root context for lifecycle goroutines; set by Start + mu sync.RWMutex instances map[string]*managedInstance // keyed by Instance.ID @@ -104,6 +107,7 @@ func New(cfg HostConfig, db *sql.DB, store *eventstore.Store, reg *registry.Regi // spawn/handshake are reported through DriverEvents in the event log, not // returned from this call. func (h *Host) Start(ctx context.Context) error { + h.ctx = ctx for _, inst := range h.cfgData.Instances { if !inst.Enabled { continue @@ -135,4 +139,53 @@ func (h *Host) Stop(ctx context.Context) { }) } +// RegisterInstance adds a new driver instance and begins its lifecycle goroutine. +// Returns an error if an instance with that ID is already registered, if the host +// has not been started, or if the host has been stopped. +// IDs must not conflict with instances already running from drivers.toml. +func (h *Host) RegisterInstance(_ context.Context, id, driverName, binary string, params []byte) error { + if h.ctx == nil { + return fmt.Errorf("carport host not started") + } + select { + case <-h.stopped: + return fmt.Errorf("carport host is stopped") + default: + } + h.mu.Lock() + if _, exists := h.instances[id]; exists { + h.mu.Unlock() + return fmt.Errorf("instance %q already registered", id) + } + inst := Instance{ + ID: id, + Binary: binary, + Enabled: true, + ConfigJSON: params, + Lifecycle: defaultLifecycleConfig(), + } + m := &managedInstance{cfg: inst, state: StateDeclared} + h.instances[id] = m + h.mu.Unlock() + h.launchLifecycle(h.ctx, m) + return nil +} + +// UnregisterInstance stops and removes a driver instance by ID. +// Returns an error if the instance is not found. +func (h *Host) UnregisterInstance(_ context.Context, id string) error { + h.mu.Lock() + m, exists := h.instances[id] + if !exists { + h.mu.Unlock() + return fmt.Errorf("instance %q not found", id) + } + delete(h.instances, id) + h.mu.Unlock() + // Use Background context so shutdown isn't cut short by the caller's deadline. + // Shutdown duration is bounded by m.cfg.Lifecycle.ShutdownGrace. + h.shutdownInstance(context.Background(), m) + return nil +} + // launchLifecycle and shutdownInstance are implemented in supervisor.go. diff --git a/internal/carport/dynamic_test.go b/internal/carport/dynamic_test.go new file mode 100644 index 0000000..ee71bcf --- /dev/null +++ b/internal/carport/dynamic_test.go @@ -0,0 +1,59 @@ +package carport + +import ( + "context" + "testing" +) + +func TestRegisterInstance_HostNotStarted(t *testing.T) { + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: make(chan struct{}), + // ctx is nil — host not started + } + err := h.RegisterInstance(context.Background(), "new", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error when host not started (ctx nil)") + } +} + +func TestRegisterInstance_DuplicateID(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + h := &Host{ + instances: map[string]*managedInstance{ + "existing": {cfg: Instance{ID: "existing"}}, + }, + stopped: make(chan struct{}), + ctx: ctx, + } + err := h.RegisterInstance(context.Background(), "existing", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error for duplicate instance ID") + } +} + +func TestRegisterInstance_HostStopped(t *testing.T) { + stopped := make(chan struct{}) + close(stopped) // host is already stopped + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: stopped, + ctx: context.Background(), + } + err := h.RegisterInstance(context.Background(), "new", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error when host is stopped") + } +} + +func TestUnregisterInstance_NotFound(t *testing.T) { + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: make(chan struct{}), + } + err := h.UnregisterInstance(context.Background(), "missing") + if err == nil { + t.Fatal("expected error for missing instance") + } +} From 8e1fe6cb9e1fe3ed97628c62129cb36db8e0f7da Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:31:20 -0700 Subject: [PATCH 06/11] feat(daemon): wire carport.Host as real CarportManager, remove nop stub --- internal/daemon/daemon.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 6287327..682e0a4 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -45,6 +45,9 @@ import ( "github.com/fdatoo/gohome/internal/web" ) +// Compile-time assertion: *carport.Host must satisfy config.CarportManager. +var _ config.CarportManager = (*carport.Host)(nil) + type Daemon struct { cfg Config logger *slog.Logger @@ -217,7 +220,7 @@ func (d *Daemon) Run(ctx context.Context) error { mainPkl := filepath.Join(configDir, "main.pkl") switch _, statErr := os.Stat(mainPkl); { case statErr == nil: - cfgMgr, err := config.NewManager(ctx, configDir, d.store, &nopCarportManager{}) + cfgMgr, err := config.NewManager(ctx, configDir, d.store, d.carport) if err != nil { return fmt.Errorf("config manager: %w", err) } @@ -600,13 +603,3 @@ func (a *carportAdapter) Dispatch(ctx context.Context, entityID, capability stri return &starlark.DispatchResult{Ok: res.GetOk(), Error: res.GetErrorMessage()}, nil } -// nopCarportManager satisfies config.CarportManager until carport.Host gains -// RegisterInstance/UnregisterInstance methods (C5+). -type nopCarportManager struct{} - -func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _, _ string, _ []byte) error { - return nil -} -func (n *nopCarportManager) UnregisterInstance(_ context.Context, _ string) error { - return nil -} From 601b07971f676862fcd2c0cd7d099efc2851b358 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:35:52 -0700 Subject: [PATCH 07/11] =?UTF-8?q?feat(carport):=20remove=20drivers.toml=20?= =?UTF-8?q?=E2=80=94=20main.pkl=20is=20now=20the=20sole=20driver=20config?= =?UTF-8?q?=20source?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- cmd/gohomed/main.go | 2 - internal/carport/carport.go | 46 ++------ internal/carport/config.go | 122 +++----------------- internal/carport/config_test.go | 154 -------------------------- internal/carport/coverage_test.go | 154 +------------------------- internal/carport/test_helpers_test.go | 6 +- internal/daemon/config.go | 4 - internal/daemon/daemon.go | 7 +- 8 files changed, 28 insertions(+), 467 deletions(-) delete mode 100644 internal/carport/config_test.go diff --git a/cmd/gohomed/main.go b/cmd/gohomed/main.go index 5f3595a..2f86504 100644 --- a/cmd/gohomed/main.go +++ b/cmd/gohomed/main.go @@ -27,7 +27,6 @@ func run() int { adminPort = flag.Int("admin-port", 9190, "HTTP admin port for /metrics and /health") snapshotEveryEvt = flag.Int("snapshot-every-events", 10_000, "snapshot cadence: events since last") snapshotEveryDur = flag.Duration("snapshot-every-period", time.Hour, "snapshot cadence: wall-clock period") - driversTOML = flag.String("drivers-toml", "", "path to drivers.toml (default /drivers.toml)") configDir = flag.String("config-dir", "", "config directory with main.pkl (default /config)") ) flag.Parse() @@ -56,7 +55,6 @@ func run() int { AdminPort: *adminPort, SnapshotEveryEvents: *snapshotEveryEvt, SnapshotEveryPeriod: *snapshotEveryDur, - DriversTOMLPath: *driversTOML, ConfigDir: *configDir, } d := daemon.New(cfg, logger, metrics) diff --git a/internal/carport/carport.go b/internal/carport/carport.go index dfd527d..b40a319 100644 --- a/internal/carport/carport.go +++ b/internal/carport/carport.go @@ -1,6 +1,6 @@ -// Package carport hosts the driver-supervisor subsystem: drivers.toml -// configuration, per-instance subprocess lifecycle, command dispatch, and -// event ingest from drivers over the Carport gRPC protocol (v1alpha1). +// Package carport hosts the driver-supervisor subsystem: per-instance subprocess +// lifecycle, command dispatch, and event ingest from drivers over the Carport +// gRPC protocol (v1alpha1). Driver instances are declared via main.pkl. // // See docs/superpowers/specs/2026-04-21-c2-carport-protocol-design.md. package carport @@ -19,21 +19,14 @@ import ( // HostConfig is the daemon-level configuration handed to New. type HostConfig struct { - // DriversTOMLPath is the absolute path to drivers.toml. Empty or missing - // file → zero configured instances (host is a no-op but still starts). - DriversTOMLPath string - - // SocketDir is where per-instance UDS files are created. Defaults to - // /carport/ in the daemon wiring (T18); required here if - // any instance is to be spawned. + // SocketDir is where per-instance UDS files are created. SocketDir string } // Host is the public face of the carport subsystem. Instances are spawned // and supervised on Start; Stop shuts them down gracefully. type Host struct { - cfg HostConfig - cfgData *Config + cfg HostConfig db *sql.DB store *eventstore.Store @@ -78,20 +71,9 @@ type managedInstance struct { } // New constructs a Host. The host is inert until Start is called. -// Passing an empty DriversTOMLPath OR a path that does not exist yields a -// Host with zero instances; Start is then a no-op. func New(cfg HostConfig, db *sql.DB, store *eventstore.Store, reg *registry.Registry, logger *slog.Logger, metrics *observability.Metrics) (*Host, error) { - cfgData := &Config{} - if cfg.DriversTOMLPath != "" { - loaded, err := LoadConfig(cfg.DriversTOMLPath) - if err != nil { - return nil, err - } - cfgData = loaded - } return &Host{ cfg: cfg, - cfgData: cfgData, db: db, store: store, router: NewRouter(reg), @@ -102,22 +84,11 @@ func New(cfg HostConfig, db *sql.DB, store *eventstore.Store, reg *registry.Regi }, nil } -// Start launches lifecycle goroutines for each enabled instance. Non-blocking. -// After Start returns, supervision goroutines are running; errors during -// spawn/handshake are reported through DriverEvents in the event log, not -// returned from this call. +// Start initialises the host's root context. Non-blocking. +// Driver instances are added dynamically via RegisterInstance (driven by +// main.pkl evaluation in the config manager). func (h *Host) Start(ctx context.Context) error { h.ctx = ctx - for _, inst := range h.cfgData.Instances { - if !inst.Enabled { - continue - } - m := &managedInstance{cfg: inst, state: StateDeclared} - h.mu.Lock() - h.instances[inst.ID] = m - h.mu.Unlock() - h.launchLifecycle(ctx, m) - } return nil } @@ -142,7 +113,6 @@ func (h *Host) Stop(ctx context.Context) { // RegisterInstance adds a new driver instance and begins its lifecycle goroutine. // Returns an error if an instance with that ID is already registered, if the host // has not been started, or if the host has been stopped. -// IDs must not conflict with instances already running from drivers.toml. func (h *Host) RegisterInstance(_ context.Context, id, driverName, binary string, params []byte) error { if h.ctx == nil { return fmt.Errorf("carport host not started") diff --git a/internal/carport/config.go b/internal/carport/config.go index b8208aa..ac8800d 100644 --- a/internal/carport/config.go +++ b/internal/carport/config.go @@ -1,13 +1,7 @@ package carport import ( - "errors" - "fmt" - "os" - "regexp" "time" - - "github.com/BurntSushi/toml" ) // LifecycleConfig tunes per-instance timing and restart policy. @@ -24,7 +18,7 @@ type LifecycleConfig struct { RestartBudgetMax int } -// Instance is one entry in drivers.toml. +// Instance is a single driver instance declaration. type Instance struct { ID string Binary string @@ -33,92 +27,20 @@ type Instance struct { Lifecycle LifecycleConfig } -// Config is the parsed drivers.toml. -type Config struct { - Instances []Instance -} - -var idRE = regexp.MustCompile(`^[a-z0-9_\-]{1,64}$`) - -// LoadConfig reads drivers.toml at path. A missing file yields an empty Config. -// An unreadable/invalid file is an error. Every entry is validated per §5.2 of -// the C2 design doc. -func LoadConfig(path string) (*Config, error) { - data, err := os.ReadFile(path) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return &Config{}, nil - } - return nil, fmt.Errorf("read drivers.toml: %w", err) - } - - var raw struct { - Instance []struct { - ID string `toml:"id"` - Binary string `toml:"binary"` - Enabled *bool `toml:"enabled"` - ConfigJSON string `toml:"config_json"` - Lifecycle struct { - HandshakeDeadlineMs int `toml:"handshake_deadline_ms"` - HealthProbeIntervalMs int `toml:"health_probe_interval_ms"` - HealthProbeTimeoutMs int `toml:"health_probe_timeout_ms"` - HealthFailuresToRestart int `toml:"health_failures_to_restart"` - ShutdownGraceMs int `toml:"shutdown_grace_ms"` - RestartBackoffInitialMs int `toml:"restart_backoff_initial_ms"` - RestartBackoffMaxMs int `toml:"restart_backoff_max_ms"` - RestartBudgetWindowMin int `toml:"restart_budget_window_minutes"` - RestartBudgetMax int `toml:"restart_budget_max"` - } `toml:"lifecycle"` - } `toml:"instance"` - } - if err := toml.Unmarshal(data, &raw); err != nil { - return nil, fmt.Errorf("parse drivers.toml: %w", err) - } - - seen := map[string]bool{} - cfg := &Config{} - for i, r := range raw.Instance { - if !idRE.MatchString(r.ID) { - return nil, fmt.Errorf("instance[%d]: invalid id %q (want [a-z0-9_-]{1,64})", i, r.ID) - } - if seen[r.ID] { - return nil, fmt.Errorf("duplicate instance id %q", r.ID) - } - seen[r.ID] = true - if r.Binary == "" { - return nil, fmt.Errorf("instance %q: binary is required", r.ID) - } - info, err := os.Stat(r.Binary) - if err != nil { - return nil, fmt.Errorf("instance %q: binary %q: %w", r.ID, r.Binary, err) - } - if info.Mode()&0o111 == 0 { - return nil, fmt.Errorf("instance %q: binary %q is not executable", r.ID, r.Binary) - } - enabled := true - if r.Enabled != nil { - enabled = *r.Enabled - } - lc := LifecycleConfig{ - HandshakeDeadline: dur(r.Lifecycle.HandshakeDeadlineMs, 5*time.Second), - HealthProbeInterval: dur(r.Lifecycle.HealthProbeIntervalMs, 15*time.Second), - HealthProbeTimeout: dur(r.Lifecycle.HealthProbeTimeoutMs, 3*time.Second), - HealthFailuresToRestart: intd(r.Lifecycle.HealthFailuresToRestart, 3), - ShutdownGrace: dur(r.Lifecycle.ShutdownGraceMs, 10*time.Second), - RestartBackoffInitial: dur(r.Lifecycle.RestartBackoffInitialMs, 1*time.Second), - RestartBackoffMax: dur(r.Lifecycle.RestartBackoffMaxMs, 60*time.Second), - RestartBudgetWindow: time.Duration(intd(r.Lifecycle.RestartBudgetWindowMin, 10)) * time.Minute, - RestartBudgetMax: intd(r.Lifecycle.RestartBudgetMax, 10), - } - cfg.Instances = append(cfg.Instances, Instance{ - ID: r.ID, - Binary: r.Binary, - Enabled: enabled, - ConfigJSON: []byte(r.ConfigJSON), - Lifecycle: lc, - }) +// defaultLifecycleConfig returns the defaults used for dynamically registered +// instances (those coming from main.pkl rather than drivers.toml). +func defaultLifecycleConfig() LifecycleConfig { + return LifecycleConfig{ + HandshakeDeadline: 5 * time.Second, + HealthProbeInterval: 15 * time.Second, + HealthProbeTimeout: 3 * time.Second, + HealthFailuresToRestart: 3, + ShutdownGrace: 10 * time.Second, + RestartBackoffInitial: time.Second, + RestartBackoffMax: 60 * time.Second, + RestartBudgetWindow: 10 * time.Minute, + RestartBudgetMax: 10, } - return cfg, nil } func dur(ms int, def time.Duration) time.Duration { @@ -134,19 +56,3 @@ func intd(v, def int) int { } return v } - -// defaultLifecycleConfig returns the defaults used for dynamically registered -// instances (those coming from main.pkl rather than drivers.toml). -func defaultLifecycleConfig() LifecycleConfig { - return LifecycleConfig{ - HandshakeDeadline: 5 * time.Second, - HealthProbeInterval: 15 * time.Second, - HealthProbeTimeout: 3 * time.Second, - HealthFailuresToRestart: 3, - ShutdownGrace: 10 * time.Second, - RestartBackoffInitial: time.Second, - RestartBackoffMax: 60 * time.Second, - RestartBudgetWindow: 10 * time.Minute, - RestartBudgetMax: 10, - } -} diff --git a/internal/carport/config_test.go b/internal/carport/config_test.go deleted file mode 100644 index 4917bc6..0000000 --- a/internal/carport/config_test.go +++ /dev/null @@ -1,154 +0,0 @@ -package carport_test - -import ( - "os" - "path/filepath" - "testing" - "time" - - "github.com/fdatoo/gohome/internal/carport" -) - -func writeTOML(t *testing.T, content string) string { - t.Helper() - dir := t.TempDir() - p := filepath.Join(dir, "drivers.toml") - if err := os.WriteFile(p, []byte(content), 0o644); err != nil { - t.Fatal(err) - } - return p -} - -func TestLoadConfig_EmptyFileIsValid(t *testing.T) { - p := writeTOML(t, "") - cfg, err := carport.LoadConfig(p) - if err != nil { - t.Fatalf("LoadConfig: %v", err) - } - if len(cfg.Instances) != 0 { - t.Fatalf("want 0 instances, got %d", len(cfg.Instances)) - } -} - -func TestLoadConfig_HappyPath(t *testing.T) { - bin := filepath.Join(t.TempDir(), "fake") - if err := os.WriteFile(bin, []byte("#!/bin/sh\nexit 0"), 0o755); err != nil { - t.Fatal(err) - } - p := writeTOML(t, ` -[[instance]] -id = "hue_main" -binary = "`+bin+`" -enabled = true -config_json = "{\"x\": 1}" -`) - cfg, err := carport.LoadConfig(p) - if err != nil { - t.Fatalf("LoadConfig: %v", err) - } - if len(cfg.Instances) != 1 { - t.Fatalf("want 1 instance, got %d", len(cfg.Instances)) - } - got := cfg.Instances[0] - if got.ID != "hue_main" { - t.Errorf("ID = %q", got.ID) - } - if got.Binary != bin { - t.Errorf("Binary = %q", got.Binary) - } - if !got.Enabled { - t.Error("Enabled = false, want true") - } - if string(got.ConfigJSON) != `{"x": 1}` { - t.Errorf("ConfigJSON = %q", got.ConfigJSON) - } - if got.Lifecycle.HealthProbeInterval != 15*time.Second { - t.Errorf("HealthProbeInterval default = %v", got.Lifecycle.HealthProbeInterval) - } -} - -func TestLoadConfig_RejectsDuplicateID(t *testing.T) { - bin := filepath.Join(t.TempDir(), "fake") - _ = os.WriteFile(bin, []byte("x"), 0o755) - p := writeTOML(t, ` -[[instance]] -id = "a" -binary = "`+bin+`" - -[[instance]] -id = "a" -binary = "`+bin+`" -`) - _, err := carport.LoadConfig(p) - if err == nil { - t.Fatal("expected error for duplicate id") - } -} - -func TestLoadConfig_RejectsInvalidID(t *testing.T) { - bin := filepath.Join(t.TempDir(), "fake") - _ = os.WriteFile(bin, []byte("x"), 0o755) - longID := "a" + string(make([]byte, 80)) // > 64 chars; fails regex - for _, id := range []string{"", "UPPER", "with space", longID} { - p := writeTOML(t, ` -[[instance]] -id = "`+id+`" -binary = "`+bin+`" -`) - if _, err := carport.LoadConfig(p); err == nil { - t.Errorf("expected error for id %q", id) - } - } -} - -func TestLoadConfig_RejectsMissingBinary(t *testing.T) { - p := writeTOML(t, ` -[[instance]] -id = "x" -binary = "/no/such/file/exists/here" -`) - _, err := carport.LoadConfig(p) - if err == nil { - t.Fatal("expected error for missing binary") - } -} - -func TestLoadConfig_AcceptsLifecycleOverrides(t *testing.T) { - bin := filepath.Join(t.TempDir(), "fake") - _ = os.WriteFile(bin, []byte("x"), 0o755) - p := writeTOML(t, ` -[[instance]] -id = "x" -binary = "`+bin+`" -[instance.lifecycle] -health_probe_interval_ms = 5000 -health_failures_to_restart = 5 -shutdown_grace_ms = 20000 -restart_budget_window_minutes = 30 -restart_budget_max = 3 -`) - cfg, err := carport.LoadConfig(p) - if err != nil { - t.Fatal(err) - } - inst := cfg.Instances[0] - if inst.Lifecycle.HealthProbeInterval != 5*time.Second { - t.Errorf("HealthProbeInterval = %v", inst.Lifecycle.HealthProbeInterval) - } - if inst.Lifecycle.HealthFailuresToRestart != 5 { - t.Errorf("HealthFailuresToRestart = %d", inst.Lifecycle.HealthFailuresToRestart) - } - if inst.Lifecycle.RestartBudgetWindow != 30*time.Minute { - t.Errorf("RestartBudgetWindow = %v", inst.Lifecycle.RestartBudgetWindow) - } -} - -func TestLoadConfig_MissingFileReturnsEmptyConfig(t *testing.T) { - cfg, err := carport.LoadConfig(filepath.Join(t.TempDir(), "nope.toml")) - if err != nil { - t.Fatalf("LoadConfig missing: %v", err) - } - if len(cfg.Instances) != 0 { - t.Fatalf("want 0 instances, got %d", len(cfg.Instances)) - } -} diff --git a/internal/carport/coverage_test.go b/internal/carport/coverage_test.go index 1bbb187..51b75eb 100644 --- a/internal/carport/coverage_test.go +++ b/internal/carport/coverage_test.go @@ -4,8 +4,6 @@ import ( "bytes" "context" "log/slog" - "os" - "path/filepath" "strings" "testing" "time" @@ -192,128 +190,16 @@ func TestIngestMessage_DriverEventFillsInstanceID(t *testing.T) { } } -// ---- LoadConfig error paths ---- - -func TestLoadConfig_MissingFileReturnsEmpty(t *testing.T) { - cfg, err := carport.LoadConfig(filepath.Join(t.TempDir(), "absent.toml")) - if err != nil { - t.Fatalf("missing file should be empty config, got err: %v", err) - } - if len(cfg.Instances) != 0 { - t.Errorf("Instances = %d, want 0", len(cfg.Instances)) - } -} - -func TestLoadConfig_BadTOML(t *testing.T) { - p := filepath.Join(t.TempDir(), "bad.toml") - _ = os.WriteFile(p, []byte("][]not valid toml"), 0o644) - if _, err := carport.LoadConfig(p); err == nil { - t.Fatal("expected parse error") - } -} - -func TestLoadConfig_InvalidID(t *testing.T) { - p := filepath.Join(t.TempDir(), "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "BAD ID!" -binary = "/bin/ls" -`), 0o644) - if _, err := carport.LoadConfig(p); err == nil || !strings.Contains(err.Error(), "invalid id") { - t.Errorf("err = %v, want 'invalid id'", err) - } -} - -func TestLoadConfig_DuplicateID(t *testing.T) { - p := filepath.Join(t.TempDir(), "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "a" -binary = "/bin/ls" -[[instance]] -id = "a" -binary = "/bin/ls" -`), 0o644) - if _, err := carport.LoadConfig(p); err == nil || !strings.Contains(err.Error(), "duplicate") { - t.Errorf("err = %v, want 'duplicate'", err) - } -} - -func TestLoadConfig_BinaryRequired(t *testing.T) { - p := filepath.Join(t.TempDir(), "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "a" -`), 0o644) - if _, err := carport.LoadConfig(p); err == nil || !strings.Contains(err.Error(), "binary is required") { - t.Errorf("err = %v, want 'binary is required'", err) - } -} - -func TestLoadConfig_BinaryNotFound(t *testing.T) { - p := filepath.Join(t.TempDir(), "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "a" -binary = "/no/such/path/binary" -`), 0o644) - if _, err := carport.LoadConfig(p); err == nil { - t.Fatal("expected error for missing binary") - } -} - -func TestLoadConfig_BinaryNotExecutable(t *testing.T) { - dir := t.TempDir() - bin := filepath.Join(dir, "notexec") - _ = os.WriteFile(bin, []byte("data"), 0o644) // no exec bit - p := filepath.Join(dir, "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "a" -binary = "`+bin+`" -`), 0o644) - if _, err := carport.LoadConfig(p); err == nil || !strings.Contains(err.Error(), "not executable") { - t.Errorf("err = %v, want 'not executable'", err) - } -} - -func TestLoadConfig_DefaultsApplied(t *testing.T) { - dir := t.TempDir() - bin := filepath.Join(dir, "ok") - _ = os.WriteFile(bin, []byte("#!/bin/sh\nexit 0\n"), 0o755) - p := filepath.Join(dir, "x.toml") - _ = os.WriteFile(p, []byte(`[[instance]] -id = "a" -binary = "`+bin+`" -`), 0o644) - cfg, err := carport.LoadConfig(p) - if err != nil { - t.Fatalf("LoadConfig: %v", err) - } - if len(cfg.Instances) != 1 { - t.Fatalf("got %d instances", len(cfg.Instances)) - } - inst := cfg.Instances[0] - if !inst.Enabled { - t.Error("default Enabled = false; want true") - } - // Defaults applied: - if inst.Lifecycle.HandshakeDeadline != 5*time.Second { - t.Errorf("HandshakeDeadline = %v", inst.Lifecycle.HandshakeDeadline) - } - if inst.Lifecycle.HealthFailuresToRestart != 3 { - t.Errorf("HealthFailuresToRestart = %d", inst.Lifecycle.HealthFailuresToRestart) - } - if inst.Lifecycle.RestartBudgetMax != 10 { - t.Errorf("RestartBudgetMax = %d", inst.Lifecycle.RestartBudgetMax) - } -} - // ---- New / Stop / InstanceState edges ---- -func TestNew_EmptyDriversTOMLPath(t *testing.T) { +func TestNew_NoInstances(t *testing.T) { f := newStoreFixtureForTest(t) logger := newQuietLogger() metrics := newQuietMetrics() - h, err := carport.New(carport.HostConfig{DriversTOMLPath: "", SocketDir: t.TempDir()}, + h, err := carport.New(carport.HostConfig{SocketDir: t.TempDir()}, f.db, f.store, nil, logger, metrics) if err != nil { - t.Fatalf("New empty path should succeed: %v", err) + t.Fatalf("New should succeed: %v", err) } if h == nil { t.Fatal("Host nil") @@ -327,9 +213,7 @@ func TestStartStop_NoInstances(t *testing.T) { f := newStoreFixtureForTest(t) logger := newQuietLogger() metrics := newQuietMetrics() - cfgPath := filepath.Join(t.TempDir(), "drivers.toml") - _ = os.WriteFile(cfgPath, []byte(""), 0o644) - h, err := carport.New(carport.HostConfig{DriversTOMLPath: cfgPath, SocketDir: t.TempDir()}, + h, err := carport.New(carport.HostConfig{SocketDir: t.TempDir()}, f.db, f.store, nil, logger, metrics) if err != nil { t.Fatal(err) @@ -343,33 +227,3 @@ func TestStartStop_NoInstances(t *testing.T) { h.Stop(context.Background()) h.Stop(context.Background()) } - -func TestStart_DisabledInstanceSkipped(t *testing.T) { - dir := t.TempDir() - bin := filepath.Join(dir, "ok") - _ = os.WriteFile(bin, []byte("#!/bin/sh\nexit 0\n"), 0o755) - cfgPath := filepath.Join(dir, "drivers.toml") - _ = os.WriteFile(cfgPath, []byte(`[[instance]] -id = "off" -binary = "`+bin+`" -enabled = false -`), 0o644) - - f := newStoreFixtureForTest(t) - h, err := carport.New(carport.HostConfig{DriversTOMLPath: cfgPath, SocketDir: t.TempDir()}, - f.db, f.store, nil, newQuietLogger(), newQuietMetrics()) - if err != nil { - t.Fatal(err) - } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - if err := h.Start(ctx); err != nil { - t.Fatal(err) - } - defer h.Stop(context.Background()) - // Disabled instance is not entered into h.instances → InstanceState returns StateDeclared - // (the “unknown” fallthrough). - if state := h.InstanceState("off"); state != carport.StateDeclared { - t.Errorf("disabled instance state = %s, want StateDeclared (unknown)", state) - } -} diff --git a/internal/carport/test_helpers_test.go b/internal/carport/test_helpers_test.go index 9cf87ae..4b6e0a4 100644 --- a/internal/carport/test_helpers_test.go +++ b/internal/carport/test_helpers_test.go @@ -5,8 +5,6 @@ import ( "context" "database/sql" "log/slog" - "os" - "path/filepath" "testing" carportpb "github.com/fdatoo/gohome/gen/gohome/carport/v1alpha1" @@ -62,11 +60,9 @@ func newHostForDispatch(t *testing.T, f *storeFixture, seed func(*storeFixture)) if seed != nil { seed(f) } - cfgPath := filepath.Join(t.TempDir(), "drivers.toml") - _ = os.WriteFile(cfgPath, []byte(""), 0o644) logger := observability.Init(observability.LogConfig{Level: slog.LevelWarn, Format: "json", Output: &bytes.Buffer{}}) metrics := observability.NewMetrics() - h, err := carport.New(carport.HostConfig{DriversTOMLPath: cfgPath, SocketDir: t.TempDir()}, + h, err := carport.New(carport.HostConfig{SocketDir: t.TempDir()}, f.db, f.store, reg, logger, metrics) if err != nil { t.Fatal(err) diff --git a/internal/daemon/config.go b/internal/daemon/config.go index 41280cd..1244d32 100644 --- a/internal/daemon/config.go +++ b/internal/daemon/config.go @@ -15,7 +15,6 @@ type Config struct { SocketPath string // UNIX socket for CLI mutative ops SnapshotEveryEvents int SnapshotEveryPeriod time.Duration - DriversTOMLPath string // resolved against DataDir in Run; "@data/drivers.toml" is the sentinel CarportSocketDir string // resolved against DataDir in Run; "@data/carport" is the sentinel ConfigDir string // resolved against DataDir in Run; "@data/config" is the sentinel } @@ -39,9 +38,6 @@ func (c *Config) WithDefaults() { if c.SnapshotEveryPeriod == 0 { c.SnapshotEveryPeriod = time.Hour } - if c.DriversTOMLPath == "" { - c.DriversTOMLPath = "@data/drivers.toml" - } if c.CarportSocketDir == "" { c.CarportSocketDir = "@data/carport" } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 682e0a4..838de3d 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -186,17 +186,12 @@ func (d *Daemon) Run(ctx context.Context) error { } // Phase 4.5: carport — driver supervisor - driversTOML := d.cfg.DriversTOMLPath - if driversTOML == "@data/drivers.toml" { - driversTOML = filepath.Join(dataDir, "drivers.toml") - } socketDir := d.cfg.CarportSocketDir if socketDir == "@data/carport" { socketDir = filepath.Join(dataDir, "carport") } cport, err := carport.New(carport.HostConfig{ - DriversTOMLPath: driversTOML, - SocketDir: socketDir, + SocketDir: socketDir, }, d.db, d.store, d.registry, d.logger, d.metrics) if err != nil { return fmt.Errorf("carport: %w", err) From 35c005531541df9f48435bc19f6c654b6b700247 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:39:16 -0700 Subject: [PATCH 08/11] fix(carport): update integration test and stale comments after drivers.toml removal --- internal/carport/errors.go | 7 ++++--- internal/carport/fsm.go | 2 +- internal/carport/integration_test.go | 28 +++++----------------------- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/internal/carport/errors.go b/internal/carport/errors.go index ab8893f..0b2c08c 100644 --- a/internal/carport/errors.go +++ b/internal/carport/errors.go @@ -1,6 +1,7 @@ -// Package carport hosts the driver-supervisor subsystem: drivers.toml -// configuration, per-instance subprocess lifecycle, command dispatch, and -// event ingest from drivers over the Carport gRPC protocol (v1alpha1). +// Package carport hosts the driver-supervisor subsystem: per-instance subprocess +// lifecycle, command dispatch, and event ingest from drivers over the Carport +// gRPC protocol (v1alpha1). Driver instances are registered dynamically via +// RegisterInstance. // // See docs/superpowers/specs/2026-04-21-c2-carport-protocol-design.md. package carport diff --git a/internal/carport/fsm.go b/internal/carport/fsm.go index ca718e6..e1825c8 100644 --- a/internal/carport/fsm.go +++ b/internal/carport/fsm.go @@ -6,7 +6,7 @@ import "fmt" type State int const ( - StateDeclared State = iota // row exists in drivers.toml; nothing running yet + StateDeclared State = iota // instance registered via RegisterInstance; nothing running yet StateSpawning StateAwaitingHandshake StateRunning diff --git a/internal/carport/integration_test.go b/internal/carport/integration_test.go index cdbe60a..d80fd16 100644 --- a/internal/carport/integration_test.go +++ b/internal/carport/integration_test.go @@ -74,34 +74,12 @@ func runScenario(t *testing.T, mode string, until func(*carport.Host) bool) *car t.Fatal(err) } - cfgDir := t.TempDir() - tomlPath := filepath.Join(cfgDir, "drivers.toml") - // Short timeouts to keep the test fast. - content := "[[instance]]\n" + - "id = \"test_one\"\n" + - "binary = \"" + bin + "\"\n" + - "enabled = true\n" + - "config_json = \"{\\\"TESTDRIVER_MODE\\\":\\\"" + mode + "\\\"}\"\n" + - "[instance.lifecycle]\n" + - "handshake_deadline_ms = 2000\n" + - "health_probe_interval_ms = 500\n" + - "health_probe_timeout_ms = 300\n" + - "health_failures_to_restart = 2\n" + - "restart_backoff_initial_ms = 100\n" + - "restart_backoff_max_ms = 500\n" + - "restart_budget_window_minutes = 1\n" + - "restart_budget_max = 3\n" + - "shutdown_grace_ms = 1000\n" - if err := os.WriteFile(tomlPath, []byte(content), 0o644); err != nil { - t.Fatal(err) - } - sockDir, err := os.MkdirTemp("", "ghsd") if err != nil { t.Fatal(err) } t.Cleanup(func() { _ = os.RemoveAll(sockDir) }) - h, err := carport.New(carport.HostConfig{DriversTOMLPath: tomlPath, SocketDir: sockDir}, + h, err := carport.New(carport.HostConfig{SocketDir: sockDir}, f.db, f.store, reg, newTestLogger(), newTestMetrics()) if err != nil { t.Fatal(err) @@ -114,6 +92,10 @@ func runScenario(t *testing.T, mode string, until func(*carport.Host) bool) *car if err := h.Start(ctx); err != nil { t.Fatal(err) } + params := []byte(`{"TESTDRIVER_MODE":"` + mode + `"}`) + if err := h.RegisterInstance(ctx, "test_one", "testdriver", bin, params); err != nil { + t.Fatal(err) + } if !waitFor(10*time.Second, func() bool { return until(h) }) { t.Fatalf("scenario %s: never reached expected state; current=%s", mode, h.InstanceState("test_one")) } From df03d32940818d62102466ff6773f07d2386723e Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:43:08 -0700 Subject: [PATCH 09/11] docs(design): add dynamic carport management implementation plan --- .../2026-05-01-dynamic-carport-management.md | 891 ++++++++++++++++++ 1 file changed, 891 insertions(+) create mode 100644 docs/design/plans/2026-05-01-dynamic-carport-management.md diff --git a/docs/design/plans/2026-05-01-dynamic-carport-management.md b/docs/design/plans/2026-05-01-dynamic-carport-management.md new file mode 100644 index 0000000..4e669d4 --- /dev/null +++ b/docs/design/plans/2026-05-01-dynamic-carport-management.md @@ -0,0 +1,891 @@ +# Dynamic Carport Management Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Implement `RegisterInstance`/`UnregisterInstance` on `carport.Host` and wire it as the real `CarportManager` in the daemon, so driver instances declared in `main.pkl` are spawned and supervised at runtime. + +**Architecture:** Add a `binary` field to the Pkl `DriverInstance` base class and `DriverInstanceConfig` proto so the executable path flows through the evaluation pipeline. Implement `RegisterInstance`/`UnregisterInstance` on `carport.Host` using the existing `launchLifecycle`/`shutdownInstance` machinery. Replace the `nopCarportManager` stub in `daemon.go` with the real `d.carport`. Drivers from `drivers.toml` and `main.pkl` coexist; IDs must not overlap — `RegisterInstance` returns an error on duplicate. + +**Tech Stack:** Go, `internal/carport`, `internal/config`, `proto/gohome/config/v1`; `task proto` for codegen; `task build` / `task test` throughout. + +--- + +## Codebase orientation + +Read these files before starting: + +| File | Why | +|---|---| +| `internal/carport/carport.go` | `Host` struct + `Start`/`Stop` — new methods go here | +| `internal/carport/supervisor.go` | `launchLifecycle(ctx, m)` and `shutdownInstance(ctx, m)` — reused unchanged | +| `internal/carport/config.go` | `Instance` struct and lifecycle defaults — extract `defaultLifecycleConfig()` here | +| `internal/config/manager.go` | `CarportManager` interface — adding `binary` param to `RegisterInstance` | +| `internal/config/evaluator.go` | JSON parsing of driver instances — add `binary` extraction | +| `internal/daemon/daemon.go` | Phase 4.5 (carport) then 4.6 (config) — swap nop for real carport at line ~220 | + +--- + +## File map + +### Modified files + +| File | Change | +|---|---| +| `internal/config/pkl/gohome/carport.pkl` | Add `binary: String(!isEmpty)` to `DriverInstance` | +| `proto/gohome/config/v1/snapshot.proto` | Add `string binary = 5` to `DriverInstanceConfig` | +| `internal/config/evaluator.go` | Extract `binary` from driver instance JSON; populate proto field | +| `internal/config/manager.go` | Add `binary string` to `CarportManager.RegisterInstance`; pass it in `Apply` | +| `internal/config/manager_test.go` | Update `fakeCarport.RegisterInstance` signature | +| `internal/carport/config.go` | Add `defaultLifecycleConfig()` helper | +| `internal/carport/carport.go` | Add `ctx` field; implement `RegisterInstance` + `UnregisterInstance` | +| `internal/daemon/daemon.go` | Pass `d.carport` to `config.NewManager`; remove `nopCarportManager`; add compile-time assertion | +| `internal/cli/config.go` | Update `nopCarportManager.RegisterInstance` signature | + +### New files + +| File | Responsibility | +|---|---| +| `internal/carport/dynamic_test.go` | Unit tests for `RegisterInstance`/`UnregisterInstance` error cases | + +--- + +## Task 1: Add `binary` to Pkl schema, proto, and evaluator + +**Files:** +- Modify: `internal/config/pkl/gohome/carport.pkl` +- Modify: `proto/gohome/config/v1/snapshot.proto` +- Modify: `internal/config/evaluator.go` + +- [ ] **Step 1: Add `binary` to `DriverInstance` in carport.pkl** + +Full file content for `internal/config/pkl/gohome/carport.pkl`: + +```pkl +module gohome.carport + +// DriverInstance is the base class for all driver instance configs. +// Driver authors extend this with their own typed fields. +abstract class DriverInstance { + id: String(!isEmpty) + driverName: String(!isEmpty) + binary: String(!isEmpty) +} +``` + +- [ ] **Step 2: Add `binary` to `DriverInstanceConfig` proto** + +In `proto/gohome/config/v1/snapshot.proto`, update `DriverInstanceConfig`: + +```protobuf +message DriverInstanceConfig { + string id = 1; + string driver_name = 2; + bytes config_hash = 3; + bytes params = 4; + string binary = 5; +} +``` + +- [ ] **Step 3: Regenerate proto** + +```bash +task proto +``` + +Expected: `gen/gohome/config/v1/snapshot.pb.go` updated, `GetBinary()` accessor present. + +- [ ] **Step 4: Extract `binary` in evaluator.go** + +In `internal/config/evaluator.go`, find the anonymous struct inside `parseConfigJSON` that extracts per-instance fields: + +```go +var base struct { + ID string `json:"id"` + DriverName string `json:"driverName"` +} +``` + +Replace with: + +```go +var base struct { + ID string `json:"id"` + DriverName string `json:"driverName"` + Binary string `json:"binary"` +} +``` + +In the same block where the proto struct is constructed, add `Binary`: + +```go +snap.DriverInstances = append(snap.DriverInstances, &configpb.DriverInstanceConfig{ + Id: base.ID, + DriverName: base.DriverName, + Binary: base.Binary, + ConfigHash: h[:], + Params: rawInst, +}) +``` + +- [ ] **Step 5: Build and test** + +```bash +task build && task test +``` + +Expected: compiles; all tests pass. No behaviour change yet. + +- [ ] **Step 6: Commit** + +```bash +git add internal/config/pkl/gohome/carport.pkl proto/gohome/config/v1/snapshot.proto gen/ internal/config/evaluator.go +git commit -m "feat(carport,config): add binary field to DriverInstance and DriverInstanceConfig proto" +``` + +--- + +## Task 2: Update CarportManager interface and all call sites + +**Files:** +- Modify: `internal/config/manager.go` +- Modify: `internal/config/manager_test.go` +- Modify: `internal/daemon/daemon.go` +- Modify: `internal/cli/config.go` + +- [ ] **Step 1: Update the interface in manager.go** + +In `internal/config/manager.go`, change `CarportManager`: + +```go +type CarportManager interface { + RegisterInstance(ctx context.Context, id, driverName, binary string, params []byte) error + UnregisterInstance(ctx context.Context, id string) error +} +``` + +- [ ] **Step 2: Pass binary in Apply** + +Still in `manager.go`, in the `Apply` method update both `RegisterInstance` calls: + +```go +// In DriverInstancesAdded loop: +if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetBinary(), di.GetParams()); err != nil { + return fmt.Errorf("register %q: %w", id, err) +} + +// In DriverInstancesChanged loop (re-register after unregister): +if err := m.carportMgr.RegisterInstance(ctx, di.GetId(), di.GetDriverName(), di.GetBinary(), di.GetParams()); err != nil { + return fmt.Errorf("re-register changed %q: %w", id, err) +} +``` + +- [ ] **Step 3: Update fakeCarport in manager_test.go** + +In `internal/config/manager_test.go`, update `fakeCarport`: + +```go +type fakeCarport struct { + registered []string + unregistered []string +} + +func (f *fakeCarport) RegisterInstance(_ context.Context, id, _, _ string, _ []byte) error { + f.registered = append(f.registered, id) + return nil +} + +func (f *fakeCarport) UnregisterInstance(_ context.Context, id string) error { + f.unregistered = append(f.unregistered, id) + return nil +} +``` + +- [ ] **Step 4: Update nopCarportManager in daemon.go** + +In `internal/daemon/daemon.go`, update the method signature: + +```go +func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _, _ string, _ []byte) error { + return nil +} +``` + +- [ ] **Step 5: Update nopCarportManager in cli/config.go** + +In `internal/cli/config.go`: + +```go +func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _, _ string, _ []byte) error { + return nil +} +``` + +- [ ] **Step 6: Build and test** + +```bash +task build && task test +``` + +Expected: compiles; all tests pass. + +- [ ] **Step 7: Commit** + +```bash +git add internal/config/manager.go internal/config/manager_test.go internal/daemon/daemon.go internal/cli/config.go +git commit -m "feat(config): add binary param to CarportManager.RegisterInstance" +``` + +--- + +## Task 3: Add defaultLifecycleConfig helper to carport/config.go + +**Files:** +- Modify: `internal/carport/config.go` + +- [ ] **Step 1: Add the helper** + +In `internal/carport/config.go`, add `defaultLifecycleConfig()` after the `intd` helper at the bottom of the file: + +```go +// defaultLifecycleConfig returns the defaults used for dynamically registered +// instances (those coming from main.pkl rather than drivers.toml). +func defaultLifecycleConfig() LifecycleConfig { + return LifecycleConfig{ + HandshakeDeadline: 5 * time.Second, + HealthProbeInterval: 15 * time.Second, + HealthProbeTimeout: 3 * time.Second, + HealthFailuresToRestart: 3, + ShutdownGrace: 10 * time.Second, + RestartBackoffInitial: time.Second, + RestartBackoffMax: 60 * time.Second, + RestartBudgetWindow: 10 * time.Minute, + RestartBudgetMax: 10, + } +} +``` + +- [ ] **Step 2: Build and test** + +```bash +task build && task test +``` + +Expected: passes. `LoadConfig` behaviour is unchanged. + +- [ ] **Step 3: Commit** + +```bash +git add internal/carport/config.go +git commit -m "refactor(carport): extract defaultLifecycleConfig helper" +``` + +--- + +## Task 4: Implement RegisterInstance and UnregisterInstance on carport.Host + +**Files:** +- Modify: `internal/carport/carport.go` +- Create: `internal/carport/dynamic_test.go` + +- [ ] **Step 1: Write failing tests** + +Create `internal/carport/dynamic_test.go`: + +```go +package carport + +import ( + "context" + "testing" +) + +func TestRegisterInstance_HostNotStarted(t *testing.T) { + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: make(chan struct{}), + // ctx is nil — host not started + } + err := h.RegisterInstance(context.Background(), "new", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error when host not started (ctx nil)") + } +} + +func TestRegisterInstance_DuplicateID(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + h := &Host{ + instances: map[string]*managedInstance{ + "existing": {cfg: Instance{ID: "existing"}}, + }, + stopped: make(chan struct{}), + ctx: ctx, + } + err := h.RegisterInstance(context.Background(), "existing", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error for duplicate instance ID") + } +} + +func TestRegisterInstance_HostStopped(t *testing.T) { + stopped := make(chan struct{}) + close(stopped) // host is already stopped + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: stopped, + ctx: context.Background(), + } + err := h.RegisterInstance(context.Background(), "new", "fake", "/bin/fake", nil) + if err == nil { + t.Fatal("expected error when host is stopped") + } +} + +func TestUnregisterInstance_NotFound(t *testing.T) { + h := &Host{ + instances: map[string]*managedInstance{}, + stopped: make(chan struct{}), + } + err := h.UnregisterInstance(context.Background(), "missing") + if err == nil { + t.Fatal("expected error for missing instance") + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +go test ./internal/carport/... -run "TestRegisterInstance|TestUnregisterInstance" -v +``` + +Expected: FAIL — `RegisterInstance` and `UnregisterInstance` undefined; `ctx` field missing. + +- [ ] **Step 3: Add ctx field to Host** + +In `internal/carport/carport.go`, add `ctx` to the `Host` struct (after `metrics`): + +```go +type Host struct { + cfg HostConfig + cfgData *Config + + db *sql.DB + store *eventstore.Store + router *Router + logger *slog.Logger + metrics *observability.Metrics + + ctx context.Context // root context for lifecycle goroutines; set by Start + + mu sync.RWMutex + instances map[string]*managedInstance + + stopOnce sync.Once + stopped chan struct{} +} +``` + +- [ ] **Step 4: Store ctx in Start** + +In `carport.go`, update `Start` to store the context: + +```go +func (h *Host) Start(ctx context.Context) error { + h.ctx = ctx + for _, inst := range h.cfgData.Instances { + if !inst.Enabled { + continue + } + m := &managedInstance{cfg: inst, state: StateDeclared} + h.mu.Lock() + h.instances[inst.ID] = m + h.mu.Unlock() + h.launchLifecycle(ctx, m) + } + return nil +} +``` + +- [ ] **Step 5: Implement RegisterInstance and UnregisterInstance** + +In `internal/carport/carport.go`, add these methods after `Stop` (and before the `// launchLifecycle and shutdownInstance are implemented in supervisor.go` comment): + +```go +// RegisterInstance adds a new driver instance and begins its lifecycle goroutine. +// Returns an error if an instance with that ID is already registered, if the host +// has not been started, or if the host has been stopped. +// IDs must not conflict with instances already running from drivers.toml. +func (h *Host) RegisterInstance(_ context.Context, id, driverName, binary string, params []byte) error { + if h.ctx == nil { + return fmt.Errorf("carport host not started") + } + select { + case <-h.stopped: + return fmt.Errorf("carport host is stopped") + default: + } + h.mu.Lock() + if _, exists := h.instances[id]; exists { + h.mu.Unlock() + return fmt.Errorf("instance %q already registered", id) + } + inst := Instance{ + ID: id, + Binary: binary, + Enabled: true, + ConfigJSON: params, + Lifecycle: defaultLifecycleConfig(), + } + m := &managedInstance{cfg: inst, state: StateDeclared} + h.instances[id] = m + h.mu.Unlock() + h.launchLifecycle(h.ctx, m) + return nil +} + +// UnregisterInstance stops and removes a driver instance by ID. +// Returns an error if the instance is not found. +func (h *Host) UnregisterInstance(_ context.Context, id string) error { + h.mu.Lock() + m, exists := h.instances[id] + if !exists { + h.mu.Unlock() + return fmt.Errorf("instance %q not found", id) + } + delete(h.instances, id) + h.mu.Unlock() + // Use Background context so shutdown isn't cut short by the caller's deadline. + // Shutdown duration is bounded by m.cfg.Lifecycle.ShutdownGrace. + h.shutdownInstance(context.Background(), m) + return nil +} +``` + +`fmt` is already imported in `carport.go`. `context` is already imported. + +- [ ] **Step 6: Run tests to verify they pass** + +```bash +go test ./internal/carport/... -run "TestRegisterInstance|TestUnregisterInstance" -v +``` + +Expected: all four tests PASS. + +- [ ] **Step 7: Full build and test** + +```bash +task build && task test +``` + +Expected: all pass. + +- [ ] **Step 8: Commit** + +```bash +git add internal/carport/carport.go internal/carport/dynamic_test.go +git commit -m "feat(carport): implement RegisterInstance and UnregisterInstance on Host" +``` + +--- + +## Task 5: Wire real carport into daemon + +**Files:** +- Modify: `internal/daemon/daemon.go` + +- [ ] **Step 1: Replace nopCarportManager with d.carport** + +In `internal/daemon/daemon.go`, find the config manager construction at phase 4.6 (search for `nopCarportManager`): + +```go +cfgMgr, err := config.NewManager(ctx, configDir, d.store, &nopCarportManager{}) +``` + +Replace with: + +```go +cfgMgr, err := config.NewManager(ctx, configDir, d.store, d.carport) +``` + +- [ ] **Step 2: Remove the nopCarportManager type** + +Delete the entire `nopCarportManager` block (the comment, struct, and two methods): + +```go +// DELETE this entire block: +// nopCarportManager satisfies config.CarportManager until carport.Host gains +// RegisterInstance/UnregisterInstance methods (C5+). +type nopCarportManager struct{} + +func (n *nopCarportManager) RegisterInstance(_ context.Context, _, _, _ string, _ []byte) error { + return nil +} +func (n *nopCarportManager) UnregisterInstance(_ context.Context, _ string) error { + return nil +} +``` + +- [ ] **Step 3: Add compile-time interface assertion** + +In `internal/daemon/daemon.go`, add after the `import` block: + +```go +// Compile-time assertion: *carport.Host must satisfy config.CarportManager. +var _ config.CarportManager = (*carport.Host)(nil) +``` + +- [ ] **Step 4: Build** + +```bash +task build +``` + +Expected: compiles. The assertion confirms the interface is satisfied. If it fails, the missing method will be named in the error. + +- [ ] **Step 5: Full test suite with race detector** + +```bash +task test && task test:race +``` + +Expected: all pass, no races. + +- [ ] **Step 6: Commit** + +```bash +git add internal/daemon/daemon.go +git commit -m "feat(daemon): wire carport.Host as real CarportManager, remove nop stub" +``` + +--- + +## Task 6: Remove drivers.toml support + +Now that `main.pkl` is the canonical driver source, remove the static TOML loading path entirely. + +**Files:** +- Modify: `internal/carport/config.go` — delete `Config` struct and `LoadConfig()` +- Delete: `internal/carport/config_test.go` — only tested `LoadConfig` +- Modify: `internal/carport/carport.go` — remove `cfgData` field and TOML loop from `Start`; remove `DriversTOMLPath` from `HostConfig` +- Modify: `internal/daemon/config.go` — remove `DriversTOMLPath` field and its default +- Modify: `internal/daemon/daemon.go` — remove `driversTOML` resolution block and flag passthrough +- Modify: `cmd/gohomed/main.go` — remove `--drivers-toml` flag + +- [ ] **Step 1: Prune config.go — delete LoadConfig and Config** + +In `internal/carport/config.go`, delete the `Config` struct and `LoadConfig` function. Keep `LifecycleConfig`, `Instance`, `defaultLifecycleConfig`, `dur`, and `intd`. The file should look like this after the edit: + +```go +package carport + +import ( + "time" +) + +// LifecycleConfig tunes per-instance timing and restart policy. +// Zero values get replaced by defaults during load. +type LifecycleConfig struct { + HandshakeDeadline time.Duration + HealthProbeInterval time.Duration + HealthProbeTimeout time.Duration + HealthFailuresToRestart int + ShutdownGrace time.Duration + RestartBackoffInitial time.Duration + RestartBackoffMax time.Duration + RestartBudgetWindow time.Duration + RestartBudgetMax int +} + +// Instance is a single driver instance declaration. +type Instance struct { + ID string + Binary string + Enabled bool + ConfigJSON []byte + Lifecycle LifecycleConfig +} + +// defaultLifecycleConfig returns the defaults used for dynamically registered +// instances (those coming from main.pkl rather than drivers.toml). +func defaultLifecycleConfig() LifecycleConfig { + return LifecycleConfig{ + HandshakeDeadline: 5 * time.Second, + HealthProbeInterval: 15 * time.Second, + HealthProbeTimeout: 3 * time.Second, + HealthFailuresToRestart: 3, + ShutdownGrace: 10 * time.Second, + RestartBackoffInitial: time.Second, + RestartBackoffMax: 60 * time.Second, + RestartBudgetWindow: 10 * time.Minute, + RestartBudgetMax: 10, + } +} + +func dur(ms int, def time.Duration) time.Duration { + if ms <= 0 { + return def + } + return time.Duration(ms) * time.Millisecond +} + +func intd(v, def int) int { + if v <= 0 { + return def + } + return v +} +``` + +- [ ] **Step 2: Delete config_test.go** + +```bash +rm internal/carport/config_test.go +``` + +- [ ] **Step 3: Update HostConfig and Host in carport.go** + +In `internal/carport/carport.go`, remove `DriversTOMLPath` from `HostConfig`: + +```go +type HostConfig struct { + // SocketDir is where per-instance UDS files are created. Defaults to + // /carport/ in the daemon wiring; required for any instance to be spawned. + SocketDir string +} +``` + +Remove the `cfgData *Config` field from `Host`: + +```go +type Host struct { + cfg HostConfig + + db *sql.DB + store *eventstore.Store + router *Router + logger *slog.Logger + metrics *observability.Metrics + + ctx context.Context + + mu sync.RWMutex + instances map[string]*managedInstance + + stopOnce sync.Once + stopped chan struct{} +} +``` + +Simplify `New` — no more TOML loading: + +```go +func New(cfg HostConfig, db *sql.DB, store *eventstore.Store, reg *registry.Registry, logger *slog.Logger, metrics *observability.Metrics) (*Host, error) { + return &Host{ + cfg: cfg, + db: db, + store: store, + router: NewRouter(reg), + logger: logger.With("subsystem", "carport"), + metrics: metrics, + instances: map[string]*managedInstance{}, + stopped: make(chan struct{}), + }, nil +} +``` + +Simplify `Start` — remove the TOML instance loop (keep the `h.ctx = ctx` line added in Task 4): + +```go +func (h *Host) Start(ctx context.Context) error { + h.ctx = ctx + return nil +} +``` + +- [ ] **Step 4: Build to catch compile errors** + +```bash +task build +``` + +Expected: compile errors pointing to the `DriversTOMLPath` references in daemon. Fix them in the next steps. + +- [ ] **Step 5: Remove DriversTOMLPath from daemon/config.go** + +In `internal/daemon/config.go`, remove the `DriversTOMLPath` field and its default: + +```go +type Config struct { + DataDir string + LogLevel slog.Level + LogFormat string + AdminPort int + SocketPath string + SnapshotEveryEvents int + SnapshotEveryPeriod time.Duration + CarportSocketDir string + ConfigDir string +} + +func (c *Config) WithDefaults() { + if c.DataDir == "" { + c.DataDir = "~/.local/share/gohome" + } + if c.LogFormat == "" { + c.LogFormat = "auto" + } + if c.AdminPort == 0 { + c.AdminPort = 9190 + } + if c.SocketPath == "" { + c.SocketPath = "gohomed.sock" + } + if c.SnapshotEveryEvents == 0 { + c.SnapshotEveryEvents = 10_000 + } + if c.SnapshotEveryPeriod == 0 { + c.SnapshotEveryPeriod = time.Hour + } + if c.CarportSocketDir == "" { + c.CarportSocketDir = "@data/carport" + } + if c.ConfigDir == "" { + c.ConfigDir = "@data/config" + } +} +``` + +- [ ] **Step 6: Remove driversTOML resolution from daemon.go** + +In `internal/daemon/daemon.go`, find the phase 4.5 carport block (around line 186) and remove the `driversTOML` resolution. The block should become: + +```go +// Phase 4.5: carport — driver supervisor +socketDir := d.cfg.CarportSocketDir +if socketDir == "@data/carport" { + socketDir = filepath.Join(dataDir, "carport") +} +cport, err := carport.New(carport.HostConfig{ + SocketDir: socketDir, +}, d.db, d.store, d.registry, d.logger, d.metrics) +if err != nil { + return fmt.Errorf("carport: %w", err) +} +d.carport = cport +if err := d.carport.Start(ctx); err != nil { + return fmt.Errorf("carport start: %w", err) +} +``` + +- [ ] **Step 7: Remove --drivers-toml flag from cmd/gohomed/main.go** + +In `cmd/gohomed/main.go`, delete the flag declaration: + +```go +// DELETE: +driversTOML = flag.String("drivers-toml", "", "path to drivers.toml (default /drivers.toml)") +``` + +And remove `DriversTOMLPath` from the `daemon.Config` construction: + +```go +cfg := daemon.Config{ + DataDir: *dataDir, + LogLevel: level, + LogFormat: *logFormat, + AdminPort: *adminPort, + SnapshotEveryEvents: *snapshotEveryEvt, + SnapshotEveryPeriod: *snapshotEveryDur, + ConfigDir: *configDir, +} +``` + +- [ ] **Step 8: Build and full test suite** + +```bash +task build && task test +``` + +Expected: compiles and all tests pass. + +- [ ] **Step 9: Commit** + +```bash +git add internal/carport/config.go internal/carport/carport.go internal/daemon/config.go internal/daemon/daemon.go cmd/gohomed/main.go +git rm internal/carport/config_test.go +git commit -m "feat(carport): remove drivers.toml — main.pkl is now the sole driver config source" +``` + +--- + +## Task 7: Definition of done + +- [ ] **Step 1: Build** + +```bash +task build +``` + +Expected: no errors. + +- [ ] **Step 2: All tests** + +```bash +task test && task test:race +``` + +Expected: PASS, no races. + +- [ ] **Step 3: Integration tests** + +```bash +task test:integration +``` + +Expected: PASS (requires `pkl` on PATH). + +- [ ] **Step 4: Smoke test — driver from main.pkl launches** + +```bash +mkdir -p /tmp/gohome-smoke/config +cat > /tmp/gohome-smoke/config/main.pkl << 'EOF' +amends "gohome:config" + +import "gohome:carport" as cp + +local class SmokeDriver extends cp.DriverInstance {} + +driverInstances { + new SmokeDriver { + id = "smoke-1" + driverName = "fake" + binary = "/nonexistent-binary" + } +} +EOF +./dist/gohomed --data-dir /tmp/gohome-smoke --log-level debug 2>&1 | head -40 +``` + +Expected log lines (in order): +1. `config applied` — confirms `Apply` ran and called `RegisterInstance` +2. `spawn_error` or similar for `smoke-1` — confirms `launchLifecycle` ran (binary doesn't exist) + +- [ ] **Step 5: Verify --drivers-toml is gone** + +```bash +./dist/gohomed --help 2>&1 | grep drivers-toml +``` + +Expected: no output. + +- [ ] **Step 6: Verify duplicate ID error** + +Declare the same `id` twice in `main.pkl` and confirm the daemon exits with a clear error during `Apply`. + +--- + +## Known limitations (follow-up work) + +- **No lifecycle tuning from Pkl**: dynamically registered instances always use `defaultLifecycleConfig()`. Add lifecycle fields to `DriverInstance` in a follow-up. +- **Migration**: existing `drivers.toml` entries must be moved to `main.pkl` before upgrading. The `--drivers-toml` flag is gone. From c8020977af68d5426c1d72c89f1c2be2dccdfa33 Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:46:45 -0700 Subject: [PATCH 10/11] fix(carport,daemon): remove unused helpers, suppress contextcheck, tidy go.mod --- go.mod | 1 - go.sum | 2 -- internal/carport/carport.go | 4 ++-- internal/carport/config.go | 13 ------------- internal/daemon/daemon.go | 1 - 5 files changed, 2 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index f0ad370..f8402b4 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.25.9 require ( connectrpc.com/connect v1.19.2 - github.com/BurntSushi/toml v1.6.0 github.com/apple/pkl-go v0.13.2 github.com/benbjohnson/immutable v0.4.3 github.com/charmbracelet/lipgloss v1.1.0 diff --git a/go.sum b/go.sum index 2d8269c..c5d9602 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ connectrpc.com/connect v1.19.2 h1:McQ83FGdzL+t60peksi0gXC7MQ/iLKgLduAnThbM0mo= connectrpc.com/connect v1.19.2/go.mod h1:tN20fjdGlewnSFeZxLKb0xwIZ6ozc3OQs2hTXy4du9w= -github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= -github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/apple/pkl-go v0.13.2 h1:UCug0neH9Ufw0Loujosv5/UUXEXzY76HXbjqYqlPSGI= github.com/apple/pkl-go v0.13.2/go.mod h1:Ko3AgXOKd/vVYtsRZgoCZhymymz9RxqCIcfdZhOX85I= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= diff --git a/internal/carport/carport.go b/internal/carport/carport.go index b40a319..c6493c8 100644 --- a/internal/carport/carport.go +++ b/internal/carport/carport.go @@ -137,7 +137,7 @@ func (h *Host) RegisterInstance(_ context.Context, id, driverName, binary string m := &managedInstance{cfg: inst, state: StateDeclared} h.instances[id] = m h.mu.Unlock() - h.launchLifecycle(h.ctx, m) + h.launchLifecycle(h.ctx, m) //nolint:contextcheck // lifecycle goroutine must outlive the caller's context return nil } @@ -154,7 +154,7 @@ func (h *Host) UnregisterInstance(_ context.Context, id string) error { h.mu.Unlock() // Use Background context so shutdown isn't cut short by the caller's deadline. // Shutdown duration is bounded by m.cfg.Lifecycle.ShutdownGrace. - h.shutdownInstance(context.Background(), m) + h.shutdownInstance(context.Background(), m) //nolint:contextcheck return nil } diff --git a/internal/carport/config.go b/internal/carport/config.go index ac8800d..7063f3d 100644 --- a/internal/carport/config.go +++ b/internal/carport/config.go @@ -43,16 +43,3 @@ func defaultLifecycleConfig() LifecycleConfig { } } -func dur(ms int, def time.Duration) time.Duration { - if ms <= 0 { - return def - } - return time.Duration(ms) * time.Millisecond -} - -func intd(v, def int) int { - if v <= 0 { - return def - } - return v -} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 838de3d..2242614 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -597,4 +597,3 @@ func (a *carportAdapter) Dispatch(ctx context.Context, entityID, capability stri } return &starlark.DispatchResult{Ok: res.GetOk(), Error: res.GetErrorMessage()}, nil } - From 7f5382f367b63969961a959af17531d015140bcc Mon Sep 17 00:00:00 2001 From: Fynn Datoo Date: Fri, 1 May 2026 02:58:00 -0700 Subject: [PATCH 11/11] fix(carport): restore integration test lifecycle timeouts; add RegisterInstanceWithLifecycle --- internal/carport/carport.go | 31 ++++++++++++++++++++++++++++ internal/carport/config.go | 1 - internal/carport/integration_test.go | 16 +++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/internal/carport/carport.go b/internal/carport/carport.go index c6493c8..6b22be2 100644 --- a/internal/carport/carport.go +++ b/internal/carport/carport.go @@ -141,6 +141,37 @@ func (h *Host) RegisterInstance(_ context.Context, id, driverName, binary string return nil } +// RegisterInstanceWithLifecycle is like RegisterInstance but accepts an explicit +// LifecycleConfig, allowing callers (e.g. integration tests) to override timing. +// The CarportManager interface uses RegisterInstance which applies defaultLifecycleConfig. +func (h *Host) RegisterInstanceWithLifecycle(_ context.Context, id, _, binary string, params []byte, lc LifecycleConfig) error { + if h.ctx == nil { + return fmt.Errorf("carport host not started") + } + select { + case <-h.stopped: + return fmt.Errorf("carport host is stopped") + default: + } + h.mu.Lock() + if _, exists := h.instances[id]; exists { + h.mu.Unlock() + return fmt.Errorf("instance %q already registered", id) + } + inst := Instance{ + ID: id, + Binary: binary, + Enabled: true, + ConfigJSON: params, + Lifecycle: lc, + } + m := &managedInstance{cfg: inst, state: StateDeclared} + h.instances[id] = m + h.mu.Unlock() + h.launchLifecycle(h.ctx, m) //nolint:contextcheck // lifecycle goroutine must outlive the caller's context + return nil +} + // UnregisterInstance stops and removes a driver instance by ID. // Returns an error if the instance is not found. func (h *Host) UnregisterInstance(_ context.Context, id string) error { diff --git a/internal/carport/config.go b/internal/carport/config.go index 7063f3d..aaed7ad 100644 --- a/internal/carport/config.go +++ b/internal/carport/config.go @@ -42,4 +42,3 @@ func defaultLifecycleConfig() LifecycleConfig { RestartBudgetMax: 10, } } - diff --git a/internal/carport/integration_test.go b/internal/carport/integration_test.go index d80fd16..dc7d0d0 100644 --- a/internal/carport/integration_test.go +++ b/internal/carport/integration_test.go @@ -93,7 +93,21 @@ func runScenario(t *testing.T, mode string, until func(*carport.Host) bool) *car t.Fatal(err) } params := []byte(`{"TESTDRIVER_MODE":"` + mode + `"}`) - if err := h.RegisterInstance(ctx, "test_one", "testdriver", bin, params); err != nil { + // Use short timeouts so scenarios resolve within the 10 s test window. + // The default lifecycle has 15 s health probes which would cause crash-detection + // tests to time out before the supervisor transitions out of StateRunning. + lc := carport.LifecycleConfig{ + HandshakeDeadline: 2 * time.Second, + HealthProbeInterval: 500 * time.Millisecond, + HealthProbeTimeout: 300 * time.Millisecond, + HealthFailuresToRestart: 2, + RestartBackoffInitial: 100 * time.Millisecond, + RestartBackoffMax: 500 * time.Millisecond, + RestartBudgetWindow: time.Minute, + RestartBudgetMax: 3, + ShutdownGrace: time.Second, + } + if err := h.RegisterInstanceWithLifecycle(ctx, "test_one", "testdriver", bin, params, lc); err != nil { t.Fatal(err) } if !waitFor(10*time.Second, func() bool { return until(h) }) {