Skip to content

Commit 168b9d7

Browse files
intel352claude
andcommitted
fix(wfctl): remove database-to-sqlite modernize rule
Generic database functionality should be provider-agnostic rather than recommending SQLite-specific patterns. Add design and plan docs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 947a1df commit 168b9d7

4 files changed

Lines changed: 1906 additions & 145 deletions

File tree

cmd/wfctl/modernize_rules.go

Lines changed: 0 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ func allModernizeRules() []Rule {
1515
conditionalFieldRule(),
1616
dbQueryModeRule(),
1717
dbQueryIndexRule(),
18-
databaseToSqliteRule(),
1918
absoluteDbPathRule(),
2019
emptyRoutesRule(),
2120
camelCaseConfigRule(),
@@ -501,86 +500,3 @@ func camelCaseConfigRule() Rule {
501500
}
502501
}
503502

504-
func databaseToSqliteRule() Rule {
505-
return Rule{
506-
ID: "database-to-sqlite",
507-
Description: "Convert database.workflow modules to storage.sqlite",
508-
Severity: "warning",
509-
Check: func(root *yaml.Node, raw []byte) []Finding {
510-
var findings []Finding
511-
forEachModule(root, func(mod *yaml.Node) {
512-
typeNode := findMapValue(mod, "type")
513-
if typeNode != nil && typeNode.Value == "database.workflow" {
514-
nameNode := findMapValue(mod, "name")
515-
name := ""
516-
if nameNode != nil {
517-
name = nameNode.Value
518-
}
519-
findings = append(findings, Finding{
520-
RuleID: "database-to-sqlite",
521-
Line: typeNode.Line,
522-
Message: fmt.Sprintf("Module %q uses database.workflow (use storage.sqlite instead)", name),
523-
Fixable: true,
524-
})
525-
}
526-
})
527-
return findings
528-
},
529-
Fix: func(root *yaml.Node) []Change {
530-
var changes []Change
531-
forEachModule(root, func(mod *yaml.Node) {
532-
typeNode := findMapValue(mod, "type")
533-
if typeNode == nil || typeNode.Value != "database.workflow" {
534-
return
535-
}
536-
nameNode := findMapValue(mod, "name")
537-
name := "unknown"
538-
if nameNode != nil {
539-
name = nameNode.Value
540-
}
541-
542-
// Change type
543-
typeNode.Value = "storage.sqlite"
544-
545-
// Rebuild config: extract DSN to derive dbPath
546-
cfg := findMapValue(mod, "config")
547-
if cfg == nil || cfg.Kind != yaml.MappingNode {
548-
cfg = &yaml.Node{Kind: yaml.MappingNode}
549-
mod.Content = append(mod.Content,
550-
&yaml.Node{Kind: yaml.ScalarNode, Value: "config"},
551-
cfg,
552-
)
553-
}
554-
555-
// Try to extract filename from DSN
556-
dbPath := name + ".db"
557-
dsnNode := findMapValue(cfg, "dsn")
558-
if dsnNode != nil {
559-
dsn := dsnNode.Value
560-
dsn = strings.TrimPrefix(dsn, "file:")
561-
dsn = strings.Split(dsn, "?")[0]
562-
if dsn != "" {
563-
dbPath = dsn
564-
}
565-
}
566-
567-
// Replace config contents with storage.sqlite config
568-
cfg.Content = []*yaml.Node{
569-
{Kind: yaml.ScalarNode, Value: "dbPath"},
570-
{Kind: yaml.ScalarNode, Value: dbPath},
571-
{Kind: yaml.ScalarNode, Value: "maxConnections"},
572-
{Kind: yaml.ScalarNode, Value: "5"},
573-
{Kind: yaml.ScalarNode, Value: "walMode"},
574-
{Kind: yaml.ScalarNode, Value: "true", Tag: "!!bool"},
575-
}
576-
577-
changes = append(changes, Change{
578-
RuleID: "database-to-sqlite",
579-
Line: typeNode.Line,
580-
Description: fmt.Sprintf("Converted module %q from database.workflow to storage.sqlite (dbPath: %s)", name, dbPath),
581-
})
582-
})
583-
return changes
584-
},
585-
}
586-
}

