Skip to content

Commit cb57439

Browse files
wbrezaCopilot
andauthored
feat: refactor waza new to use shared FileWriter #58 (#66)
* feat: refactor waza new to use shared FileWriter #58 Replace the inline write loop in cmd_new.go with the shared FileWriter from internal/scaffold/writer.go. Malformed SKILL.md detection still runs before FileWriter — the file is removed so FileWriter creates it fresh. Inventory now uses consistent ➕/✅ emoji indicators (always visible, not gated behind --verbose), matching the waza init behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: update squad state for #58 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent dcc3558 commit cb57439

4 files changed

Lines changed: 104 additions & 113 deletions

File tree

.squad/agents/linus/history.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,10 @@ All code roles now use `claude-opus-4.6`. Docs/Scribe/diversity use `gemini-3-pr
9999
- **Files changed:** `internal/scaffold/writer.go` (new), `internal/scaffold/writer_test.go` (new), `cmd/waza/cmd_init.go` (modified)
100100
- **What:** Created `FileWriter` service in `internal/scaffold/` that encapsulates the create-if-missing + skip-if-exists pattern used by `waza init`. Returns structured `Inventory` with per-entry outcomes. Refactored `cmd_init.go` Phase 5 to use `FileWriter` instead of its inline write loop. Inventory is always visible with ➕/✅ indicators. 8 unit tests covering all paths.
101101
- **Key learning:** The `internal/scaffold/` package already existed with template functions for eval/skill generation — `FileWriter` fits naturally alongside those. The `FileEntry` type mirrors the old `initItem` struct but is exported for reuse by `cmd_new.go` in a future PR. Empty-content file entries (e.g., when `needConfigPrompt` is false) are treated as skipped — the writer doesn't create zero-byte files.
102+
103+
### #58 — Refactor waza new to use shared FileWriter (PR pending)
104+
- **Date:** 2026-02-26
105+
- **Branch:** `squad/58-new-use-filewriter`
106+
- **Files changed:** `cmd/waza/cmd_new.go`, `cmd/waza/cmd_new_test.go`
107+
- **What:** Replaced the inline `writeFiles()` loop and `fileEntry` type in `cmd_new.go` with the shared `FileWriter` from `internal/scaffold/writer.go`. The malformed SKILL.md detection (`detectExistingSkillMD`) still runs before FileWriter — when overwrite is needed, the malformed file is deleted so FileWriter creates it fresh with ➕. Removed `lipgloss` dependency from the file. Updated 5 test assertions to match ➕/✅ emoji indicators instead of lipgloss-styled ✓/+.
108+
- **Key learning:** The `overwrite` flag pattern from the old `fileEntry` struct doesn't map directly to FileWriter's create-if-missing model. The clean solution is to `os.Remove` the malformed file before passing it to FileWriter, so the writer sees it as absent and creates it normally. This avoids adding overwrite complexity to the shared FileWriter API while keeping the malformed-detection logic intact.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Decision: FileWriter overwrite pattern uses delete-then-create
2+
3+
**By:** Linus (Backend Developer)
4+
**Date:** 2026-02-26
5+
**Issue:** #58
6+
7+
## What
8+
9+
When a file needs to be overwritten (e.g., malformed SKILL.md repair), the caller should `os.Remove` the file before passing it to `FileWriter`. The FileWriter then sees it as absent and creates it normally with ➕ indicator. This avoids adding overwrite/force-write complexity to the shared `FileWriter` API.
10+
11+
## Why
12+
13+
The FileWriter's single responsibility is create-if-missing + skip-if-exists. Adding an overwrite flag would complicate the API and make it harder to reason about. The delete-then-create pattern keeps the FileWriter simple while letting callers handle their own pre-write logic.
14+
15+
## Impact
16+
17+
Any future command that needs to overwrite an existing file via FileWriter should follow this pattern: detect the condition, remove the stale file, then call `fw.Write()`.

cmd/waza/cmd_new.go

Lines changed: 72 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ package main
22

33
import (
44
"fmt"
5+
"io"
56
"os"
67
"path/filepath"
78

8-
"github.com/charmbracelet/lipgloss"
99
"github.com/spf13/cobra"
1010
"golang.org/x/term"
1111

@@ -250,25 +250,30 @@ func scaffoldInProject(cmd *cobra.Command, projectRoot, skillName, skillMD strin
250250
}
251251
}
252252

253-
files := []fileEntry{
254-
{filepath.Join(skillDir, "SKILL.md"), skillMD, "Skill definition", overwriteSkill},
255-
{filepath.Join(evalDir, "eval.yaml"), scaffold.EvalYAML(skillName, engine, model), "Eval configuration", false},
253+
// If malformed SKILL.md needs replacement, remove it so FileWriter creates fresh
254+
if overwriteSkill {
255+
_ = os.Remove(filepath.Join(skillDir, "SKILL.md"))
256+
}
257+
258+
entries := []scaffold.FileEntry{
259+
{Path: filepath.Join(skillDir, "SKILL.md"), Label: "Skill definition", Content: skillMD},
260+
{Path: filepath.Join(evalDir, "eval.yaml"), Label: "Eval configuration", Content: scaffold.EvalYAML(skillName, engine, model)},
256261
}
257262

258263
// Only add default tasks if the tasks directory is empty
259264
if !dirHasFiles(tasksDir) {
260265
tasks := scaffold.TaskFiles(skillName)
261266
for name, content := range tasks {
262-
files = append(files, fileEntry{filepath.Join(tasksDir, name), content, taskLabel(name), false})
267+
entries = append(entries, scaffold.FileEntry{Path: filepath.Join(tasksDir, name), Label: taskLabel(name), Content: content})
263268
}
264269
}
265270

266271
// Only add default fixture if the fixtures directory is empty
267272
if !dirHasFiles(fixturesDir) {
268-
files = append(files, fileEntry{filepath.Join(fixturesDir, "sample.py"), scaffold.Fixture(), "Fixture", false})
273+
entries = append(entries, scaffold.FileEntry{Path: filepath.Join(fixturesDir, "sample.py"), Label: "Fixture", Content: scaffold.Fixture()})
269274
}
270275

271-
return writeFiles(cmd, files, skillName, existing, tasksDir, fixturesDir)
276+
return writeScaffold(cmd, entries, skillName, existing, tasksDir, fixturesDir)
272277
}
273278

274279
// scaffoldStandalone creates a self-contained skill directory.
@@ -287,136 +292,98 @@ func scaffoldStandalone(cmd *cobra.Command, skillName, skillMD string, existing,
287292
}
288293
}
289294

290-
files := []fileEntry{
291-
{filepath.Join(rootDir, "SKILL.md"), skillMD, "Skill definition", overwriteSkill},
292-
{filepath.Join(evalsDir, "eval.yaml"), scaffold.EvalYAML(skillName, engine, model), "Eval configuration", false},
295+
// If malformed SKILL.md needs replacement, remove it so FileWriter creates fresh
296+
if overwriteSkill {
297+
_ = os.Remove(filepath.Join(rootDir, "SKILL.md"))
298+
}
299+
300+
entries := []scaffold.FileEntry{
301+
{Path: filepath.Join(rootDir, "SKILL.md"), Label: "Skill definition", Content: skillMD},
302+
{Path: filepath.Join(evalsDir, "eval.yaml"), Label: "Eval configuration", Content: scaffold.EvalYAML(skillName, engine, model)},
293303
}
294304

295305
// Only add default tasks if the tasks directory is empty
296306
if !dirHasFiles(tasksDir) {
297307
tasks := scaffold.TaskFiles(skillName)
298308
for name, content := range tasks {
299-
files = append(files, fileEntry{filepath.Join(tasksDir, name), content, taskLabel(name), false})
309+
entries = append(entries, scaffold.FileEntry{Path: filepath.Join(tasksDir, name), Label: taskLabel(name), Content: content})
300310
}
301311
}
302312

303313
// Only add default fixture if the fixtures directory is empty
304314
if !dirHasFiles(fixturesDir) {
305-
files = append(files, fileEntry{filepath.Join(fixturesDir, "sample.py"), scaffold.Fixture(), "Fixture", false})
315+
entries = append(entries, scaffold.FileEntry{Path: filepath.Join(fixturesDir, "sample.py"), Label: "Fixture", Content: scaffold.Fixture()})
306316
}
307317

308-
files = append(files,
309-
fileEntry{filepath.Join(workflowDir, "eval.yml"), defaultCIWorkflow(skillName), "CI pipeline", false},
310-
fileEntry{filepath.Join(rootDir, ".gitignore"), defaultGitignore(), "Build artifacts excluded", false},
311-
fileEntry{filepath.Join(rootDir, "README.md"), defaultReadme(skillName), "Getting started guide", false},
318+
entries = append(entries,
319+
scaffold.FileEntry{Path: filepath.Join(workflowDir, "eval.yml"), Label: "CI pipeline", Content: defaultCIWorkflow(skillName)},
320+
scaffold.FileEntry{Path: filepath.Join(rootDir, ".gitignore"), Label: "Build artifacts excluded", Content: defaultGitignore()},
321+
scaffold.FileEntry{Path: filepath.Join(rootDir, "README.md"), Label: "Getting started guide", Content: defaultReadme(skillName)},
312322
)
313323

314-
return writeFiles(cmd, files, skillName, existing, tasksDir, fixturesDir)
324+
return writeScaffold(cmd, entries, skillName, existing, tasksDir, fixturesDir)
315325
}
316326

317-
// fileEntry pairs a path with its content and display label for batch writing.
318-
type fileEntry struct {
319-
path string
320-
content string
321-
label string
322-
overwrite bool // when true, overwrite existing file (e.g., malformed SKILL.md)
323-
}
327+
// writeScaffold uses the shared FileWriter to create missing files, then prints
328+
// the inventory with header and summary footer.
329+
func writeScaffold(cmd *cobra.Command, entries []scaffold.FileEntry, skillName string, existing bool, tasksDir, fixturesDir string) error {
330+
out := cmd.OutOrStdout()
331+
baseDir, _ := os.Getwd() //nolint:errcheck // best-effort for display paths
324332

325-
// writeFiles writes each file, skipping any that already exist.
326-
// Prints grouped inventory output with header, section heading, and labels.
327-
// tasksDir and fixturesDir are used to show summary lines for user-owned content.
328-
func writeFiles(cmd *cobra.Command, files []fileEntry, skillName string, existing bool, tasksDir, fixturesDir string) error {
329-
greenCheck := lipgloss.NewStyle().Foreground(lipgloss.Color("2")).Render("✓")
330-
yellowPlus := lipgloss.NewStyle().Foreground(lipgloss.Color("3")).Render("+")
333+
fw := scaffold.NewFileWriter(baseDir)
334+
inv, err := fw.Write(entries)
335+
if err != nil {
336+
return err
337+
}
331338

332339
if existing {
333-
fmt.Fprintf(cmd.OutOrStdout(), "🔧 Checking skill: %s\n", skillName) //nolint:errcheck
340+
fmt.Fprintf(out, "🔧 Checking skill: %s\n", skillName) //nolint:errcheck
334341
} else {
335-
fmt.Fprintf(cmd.OutOrStdout(), "🔧 Scaffolding skill: %s\n", skillName) //nolint:errcheck
342+
fmt.Fprintf(out, "🔧 Scaffolding skill: %s\n", skillName) //nolint:errcheck
336343
}
337-
fmt.Fprintf(cmd.OutOrStdout(), "\nSkill structure:\n\n") //nolint:errcheck
344+
fmt.Fprintf(out, "\nSkill structure:\n\n") //nolint:errcheck
338345

339-
baseDir, _ := os.Getwd() //nolint:errcheck // best-effort for display paths
340-
created := 0
341-
for _, f := range files {
342-
relPath := f.path
343-
if baseDir != "" {
344-
if abs, absErr := filepath.Abs(f.path); absErr == nil {
345-
if rel, relErr := filepath.Rel(baseDir, abs); relErr == nil {
346-
relPath = rel
347-
}
348-
}
349-
}
346+
inv.Fprint(out)
350347

351-
if _, err := os.Stat(f.path); err == nil {
352-
if f.overwrite {
353-
// Overwrite existing file (e.g., malformed SKILL.md being repaired)
354-
if err := os.WriteFile(f.path, []byte(f.content), 0o644); err != nil {
355-
return fmt.Errorf("failed to write %s: %w", f.path, err)
356-
}
357-
fmt.Fprintf(cmd.OutOrStdout(), " %s %-40s %s (updated)\n", yellowPlus, relPath, f.label) //nolint:errcheck
358-
created++
359-
continue
360-
}
361-
fmt.Fprintf(cmd.OutOrStdout(), " %s %-40s %s\n", greenCheck, relPath, f.label) //nolint:errcheck
362-
continue
363-
}
348+
// Show summary lines for user-owned tasks/fixtures directories
349+
printDirSummary(out, inv, tasksDir, baseDir, "Tasks")
350+
printDirSummary(out, inv, fixturesDir, baseDir, "Fixtures")
364351

365-
if err := os.WriteFile(f.path, []byte(f.content), 0o644); err != nil {
366-
return fmt.Errorf("failed to write %s: %w", f.path, err)
367-
}
368-
fmt.Fprintf(cmd.OutOrStdout(), " %s %-40s %s\n", yellowPlus, relPath, f.label) //nolint:errcheck
369-
created++
352+
created := inv.CreatedCount()
353+
354+
fmt.Fprintln(out) //nolint:errcheck
355+
if created == 0 {
356+
fmt.Fprintf(out, "✅ Project up to date.\n") //nolint:errcheck
357+
} else if created == len(entries) {
358+
fmt.Fprintf(out, "✅ Skill created — %d file(s) scaffolded.\n", created) //nolint:errcheck
359+
} else {
360+
fmt.Fprintf(out, "✅ Repaired — %d item(s) added.\n", created) //nolint:errcheck
370361
}
371362

372-
// Show summary lines for user-owned tasks/fixtures directories
373-
if taskCount := dirFileCount(tasksDir); taskCount > 0 {
374-
hasDefaultTasks := false
375-
for _, f := range files {
376-
if filepath.Dir(f.path) == tasksDir {
377-
hasDefaultTasks = true
378-
break
379-
}
380-
}
381-
if !hasDefaultTasks {
382-
relDir := tasksDir
383-
if abs, err := filepath.Abs(tasksDir); err == nil {
384-
if rel, err := filepath.Rel(baseDir, abs); err == nil {
385-
relDir = rel
386-
}
387-
}
388-
fmt.Fprintf(cmd.OutOrStdout(), " %s %-40s %s\n", greenCheck, relDir+string(filepath.Separator), fmt.Sprintf("Tasks (%d files)", taskCount)) //nolint:errcheck
389-
}
363+
return nil
364+
}
365+
366+
// printDirSummary prints a summary line for a directory that has existing files
367+
// but wasn't part of the scaffolded entries.
368+
func printDirSummary(out io.Writer, inv *scaffold.Inventory, dir, baseDir, label string) {
369+
count := dirFileCount(dir)
370+
if count == 0 {
371+
return
390372
}
391-
if fixtureCount := dirFileCount(fixturesDir); fixtureCount > 0 {
392-
hasDefaultFixtures := false
393-
for _, f := range files {
394-
if filepath.Dir(f.path) == fixturesDir {
395-
hasDefaultFixtures = true
396-
break
397-
}
398-
}
399-
if !hasDefaultFixtures {
400-
relDir := fixturesDir
401-
if abs, err := filepath.Abs(fixturesDir); err == nil {
402-
if rel, err := filepath.Rel(baseDir, abs); err == nil {
403-
relDir = rel
404-
}
405-
}
406-
fmt.Fprintf(cmd.OutOrStdout(), " %s %-40s %s\n", greenCheck, relDir+string(filepath.Separator), fmt.Sprintf("Fixtures (%d files)", fixtureCount)) //nolint:errcheck
373+
374+
for _, item := range inv.Items {
375+
if filepath.Dir(item.Entry.Path) == dir {
376+
return
407377
}
408378
}
409379

410-
fmt.Fprintln(cmd.OutOrStdout()) //nolint:errcheck
411-
if created == 0 {
412-
fmt.Fprintf(cmd.OutOrStdout(), "%s Project up to date.\n", greenCheck) //nolint:errcheck
413-
} else if created == len(files) {
414-
fmt.Fprintf(cmd.OutOrStdout(), "✅ Skill created — %d file(s) scaffolded.\n", created) //nolint:errcheck
415-
} else {
416-
fmt.Fprintf(cmd.OutOrStdout(), "✅ Repaired — %d item(s) added.\n", created) //nolint:errcheck
380+
relDir := dir
381+
if abs, err := filepath.Abs(dir); err == nil {
382+
if rel, err := filepath.Rel(baseDir, abs); err == nil {
383+
relDir = rel
384+
}
417385
}
418-
419-
return nil
386+
fmt.Fprintf(out, " ✅ %s %s (%d files)\n", relDir+string(filepath.Separator), label, count) //nolint:errcheck
420387
}
421388

422389
// taskLabel returns a descriptive label for a task file.

cmd/waza/cmd_new_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ func TestNewCommand_NoOverwriteSafety(t *testing.T) {
127127
require.NoError(t, err)
128128
assert.Equal(t, customContent, string(data), "valid SKILL.md should not be overwritten")
129129

130-
// Output should mention for skipped file
131-
assert.Contains(t, buf.String(), "")
130+
// Output should mention for skipped file
131+
assert.Contains(t, buf.String(), "")
132132
assert.FileExists(t, filepath.Join(dir, "evals", "my-skill", "eval.yaml"))
133133
}
134134

@@ -160,8 +160,8 @@ func TestNewCommand_IdempotentWithExistingSkillMD(t *testing.T) {
160160
require.NoError(t, err)
161161
assert.Equal(t, validSkillMD, string(data), "existing SKILL.md should not be overwritten")
162162

163-
// Output should mention for skipped file
164-
assert.Contains(t, buf.String(), "")
163+
// Output should mention for skipped file
164+
assert.Contains(t, buf.String(), "")
165165

166166
// Eval files should be created
167167
assert.FileExists(t, filepath.Join(dir, "evals", "my-skill", "eval.yaml"))
@@ -192,7 +192,7 @@ func TestNewCommand_IdempotentRunTwice(t *testing.T) {
192192
require.NoError(t, cmd2.Execute())
193193

194194
output := buf.String()
195-
assert.Contains(t, output, "")
195+
assert.Contains(t, output, "")
196196
assert.Contains(t, output, "Project up to date")
197197
}
198198

@@ -227,7 +227,7 @@ func TestNewCommand_EmptySkillMD_NonTTY_OverwritesWithDefaults(t *testing.T) {
227227
// Warning should appear in output
228228
output := buf.String()
229229
assert.Contains(t, output, "empty or malformed")
230-
assert.Contains(t, output, "updated")
230+
assert.Contains(t, output, "")
231231

232232
// Other eval files should still be created
233233
assert.FileExists(t, filepath.Join(dir, "evals", "my-skill", "eval.yaml"))
@@ -357,9 +357,9 @@ func TestNewCommand_EmptySkillMD_EvalFilesPreExist(t *testing.T) {
357357
require.NoError(t, err)
358358
assert.Equal(t, "existing eval", string(evalData), "eval.yaml should not be overwritten")
359359

360-
// Output should show updated for SKILL.md, for eval.yaml
360+
// Output should show for repaired SKILL.md, for existing eval.yaml
361361
output := buf.String()
362-
assert.Contains(t, output, "updated")
362+
assert.Contains(t, output, "")
363363
}
364364

365365
// ── Name Validation Tests ──────────────────────────────────────────────────────

0 commit comments

Comments
 (0)