Author SHA1 Message Date
clawbot 7de4d6ec1c Rewrite script/test to the canonical race-enabled pattern (closes #67)
check / check (push) Successful in 51s
script/test now runs go test -timeout 30s -race -cover ./..., quiet on success and rerunning verbose on failure. -race exposed a data race on the process-global apex/log logger: Init reconfigured it on every CLI run while other goroutines logged through it. internal/log now mutates the global under the write lock and reads it under the read lock; WithError is dropped because its Entry logged outside that lock, and run() reports a failed command's error via log.Errorf instead (still shown under -q). The corruption fuzz test is scaled to 1500 files / 100 iterations to fit the budget under -race; it pins the same claims at reduced breadth. Independent review ran the Docker lint gate uncached.

Model: opus-4-8 (implementation, review)
model: claude-fable-5
2026-09-21 15:17:50 +02:00
clawbot 6de3f1d714 Add .editorconfig, broaden .gitignore, drop dead Drone refs (closes #72)
check / check (push) Failing after 1s
Add the canonical .editorconfig from sneak/prompts verbatim. Broaden .gitignore to cover secrets (.env, .env.*, *.key, *.pem), OS files, editor files, and Go artifacts while keeping every repo-specific entry; no tracked file becomes ignored, and bin/gitrev.sh stays tracked despite the /bin/ rule. Remove the stale .drone.yml ignore entry and the DRONE_COMMIT_SHA branch from bin/gitrev.sh, left from the Gitea Actions migration; the GITREV and git-describe paths still work.

A reader may trip over .editorconfig saying four spaces for every file while Go files are tab-indented: gofmt governs Go, editorconfig does not, and the file is taken verbatim by policy.

Disclosure: the worker's cold docker build hit the 10s test timeout in the gpg signature test; this change has no Go code, and that timeout is tracked by issues 67 and 62.

Model: opus-4-8 (implementation); fable-5-1 (merge)
2026-09-21 09:56:15 +02:00
8 changed files with 93 additions and 151 deletions
+12
View File
@@ -0,0 +1,12 @@
root = true
[*]
indent_style = space
indent_size = 4
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
[Makefile]
indent_style = tab
+21 -2
View File
@@ -10,5 +10,24 @@ modcache.tzst
# Generated manifest files
.index.mf
# Stale files
.drone.yml
# Secrets
.env
.env.*
*.key
*.pem
# OS files
.DS_Store
Thumbs.db
# Editor files
*.swp
*.swo
*~
.idea/
.vscode/
# Go build artifacts
*.log
*.out
*.test
+6 -3
View File
@@ -24,8 +24,12 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
# Completed Steps
- 2026-09-21: fixed the `-v` collision between `--verbose` and `--version`;
verbose owns `-v`, and version answers to `--version` and `-V` (#64)
- 2026-09-21: rewrote `script/test` to the canonical pattern (30s timeout,
`-race -cover`, quiet-first with verbose-on-failure rerun) and fixed the
process-global logger data race it surfaced (#67)
- 2026-09-21: added the canonical `.editorconfig`, made `.gitignore` cover
secrets, OS, editor, and Go artifacts, and removed the dead Drone CI
references from `.gitignore` and `bin/gitrev.sh` (#72)
- 2026-08-09: added `.prettierrc`/`.prettierignore`, gave `script/fmt` and
`script/fmt-check` one shared prettier file set via `script/prettier`, dropped
the `|| true` that hid prettier failures, and added a node-based Dockerfile
@@ -64,7 +68,6 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
- Move FORMAT.md from repo root to docs/ and update the AGENTS.md reference
- Pin Makefile-installed Go tools (`protoc-gen-go@v1.28.1`,
`golangci-lint@v2.12.2`) by module hash, not mutable tag
- Set `make test` timeout to 30s (currently 10s)
- Add explicit README "Rationale" heading (content exists under other
names); name the author in the README Description first line
- Reconcile root-level AGENTS.md with directory-hygiene policy (keep or
-5
View File
@@ -1,10 +1,5 @@
#!/bin/bash
#
if [[ ! -z "$DRONE_COMMIT_SHA" ]]; then
echo "${DRONE_COMMIT_SHA:0:7}"
exit 0
fi
if [[ ! -z "$GITREV" ]]; then
echo $GITREV
else
+11 -73
View File
@@ -27,7 +27,6 @@ const (
testManifest = "/manifest.mf"
testFlagBase = "--base"
testFlagNoExtra = "--no-extra-files"
testFlagVersion = "--version"
)
var errSimulatedWrite = errors.New("simulated write failure")
@@ -103,7 +102,7 @@ func TestVersionCommand(t *testing.T) {
t.Parallel()
fs := afero.NewMemMapFs()
opts := testOpts([]string{testApp, cmdVersion}, fs)
opts := testOpts([]string{testApp, "version"}, fs)
exitCode := runCLI(opts)
@@ -114,69 +113,6 @@ func TestVersionCommand(t *testing.T) {
assert.Contains(t, stdout, "abc123")
}
// TestVFlagCollision covers the -v/--verbose vs --version flag interaction
// (issue #64). Verbose owns -v; version answers to --version and -V. None of
// these invocations may produce a parser error, and the two ways of asking
// for the version must print the same thing.
func TestVFlagCollision(t *testing.T) {
t.Parallel()
// Invocations that must print the version and exit 0.
versionCases := map[string][]string{
"long version flag": {testApp, testFlagVersion},
"short version flag": {testApp, "-V"},
"verbose then version": {testApp, "-v", testFlagVersion},
"long verbose and version": {testApp, "--verbose", testFlagVersion},
}
for name, args := range versionCases {
t.Run(name, func(t *testing.T) {
t.Parallel()
opts := testOpts(args, afero.NewMemMapFs())
exitCode := runCLI(opts)
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStdout(t, opts), mfer.Version)
assert.NotContains(t, testStderr(t, opts), "two forms of the same flag")
})
}
// Invocations that must enable verbose and exit 0 without a parser error.
verboseCases := map[string][]string{
"short verbose flag": {testApp, "-v"},
"long verbose flag": {testApp, "--verbose"},
}
for name, args := range verboseCases {
t.Run(name, func(t *testing.T) {
t.Parallel()
opts := testOpts(args, afero.NewMemMapFs())
exitCode := runCLI(opts)
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStdout(t, opts), cmdGenerate,
"root should show help listing subcommands")
assert.Empty(t, testStderr(t, opts))
})
}
}
// TestVersionFlagAndCommandMatch asserts that "mfer --version" and
// "mfer version" produce identical output (issue #64).
func TestVersionFlagAndCommandMatch(t *testing.T) {
t.Parallel()
flagOpts := testOpts([]string{testApp, testFlagVersion}, afero.NewMemMapFs())
require.Equal(t, 0, runCLI(flagOpts))
cmdOpts := testOpts([]string{testApp, cmdVersion}, afero.NewMemMapFs())
require.Equal(t, 0, runCLI(cmdOpts))
assert.Equal(t, testStdout(t, flagOpts), testStdout(t, cmdOpts))
}
func TestHelpCommand(t *testing.T) {
t.Parallel()
@@ -743,11 +679,13 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
fs := afero.NewMemMapFs()
rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data
// Create many small files with random names to generate a ~1MB manifest
// Each manifest entry is roughly 50-60 bytes, so we need ~20000 files
// Create many small files with random names so the manifest has many
// entries and random single-byte flips land at varied offsets. Each
// manifest entry is roughly 50-60 bytes. Kept modest so the suite stays
// within its wall-clock budget under -race.
require.NoError(t, fs.MkdirAll(testDir, 0o755))
numFiles := 20000
numFiles := 1500
for range numFiles {
// Generate random filename
filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat",
@@ -763,11 +701,11 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode := runCLI(opts)
require.Equal(t, 0, exitCode, "generate should succeed")
// Read the valid manifest and verify it's approximately 1MB
// Read the valid manifest and verify it has real size.
validManifest, err := afero.ReadFile(fs, testManifest)
require.NoError(t, err)
require.GreaterOrEqual(t, len(validManifest), 1024*1024,
"manifest should be at least 1MB, got %d bytes", len(validManifest))
require.GreaterOrEqual(t, len(validManifest), 64*1024,
"manifest should be at least 64KB, got %d bytes", len(validManifest))
t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles)
// First corruption: truncate the manifest
@@ -790,8 +728,8 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode = runCLI(opts)
require.Equal(t, 0, exitCode, "check should pass with valid manifest")
// Now do 500 random corruption iterations
for i := range 500 {
// Now do 100 random corruption iterations
for i := range 100 {
// Corrupt: write a random byte at a random offset
corrupted := make([]byte, len(validManifest))
copy(corrupted, validManifest)
+3 -26
View File
@@ -18,7 +18,6 @@ const (
cmdGenerate = "generate"
cmdCheck = "check"
cmdExport = "export"
cmdVersion = "version"
flagProgress = "progress"
@@ -68,12 +67,6 @@ func (mfa *CLIApp) VersionString() string {
return mfer.Version
}
// printVersion writes the version line shared by the --version flag and the
// version subcommand, so both produce identical output.
func (mfa *CLIApp) printVersion() {
_, _ = fmt.Fprintf(mfa.Stdout, "%s version %s\n", mfa.appname, mfa.VersionString())
}
func (mfa *CLIApp) printBanner() {
if log.GetLevel() <= log.InfoLevel {
_, _ = fmt.Fprintln(mfa.Stdout, banner)
@@ -272,10 +265,10 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
func (mfa *CLIApp) versionCommand() *cli.Command {
return &cli.Command{
Name: cmdVersion,
Name: "version",
Usage: "Show version",
Action: func(_ *cli.Context) error {
mfa.printVersion()
_, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString())
return nil
},
@@ -331,20 +324,6 @@ func (mfa *CLIApp) run(args []string) {
log.SetOutput(mfa.Stdout, mfa.Stderr)
log.Init()
// -v means verbose, not version. urfave/cli's built-in version flag
// claims -v by default, colliding with the -v verbose alias used here and
// on every subcommand; that collision makes "mfer -v --version" fail to
// parse. Verbose is the more common meaning of -v in tools that offer
// both, so verbose keeps -v and the version flag takes the capital -V.
// VersionFlag and VersionPrinter are urfave/cli package globals; run() is
// serialized in tests, so assigning them here is safe.
cli.VersionFlag = &cli.BoolFlag{
Name: cmdVersion,
Aliases: []string{"V"},
Usage: "print the version",
}
cli.VersionPrinter = func(_ *cli.Context) { mfa.printVersion() }
mfa.app = &cli.App{
Name: mfa.appname,
Usage: "Manifest generator",
@@ -352,13 +331,11 @@ func (mfa *CLIApp) run(args []string) {
EnableBashCompletion: true,
Writer: mfa.Stdout,
ErrWriter: mfa.Stderr,
Flags: commonFlags(),
Action: func(c *cli.Context) error {
if c.Args().Len() > 0 {
return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First())
}
mfa.setVerbosity(c)
mfa.printBanner()
return cli.ShowAppHelp(c)
@@ -380,6 +357,6 @@ func (mfa *CLIApp) run(args []string) {
if err != nil {
mfa.exitCode = 1
log.WithError(err).Debugf("exiting")
log.Errorf("%s", err)
}
}
+34 -41
View File
@@ -112,13 +112,16 @@ func DisableStyling() {
}
// Init initializes the logger with the CLI handler and default log level.
//
// It reconfigures the process-global apex/log logger under the write lock so
// the global is never mutated while another goroutine holds the read lock to
// read it in emit. Without this, parallel callers (e.g. the test suite) race
// Init's SetLevel/SetHandler against concurrent log calls.
func Init() {
mu.RLock()
mu.Lock()
defer mu.Unlock()
w := stderr
mu.RUnlock()
log.SetHandler(acli.New(w))
log.SetHandler(acli.New(stderr))
log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves
}
@@ -130,74 +133,66 @@ func isEnabled(l Level) bool {
return l >= currentLevel
}
// emit calls fn while holding the read lock if messages at level l are
// enabled. Holding the read lock across the apex/log call keeps the global
// logger from being read while Init reconfigures it under the write lock.
func emit(l Level, fn func()) {
mu.RLock()
defer mu.RUnlock()
if l >= currentLevel {
fn()
}
}
// Fatalf logs a formatted message at fatal level.
func Fatalf(format string, args ...any) {
if isEnabled(FatalLevel) {
log.Fatalf(format, args...)
}
emit(FatalLevel, func() { log.Fatalf(format, args...) })
}
// Fatal logs a message at fatal level.
func Fatal(arg string) {
if isEnabled(FatalLevel) {
log.Fatal(arg)
}
emit(FatalLevel, func() { log.Fatal(arg) })
}
// Errorf logs a formatted message at error level.
func Errorf(format string, args ...any) {
if isEnabled(ErrorLevel) {
log.Errorf(format, args...)
}
emit(ErrorLevel, func() { log.Errorf(format, args...) })
}
// Error logs a message at error level.
func Error(arg string) {
if isEnabled(ErrorLevel) {
log.Error(arg)
}
emit(ErrorLevel, func() { log.Error(arg) })
}
// Warnf logs a formatted message at warn level.
func Warnf(format string, args ...any) {
if isEnabled(WarnLevel) {
log.Warnf(format, args...)
}
emit(WarnLevel, func() { log.Warnf(format, args...) })
}
// Warn logs a message at warn level.
func Warn(arg string) {
if isEnabled(WarnLevel) {
log.Warn(arg)
}
emit(WarnLevel, func() { log.Warn(arg) })
}
// Infof logs a formatted message at info level.
func Infof(format string, args ...any) {
if isEnabled(InfoLevel) {
log.Infof(format, args...)
}
emit(InfoLevel, func() { log.Infof(format, args...) })
}
// Info logs a message at info level.
func Info(arg string) {
if isEnabled(InfoLevel) {
log.Info(arg)
}
emit(InfoLevel, func() { log.Info(arg) })
}
// Verbosef logs a formatted message at verbose level.
func Verbosef(format string, args ...any) {
if isEnabled(VerboseLevel) {
log.Infof(format, args...)
}
emit(VerboseLevel, func() { log.Infof(format, args...) })
}
// Verbose logs a message at verbose level.
func Verbose(arg string) {
if isEnabled(VerboseLevel) {
log.Info(arg)
}
emit(VerboseLevel, func() { log.Info(arg) })
}
// Debugf logs a formatted message at debug level with caller location.
@@ -216,7 +211,10 @@ func Debug(arg string) {
// DebugReal logs at debug level with caller info from the specified stack depth.
func DebugReal(arg string, cs int) {
if !isEnabled(DebugLevel) {
mu.RLock()
defer mu.RUnlock()
if DebugLevel < currentLevel {
return
}
@@ -275,11 +273,6 @@ func GetLevel() Level {
return currentLevel
}
// WithError returns a log entry with the error attached.
func WithError(e error) *log.Entry {
return log.Log.WithError(e)
}
// Progressf prints a progress message that overwrites the current line.
// Use ProgressDone() when progress is complete to move to the next line.
func Progressf(format string, args ...any) {
+6 -1
View File
@@ -17,7 +17,12 @@ ensure_pb() {
main() {
cd "$ROOT"
ensure_pb
go test -v --timeout 10s ./...
go test -timeout 30s -race -cover ./... ||
{
echo "--- Rerunning with -v for details ---"
go test -timeout 30s -race -v ./...
exit 1
}
}
main "$@"