cmd/wfctl/modernize_test.go

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -404,57 +404,6 @@ pipelines:
404404
}
405405
}
406406

407-
func TestDatabaseToSqliteCheck(t *testing.T) {
408-
input := `
409-
modules:
410-
- name: my-db
411-
type: database.workflow
412-
config:
413-
driver: sqlite
414-
dsn: "file:data.db"
415-
`
416-
rule := findRule("database-to-sqlite")
417-
if rule == nil {
418-
t.Fatal("database-to-sqlite rule not found")
419-
}
420-
421-
doc := parseTestYAML(t, input)
422-
findings := rule.Check(doc, []byte(input))
423-
if len(findings) == 0 {
424-
t.Fatal("expected findings for database.workflow")
425-
}
426-
}
427-
428-
func TestDatabaseToSqliteFix(t *testing.T) {
429-
input := `
430-
modules:
431-
- name: my-db
432-
type: database.workflow
433-
config:
434-
driver: sqlite
435-
dsn: "file:data.db"
436-
`
437-
rule := findRule("database-to-sqlite")
438-
doc := parseTestYAML(t, input)
439-
changes := rule.Fix(doc)
440-
if len(changes) == 0 {
441-
t.Fatal("expected changes from fix")
442-
}
443-
444-
out, _ := yaml.Marshal(doc)
445-
result := string(out)
446-
447-
if strings.Contains(result, "database.workflow") {
448-
t.Errorf("expected type to be changed, got:\n%s", result)
449-
}
450-
if !strings.Contains(result, "storage.sqlite") {
451-
t.Errorf("expected storage.sqlite type, got:\n%s", result)
452-
}
453-
if !strings.Contains(result, "dbPath") {
454-
t.Errorf("expected dbPath in config, got:\n%s", result)
455-
}
456-
}
457-
458407
func TestAbsoluteDbPathCheck(t *testing.T) {
459408
input := `
460409
modules:
@@ -540,7 +489,6 @@ func TestModernizeAllRulesRegistered(t *testing.T) {
540489
"conditional-field",
541490
"db-query-mode",
542491
"db-query-index",
543-
"database-to-sqlite",
544492
"absolute-dbpath",
545493
"empty-routes",
546494
"camelcase-config",
@@ -579,12 +527,6 @@ func TestModernizeFullPipeline(t *testing.T) {
579527
// A config with multiple issues
580528
input := `
581529
name: test-app
582-
modules:
583-
- name: my-db
584-
type: database.workflow
585-
config:
586-
driver: sqlite
587-
dsn: "file:app.db"
588530
pipelines:
589531
check:
590532
steps:
@@ -643,9 +585,6 @@ pipelines:
643585
if strings.Contains(result, "check-input") {
644586
t.Error("hyphen-steps: check-input not renamed")
645587
}
646-
if strings.Contains(result, "database.workflow") {
647-
t.Error("database-to-sqlite: type not changed")
648-
}
649588
if strings.Contains(result, `field: "{{ .steps`) {
650589
t.Error("conditional-field: template not converted")
651590
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# wfctl modernize — YAML Config Codemod Tool
2+
3+
## Problem
4+
5+
Users encounter runtime errors from known YAML config anti-patterns that could be detected and fixed automatically. During scenario testing, 8+ recurring issues were identified that require manual debugging and fixing. A codemod tool eliminates this toil.
6+
7+
## Command Interface
8+
9+
```
10+
wfctl modernize [options] <config.yaml|directory>
11+
wfctl modernize --list-rules
12+
wfctl modernize --dry-run config/app.yaml # default: show what would change
13+
wfctl modernize --apply config/app.yaml # write changes in-place
14+
wfctl modernize --rules hyphen-steps,conditional-field config/app.yaml
15+
wfctl modernize --exclude-rules validate-syntax config/app.yaml
16+
```
17+
18+
Dry-run is the default. `--apply` writes changes in-place (git is the backup).
19+
20+
## Rules
21+
22+
| Rule ID | Description | Severity | Fixable |
23+
|---------|-------------|----------|---------|
24+
| `hyphen-steps` | Rename hyphenated step names to underscores, update all references | error | yes |
25+
| `conditional-field` | Convert `{{ }}` template syntax in `step.conditional` field to dot-path | error | yes |
26+
| `db-query-mode` | Add `mode: single` to `step.db_query` steps whose downstream templates use `.row` or `.found` | warning | yes |
27+
| `db-query-index` | Convert `.steps.X.row.Y` dot-access to `index .steps "X" "row" "Y"` in templates | error | yes |
28+
| `database-to-sqlite` | Convert `database.workflow` modules to `storage.sqlite` with `dbPath` | warning | yes |
29+
| `absolute-dbpath` | Warn on absolute `dbPath` values in `storage.sqlite` | warning | no |
30+
| `empty-routes` | Detect empty `routes` map in `step.conditional` | error | no |
31+
| `camelcase-config` | Detect snake_case config field names (engine requires camelCase) | warning | no |
32+
33+
## Architecture
34+
35+
### AST-Based Transformations
36+
37+
Uses `yaml.Node` (gopkg.in/yaml.v3) for parsing and rewriting. This preserves comments, formatting, and ordering — critical for config files users maintain by hand. The LSP already uses `yaml.Node` for diagnostics, so this pattern is established.
38+
39+
### Rule Interface
40+
41+
```go
42+
type Rule struct {
43+
ID string
44+
Description string
45+
Severity string // "error" or "warning"
46+
Check func(root *yaml.Node, cfg *config.WorkflowConfig) []Finding
47+
Fix func(root *yaml.Node) []Change
48+
}
49+
50+
type Finding struct {
51+
RuleID string
52+
Line int
53+
Column int
54+
Message string
55+
Fixable bool
56+
}
57+
58+
type Change struct {
59+
RuleID string
60+
Line int
61+
Description string
62+
}
63+
```
64+
65+
- `Check()` always runs — detects issues and reports findings
66+
- `Fix()` only runs with `--apply` — mutates the `yaml.Node` AST
67+
- After all fixes, the modified AST is marshaled back to the file
68+
69+
### Output Format
70+
71+
**Dry-run (default):**
72+
```
73+
config/app.yaml:
74+
line 12: [hyphen-steps] Step "check-xss" uses hyphens (fixable)
75+
line 45: [conditional-field] step.conditional field uses template syntax (fixable)
76+
line 78: [db-query-mode] step.db_query missing mode:single, downstream uses .row (fixable)
77+
78+
3 issues found (3 fixable). Run with --apply to fix.
79+
```
80+
81+
**Apply mode:**
82+
```
83+
config/app.yaml:
84+
line 12: [hyphen-steps] Renamed "check-xss" -> "check_xss" (+ 3 references)
85+
line 45: [conditional-field] Converted field to dot-path syntax
86+
line 78: [db-query-mode] Added mode: single
87+
88+
3 fixes applied.
89+
```
90+
91+
Supports `--format json` for CI integration.
92+
93+
## Files
94+
95+
| File | Purpose |
96+
|------|---------|
97+
| `cmd/wfctl/modernize.go` | Command entry, flag parsing, orchestration, AST helpers |
98+
| `cmd/wfctl/modernize_rules.go` | All rule implementations |
99+
| `cmd/wfctl/modernize_test.go` | Tests with inline YAML fixtures |
100+
101+
## Key Decisions
102+
103+
- **Dry-run by default** — safe; user must opt in with `--apply`
104+
- **AST-based**`yaml.Node` preserves comments and formatting better than unmarshal/marshal round-trips
105+
- **Rules are data-driven** — each rule is a struct with check/fix functions, easy to add new rules
106+
- **Directory scanning** — reuses existing `findYAMLFiles()` helper from wfctl
107+
- **No backup files** — git is the backup
108+
- **Follows existing wfctl patterns**`flag.FlagSet`, `reorderFlags()`, same file conventions

0 commit comments

Comments
 (0)