Skip to content

Commit 09b61df

Browse files
alicodingclaude
andcommitted
Fix task run/build discoverability, parameterize execution DSN, document registry semantics
Three independent, verified changes (goal-scoped work, tasks #1/#4/#6): - task build/package/build:server now echo their real output path on success, and task run's darwin/linux/windows tasks gained a missing deps: on build:native -- previously `task run` assumed a binary already existed and would fail on a fresh clone, despite its name and summary implying it's a standalone build+launch command (run:server already had this right; desktop run: didn't). - internal/adapters/execution.New now takes a full DSN (databaseURL) instead of hardcoding "sqlite:"+dbPath internally. DBOS itself already accepts Postgres/CockroachDB DSNs; the scheme choice moves to the caller (main.go), which now reads MILL_EXECUTION_DATABASE_URL as an override, falling back to the existing MILL_EXECUTION_DB_PATH/default sqlite path unchanged. A regulated deployment needing an externally managed audit database gets there via config, not an adapter rewrite. - The three ADR-0006/ADR-0015 self-registration registries (RegisterNodeType, RegisterTrigger, RegisterAuthStrategy) have two different duplicate-key behaviors (panic vs silent overwrite), undocumented until now. Recorded on all three functions plus a SPEC.md cross-reference -- not a decision, just making a real, previously-invisible inconsistency visible. Verified: go vet, golangci-lint (0 issues), go test ./internal/... -race -cover (all pass), go build for both desktop and CGO_ENABLED=0-equivalent server tags, check-loc.sh, check-rules-frontmatter.sh. Frontend checks skipped -- zero frontend files touched, matching lefthook.yml's own glob scoping. Taskfile run: fix verified via `task --dry` (confirms build:native now runs before launch) plus one live task package run earlier in the session; full live task run execution was blocked by resource contention with an unrelated, pre-existing `task dev` session on this machine and deliberately not forced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FYwojT8GdUbYSoggbvEFft
1 parent 2cb8331 commit 09b61df

13 files changed

Lines changed: 87 additions & 20 deletions

File tree

Taskfile.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,13 @@ tasks:
3838
deps: [clean]
3939
cmds:
4040
- task: "{{.GOOS}}:build"
41+
- 'echo "Built raw binary: {{.BIN_DIR}}/{{.APP_NAME}}{{exeExt}} -- this is NOT a launchable app bundle. Run `task package` to produce one, or `task run` to build and launch in one step."'
4142

4243
package:
4344
summary: Packages a production build of the application
4445
cmds:
4546
- task: "{{.GOOS}}:package"
47+
- '{{if eq .GOOS "darwin"}}echo "Packaged app: {{.BIN_DIR}}/{{.APP_NAME}}.app -- launch with: open {{.BIN_DIR}}/{{.APP_NAME}}.app"{{else if eq .GOOS "windows"}}echo "Packaged app: {{.BIN_DIR}}/{{.APP_NAME}}.exe"{{else}}echo "Packaged app: see {{.BIN_DIR}}/ for output"{{end}}'
4648

4749
run:
4850
summary: Runs the application
@@ -70,6 +72,7 @@ tasks:
7072
deps: [clean]
7173
cmds:
7274
- task: common:build:server
75+
- 'echo "Built server binary: {{.BIN_DIR}}/{{.APP_NAME}}-server{{exeExt}} -- run with: ./{{.BIN_DIR}}/{{.APP_NAME}}-server{{exeExt}}"'
7376

7477
run:server:
7578
summary: Runs the application in server mode

build/darwin/Taskfile.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,11 @@ tasks:
182182
- 'echo "Skipping codesign (not available on {{OS}}). Sign the .app on macOS before distribution."'
183183

184184
run:
185+
summary: Builds (dev-tagged) and runs the application
186+
deps:
187+
- task: build:native
188+
vars:
189+
DEV: "true"
185190
cmds:
186191
- mkdir -p "{{.BIN_DIR}}/{{.APP_NAME}}.dev.app/Contents/MacOS"
187192
- mkdir -p "{{.BIN_DIR}}/{{.APP_NAME}}.dev.app/Contents/Resources"

build/linux/Taskfile.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ tasks:
179179
OUTPUTFILE: '{{.ROOT_DIR}}/build/linux/{{.APP_NAME}}.desktop'
180180

181181
run:
182+
summary: Builds and runs the application
183+
deps:
184+
- task: build:native
185+
vars:
186+
DEV: "true"
182187
cmds:
183188
- '{{.BIN_DIR}}/{{.APP_NAME}}'
184189

build/windows/Taskfile.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ tasks:
161161
- wails3 tool msix-install-tools
162162

163163
run:
164+
summary: Builds and runs the application
165+
deps:
166+
- task: build:native
167+
vars:
168+
DEV: "true"
164169
cmds:
165170
- '{{.BIN_DIR}}/{{.APP_NAME}}.exe'
166171

executionchildworkflow_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestRunChildWorkflow_TracksRealParentChildRelationship(t *testing.T) {
4040
}
4141

4242
dbPath := filepath.Join(t.TempDir(), "exec.db")
43-
exec, err := NewExecutionService(dbPath, comp)
43+
exec, err := NewExecutionService("sqlite:"+dbPath, comp)
4444
if err != nil {
4545
t.Fatalf("NewExecutionService: %v", err)
4646
}
@@ -96,7 +96,7 @@ func TestRunChildWorkflow_IdempotencyKey_PreventsDuplicateChildRuns(t *testing.T
9696
}
9797

9898
dbPath := filepath.Join(t.TempDir(), "exec.db")
99-
exec, err := NewExecutionService(dbPath, comp)
99+
exec, err := NewExecutionService("sqlite:"+dbPath, comp)
100100
if err != nil {
101101
t.Fatalf("NewExecutionService: %v", err)
102102
}

executionservice.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,13 @@ type ExecutionService struct {
127127
}
128128

129129
// NewExecutionService builds and launches the durable-execution runtime
130-
// backed by a local SQLite file at dbPath. Registration happens inside
131-
// execution.New, before Launch, per that function's own doc comment.
132-
func NewExecutionService(dbPath string, comp *CompositionService) (*ExecutionService, error) {
130+
// backed by databaseURL (a DBOS-native DSN -- see execution.New's own
131+
// doc comment for the sqlite-by-default, Postgres-by-config reasoning).
132+
// Registration happens inside execution.New, before Launch, per that
133+
// function's own doc comment.
134+
func NewExecutionService(databaseURL string, comp *CompositionService) (*ExecutionService, error) {
133135
e := &ExecutionService{comp: comp}
134-
ctx, err := execution.New("mill", dbPath, func(ctx execution.Context) {
136+
ctx, err := execution.New("mill", databaseURL, func(ctx execution.Context) {
135137
execution.RegisterWorkflow(ctx, e.runWorkflow, execution.WithWorkflowName(millRunWorkflowName))
136138
})
137139
if err != nil {

executionservice_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestRunWorkflow_SummaryHasRealWorkflowLabelAndStepOutput(t *testing.T) {
3434
workflowID := findBuiltInWorkflowID(t, comp, "Load sample HTML")
3535

3636
dbPath := filepath.Join(t.TempDir(), "exec.db")
37-
exec, err := NewExecutionService(dbPath, comp)
37+
exec, err := NewExecutionService("sqlite:"+dbPath, comp)
3838
if err != nil {
3939
t.Fatalf("NewExecutionService: %v", err)
4040
}
@@ -89,7 +89,7 @@ func TestRedriveRun_ReturnsNewRunWithSameStepOutput(t *testing.T) {
8989
workflowID := findBuiltInWorkflowID(t, comp, "Load sample HTML")
9090

9191
dbPath := filepath.Join(t.TempDir(), "exec.db")
92-
exec, err := NewExecutionService(dbPath, comp)
92+
exec, err := NewExecutionService("sqlite:"+dbPath, comp)
9393
if err != nil {
9494
t.Fatalf("NewExecutionService: %v", err)
9595
}

internal/adapters/execution/execution.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,18 @@ import (
99
_ "github.com/dbos-inc/dbos-transact-golang/dbos/driver/sqlite"
1010
)
1111

12-
// New builds a DBOS runtime backed by a local SQLite file at dbPath --
13-
// no Postgres, no separate daemon, satisfying docs/SPEC.md §1.2's
14-
// embeddable-in-binary hard filter (research + spike in docs/adr/0004).
12+
// New builds a DBOS runtime backed by databaseURL -- a DBOS-native DSN
13+
// string (dbos.Config's own DatabaseURL field). The default caller
14+
// (main.go) passes a "sqlite:"-prefixed local file path, satisfying
15+
// docs/SPEC.md §1.2's embeddable-in-binary hard filter with zero
16+
// external dependency (research + spike in docs/adr/0004) -- but DBOS
17+
// itself also accepts Postgres/CockroachDB DSNs (confirmed directly
18+
// against dbos.Config's own doc comment), so a regulated deployment
19+
// that needs an externally-managed audit database gets there by
20+
// passing a different URL, not by swapping this adapter. The scheme
21+
// choice deliberately isn't made in this package: it's the caller's
22+
// config decision, same layering this repo already uses for every
23+
// other adapter (.claude/rules/backend.md).
1524
// register runs before Launch: DBOS resolves a workflow by registered
1625
// function identity for crash recovery, so registration must happen
1726
// before the runtime starts accepting/recovering runs -- this is the
@@ -22,10 +31,10 @@ import (
2231
// finding #5) and only activates if ConductorAPIKey/DBOS__CLOUD is
2332
// set -- Config below never sets either, which is the whole guard; DBOS
2433
// itself exposes no separate flag to assert this at runtime.
25-
func New(appName, dbPath string, register func(Context)) (Context, error) {
34+
func New(appName, databaseURL string, register func(Context)) (Context, error) {
2635
ctx, err := dbos.NewContext(context.Background(), dbos.Config{
2736
AppName: appName,
28-
DatabaseURL: "sqlite:" + dbPath,
37+
DatabaseURL: databaseURL,
2938
})
3039
if err != nil {
3140
return nil, fmt.Errorf("execution: new context: %w", err)

internal/adapters/execution/execution_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestResumeAfterFailure_DoesNotReExecuteCheckpointedStep(t *testing.T) {
5151
return a + "+" + b, nil
5252
}
5353

54-
runtime, err := New("mill-execution-test", dbPath, func(ctx Context) {
54+
runtime, err := New("mill-execution-test", "sqlite:"+dbPath, func(ctx Context) {
5555
RegisterWorkflow(ctx, workflow)
5656
})
5757
if err != nil {
@@ -125,7 +125,7 @@ func TestResumeAfterFailure_DoesNotReExecuteCheckpointedStep(t *testing.T) {
125125
// document that reasoning as executable, not to probe DBOS's internals.
126126
func TestNew_NeverConfiguresConductor(t *testing.T) {
127127
dbPath := filepath.Join(t.TempDir(), "phonehome.db")
128-
runtime, err := New("mill-phonehome-test", dbPath, func(Context) {})
128+
runtime, err := New("mill-phonehome-test", "sqlite:"+dbPath, func(Context) {})
129129
if err != nil {
130130
t.Fatalf("New: %v", err)
131131
}

internal/domain/composition/integration.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ var authStrategies = map[httprequest.AuthType]AuthStrategy{}
5858
// Triggers -- each AuthType's strategy lives in its own small file,
5959
// registered via init(), so adding a new AuthType (mTLS included) is a
6060
// pure addition, never a change to an existing strategy's file.
61+
//
62+
// Documented divergence from its two siblings (RegisterNodeType,
63+
// RegisterTrigger): this map assignment silently overwrites on a
64+
// duplicate AuthType instead of panicking -- neither behavior was a
65+
// deliberate choice recorded anywhere before this comment, it's simply
66+
// what a bare map write does. Not a bug fix, since nothing depends on
67+
// either behavior today (every AuthType strategy registers exactly
68+
// once, from this repo's own init() files) -- but a future extension
69+
// point that wants a registry to support intentional substitution
70+
// should decide that on purpose, not inherit whichever of these two
71+
// shapes it happened to copy from.
6172
func RegisterAuthStrategy(t httprequest.AuthType, fn AuthStrategy) {
6273
authStrategies[t] = fn
6374
}

0 commit comments

Comments
 (0)