Develop - #12
Conversation
Add internal/detect with marker-file detectors for Flutter (vs pure Dart; app/module/plugin), React Native (bare, Expo managed, Expo prebuild), native Android (app vs library, KMP flag), and native iOS (Xcode/SPM/Podfile). A depth-limited prune-on-detect scanner with skip lists and a containment sweep attributes android/ios folders to their Flutter or RN parent and surfaces each monorepo project once. Add the Cobra-based anvil detect command (table and --json, with --path and --depth). 16 fixture-tree tests cover the edge cases (node_modules exclusion, monorepo, KMP, plugin example pruning). Also apply the no-emojis, no-dash-connectors, minimal-comments style across docs and record it in CLAUDE.md.
feat(detect): stack detection engine and anvil detect
* feat(build): guided build lifecycle and anvil build Add the driver contract (internal/driver) and drivers for Flutter, React Native, native Android, native iOS, Swift (SPM), and Kotlin/JVM, with --flavor threaded into build and test steps. Add the runner (internal/pipeline) that streams combined output, captures exit codes, classifies results, collects artifacts, and fail-fasts. Add internal/tui with a Bubble Tea view and a plain non-TTY renderer, and the anvil build command (--path/--target/--flavor/--release/--dry-run/--plain). Refine detection so Gradle is not always Android and Package.swift is Swift, adding Swift and Kotlin stacks. Tests cover driver steps, the runner via a real subprocess, and the plain renderer. * fix: remove unused dirExists helper (staticcheck U1000)
Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv.
📝 WalkthroughWalkthroughThe change adds Cobra-based ChangesAnvil lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
cmd/root.go (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
Executecomment.Line 20 only restates the function behavior. As per coding guidelines, “Avoid unnecessary comments. Use self-explanatory names, and add comments only for non-obvious constraints that cannot be expressed in code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/root.go` at line 20, Remove the redundant comment immediately preceding the Execute function in cmd/root.go, leaving the function implementation unchanged.Source: Coding guidelines
cmd/sign.go (1)
41-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider validating
--export-methodagainst the documented values.The flag help text says only
developmentorad-hocare supported, but any string is accepted and written verbatim intoExportOptions.plist(viasetupIOSSigning), deferring the error to a later, less obviousxcodebuildfailure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/sign.go` around lines 41 - 42, Validate signMethod from the --export-method flag before setupIOSSigning writes ExportOptions.plist, accepting only the documented development and ad-hoc values. Reject unsupported values with a clear user-facing error and preserve the existing behavior for valid values.internal/sign/sign.go (2)
40-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
exec.CommandContextper golangci-lint'snoctxrule.Both
keytoolinvocations useexec.Commandwithout a context, flagged bynoctx. Since the repo's./checkwrapper runs lint as part of the gate, this should be fixed at both sites with the same pattern (accept/pass acontext.Context, or usecontext.Background()if no cancellation is currently threaded through).
internal/sign/sign.go#L40: changeexec.Command("keytool", args...)toexec.CommandContext(ctx, "keytool", args...), threading acontext.ContextintoGenerateKeystore.internal/sign/sign_test.go#L36: changeexec.Command("keytool", "-list", ...)toexec.CommandContext(context.Background(), "keytool", "-list", ...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sign/sign.go` at line 40, Replace both context-free keytool invocations with exec.CommandContext: thread a context.Context into GenerateKeystore in internal/sign/sign.go at line 40, and use context.Background() in internal/sign/sign_test.go at line 36. Update affected callers and imports as needed while preserving existing command arguments and behavior.Source: Linters/SAST tools
116-116: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCheck
f.Close()errors per golangci-lint'serrcheckrule.Both
WireAndroidGradleandEnsureGitignoreopen a file withos.OpenFileanddefer f.Close()without checking the returned error, whicherrcheckflags and which the project's./checkgate would fail on.
internal/sign/sign.go#L116: capture the close error inWireAndroidGradle(e.g., via a named return or explicit check) instead ofdefer f.Close().internal/sign/sign.go#L168: same fix inEnsureGitignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sign/sign.go` at line 116, Handle errors returned by deferred file closes in both WireAndroidGradle and EnsureGitignore instead of discarding f.Close() results. Update internal/sign/sign.go at lines 116-116 and 168-168, using named returns or explicit deferred checks, and propagate any close error appropriately so errcheck passes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 80-85: Update the “Where things are” section in CLAUDE.md to
document the implemented build and sign commands under cmd/ and the signing
implementation under internal/sign, while preserving the existing root, version,
detect, driver, pipeline, documentation, and task references.
In `@cmd/build.go`:
- Around line 103-105: The output writes in chooseProject must handle errors:
propagate failures from both fmt.Fprintln/Fprintf calls through the function’s
existing error path. In internal/pipeline/pipeline_test.go lines 21-23,
explicitly assert the write result or discard it to satisfy errcheck.
In `@cmd/sign.go`:
- Around line 164-173: Validate teamID after the interactive huh form completes
in the team-ID resolution block, matching resolveSecret’s empty-value behavior.
If the prompt returns an empty value, return an error instructing the user to
set --team-id or ANVIL_TEAM_ID before proceeding to WriteExportOptions.
- Around line 63-72: Update the control flow in the signing command around the
len(extra) check so signDryRun is handled before the empty-extra confirmation
branch. Ensure Android dry-runs call printPlan and return without printing
“Signing configured,” while preserving the existing confirmation for non-dry-run
cases with no extra actions.
In `@docs/ARCHITECTURE.md`:
- Line 52: Align the Driver contract sketch and related phase-selection text in
ARCHITECTURE.md with internal/driver.Driver by removing or revising the
unsupported Capabilities requirement and documenting only the implemented
methods: Name, Steps, Classify, and Artifacts. Keep the documentation consistent
with the registry and existing drivers rather than changing implementation or
tests.
- Around line 8-16: Update the architecture diagram fenced block in
ARCHITECTURE.md to specify the text language identifier, preserving the diagram
content unchanged.
In `@internal/detect/detect.go`:
- Around line 67-69: Update the walk callback in detect to return any non-nil
filesystem traversal err immediately before accessing d.IsDir(), while
preserving the existing behavior of skipping non-directory entries. This ensures
permission and I/O errors propagate instead of being silently ignored.
In `@internal/tui/plain.go`:
- Around line 15-43: Update Plain to track an outputOK flag and capture errors
from every fmt.Fprint/Fprintf call to out, marking outputOK false whenever a
write fails. Return success && outputOK so pipeline.KindDone cannot mask output
failures, while preserving the existing build success handling.
In `@README.md`:
- Around line 7-8: Update the README capability descriptions and
milestone/status sections to match the implementation: describe build and
signing as available, remove or clearly label upload as planned, and add Swift
and Kotlin to the v1 supported stack list. Ensure all related statements remain
consistent across the command overview and referenced milestone sections.
---
Nitpick comments:
In `@cmd/root.go`:
- Line 20: Remove the redundant comment immediately preceding the Execute
function in cmd/root.go, leaving the function implementation unchanged.
In `@cmd/sign.go`:
- Around line 41-42: Validate signMethod from the --export-method flag before
setupIOSSigning writes ExportOptions.plist, accepting only the documented
development and ad-hoc values. Reject unsupported values with a clear
user-facing error and preserve the existing behavior for valid values.
In `@internal/sign/sign.go`:
- Line 40: Replace both context-free keytool invocations with
exec.CommandContext: thread a context.Context into GenerateKeystore in
internal/sign/sign.go at line 40, and use context.Background() in
internal/sign/sign_test.go at line 36. Update affected callers and imports as
needed while preserving existing command arguments and behavior.
- Line 116: Handle errors returned by deferred file closes in both
WireAndroidGradle and EnsureGitignore instead of discarding f.Close() results.
Update internal/sign/sign.go at lines 116-116 and 168-168, using named returns
or explicit deferred checks, and propagate any close error appropriately so
errcheck passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d826798e-5686-468a-bce9-593e16d4f781
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (37)
CHANGELOG.mdCLAUDE.mdREADME.mdcmd/build.gocmd/detect.gocmd/root.gocmd/sign.gocmd/version.godocs/ARCHITECTURE.mddocs/ROADMAP.mdgo.modinternal/detect/android.gointernal/detect/detect.gointernal/detect/detect_test.gointernal/detect/flutter.gointernal/detect/ios.gointernal/detect/kotlin.gointernal/detect/reactnative.gointernal/detect/swift.gointernal/driver/android.gointernal/driver/driver.gointernal/driver/driver_test.gointernal/driver/flutter.gointernal/driver/ios.gointernal/driver/kotlin.gointernal/driver/reactnative.gointernal/driver/registry.gointernal/driver/swift.gointernal/pipeline/pipeline.gointernal/pipeline/pipeline_test.gointernal/sign/sign.gointernal/sign/sign_test.gointernal/tui/plain.gointernal/tui/plain_test.gointernal/tui/tui.gotasks/lessons.mdtasks/todo.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
🧹 Nitpick comments (4)
cmd/root.go (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
Executecomment.Line 20 only restates the function behavior. As per coding guidelines, “Avoid unnecessary comments. Use self-explanatory names, and add comments only for non-obvious constraints that cannot be expressed in code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/root.go` at line 20, Remove the redundant comment immediately preceding the Execute function in cmd/root.go, leaving the function implementation unchanged.Source: Coding guidelines
cmd/sign.go (1)
41-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider validating
--export-methodagainst the documented values.The flag help text says only
developmentorad-hocare supported, but any string is accepted and written verbatim intoExportOptions.plist(viasetupIOSSigning), deferring the error to a later, less obviousxcodebuildfailure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/sign.go` around lines 41 - 42, Validate signMethod from the --export-method flag before setupIOSSigning writes ExportOptions.plist, accepting only the documented development and ad-hoc values. Reject unsupported values with a clear user-facing error and preserve the existing behavior for valid values.internal/sign/sign.go (2)
40-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
exec.CommandContextper golangci-lint'snoctxrule.Both
keytoolinvocations useexec.Commandwithout a context, flagged bynoctx. Since the repo's./checkwrapper runs lint as part of the gate, this should be fixed at both sites with the same pattern (accept/pass acontext.Context, or usecontext.Background()if no cancellation is currently threaded through).
internal/sign/sign.go#L40: changeexec.Command("keytool", args...)toexec.CommandContext(ctx, "keytool", args...), threading acontext.ContextintoGenerateKeystore.internal/sign/sign_test.go#L36: changeexec.Command("keytool", "-list", ...)toexec.CommandContext(context.Background(), "keytool", "-list", ...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sign/sign.go` at line 40, Replace both context-free keytool invocations with exec.CommandContext: thread a context.Context into GenerateKeystore in internal/sign/sign.go at line 40, and use context.Background() in internal/sign/sign_test.go at line 36. Update affected callers and imports as needed while preserving existing command arguments and behavior.Source: Linters/SAST tools
116-116: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCheck
f.Close()errors per golangci-lint'serrcheckrule.Both
WireAndroidGradleandEnsureGitignoreopen a file withos.OpenFileanddefer f.Close()without checking the returned error, whicherrcheckflags and which the project's./checkgate would fail on.
internal/sign/sign.go#L116: capture the close error inWireAndroidGradle(e.g., via a named return or explicit check) instead ofdefer f.Close().internal/sign/sign.go#L168: same fix inEnsureGitignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sign/sign.go` at line 116, Handle errors returned by deferred file closes in both WireAndroidGradle and EnsureGitignore instead of discarding f.Close() results. Update internal/sign/sign.go at lines 116-116 and 168-168, using named returns or explicit deferred checks, and propagate any close error appropriately so errcheck passes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 80-85: Update the “Where things are” section in CLAUDE.md to
document the implemented build and sign commands under cmd/ and the signing
implementation under internal/sign, while preserving the existing root, version,
detect, driver, pipeline, documentation, and task references.
In `@cmd/build.go`:
- Around line 103-105: The output writes in chooseProject must handle errors:
propagate failures from both fmt.Fprintln/Fprintf calls through the function’s
existing error path. In internal/pipeline/pipeline_test.go lines 21-23,
explicitly assert the write result or discard it to satisfy errcheck.
In `@cmd/sign.go`:
- Around line 164-173: Validate teamID after the interactive huh form completes
in the team-ID resolution block, matching resolveSecret’s empty-value behavior.
If the prompt returns an empty value, return an error instructing the user to
set --team-id or ANVIL_TEAM_ID before proceeding to WriteExportOptions.
- Around line 63-72: Update the control flow in the signing command around the
len(extra) check so signDryRun is handled before the empty-extra confirmation
branch. Ensure Android dry-runs call printPlan and return without printing
“Signing configured,” while preserving the existing confirmation for non-dry-run
cases with no extra actions.
In `@docs/ARCHITECTURE.md`:
- Line 52: Align the Driver contract sketch and related phase-selection text in
ARCHITECTURE.md with internal/driver.Driver by removing or revising the
unsupported Capabilities requirement and documenting only the implemented
methods: Name, Steps, Classify, and Artifacts. Keep the documentation consistent
with the registry and existing drivers rather than changing implementation or
tests.
- Around line 8-16: Update the architecture diagram fenced block in
ARCHITECTURE.md to specify the text language identifier, preserving the diagram
content unchanged.
In `@internal/detect/detect.go`:
- Around line 67-69: Update the walk callback in detect to return any non-nil
filesystem traversal err immediately before accessing d.IsDir(), while
preserving the existing behavior of skipping non-directory entries. This ensures
permission and I/O errors propagate instead of being silently ignored.
In `@internal/tui/plain.go`:
- Around line 15-43: Update Plain to track an outputOK flag and capture errors
from every fmt.Fprint/Fprintf call to out, marking outputOK false whenever a
write fails. Return success && outputOK so pipeline.KindDone cannot mask output
failures, while preserving the existing build success handling.
In `@README.md`:
- Around line 7-8: Update the README capability descriptions and
milestone/status sections to match the implementation: describe build and
signing as available, remove or clearly label upload as planned, and add Swift
and Kotlin to the v1 supported stack list. Ensure all related statements remain
consistent across the command overview and referenced milestone sections.
---
Nitpick comments:
In `@cmd/root.go`:
- Line 20: Remove the redundant comment immediately preceding the Execute
function in cmd/root.go, leaving the function implementation unchanged.
In `@cmd/sign.go`:
- Around line 41-42: Validate signMethod from the --export-method flag before
setupIOSSigning writes ExportOptions.plist, accepting only the documented
development and ad-hoc values. Reject unsupported values with a clear
user-facing error and preserve the existing behavior for valid values.
In `@internal/sign/sign.go`:
- Line 40: Replace both context-free keytool invocations with
exec.CommandContext: thread a context.Context into GenerateKeystore in
internal/sign/sign.go at line 40, and use context.Background() in
internal/sign/sign_test.go at line 36. Update affected callers and imports as
needed while preserving existing command arguments and behavior.
- Line 116: Handle errors returned by deferred file closes in both
WireAndroidGradle and EnsureGitignore instead of discarding f.Close() results.
Update internal/sign/sign.go at lines 116-116 and 168-168, using named returns
or explicit deferred checks, and propagate any close error appropriately so
errcheck passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d826798e-5686-468a-bce9-593e16d4f781
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (37)
CHANGELOG.mdCLAUDE.mdREADME.mdcmd/build.gocmd/detect.gocmd/root.gocmd/sign.gocmd/version.godocs/ARCHITECTURE.mddocs/ROADMAP.mdgo.modinternal/detect/android.gointernal/detect/detect.gointernal/detect/detect_test.gointernal/detect/flutter.gointernal/detect/ios.gointernal/detect/kotlin.gointernal/detect/reactnative.gointernal/detect/swift.gointernal/driver/android.gointernal/driver/driver.gointernal/driver/driver_test.gointernal/driver/flutter.gointernal/driver/ios.gointernal/driver/kotlin.gointernal/driver/reactnative.gointernal/driver/registry.gointernal/driver/swift.gointernal/pipeline/pipeline.gointernal/pipeline/pipeline_test.gointernal/sign/sign.gointernal/sign/sign_test.gointernal/tui/plain.gointernal/tui/plain_test.gointernal/tui/tui.gotasks/lessons.mdtasks/todo.md
🛑 Comments failed to post (9)
CLAUDE.md (1)
80-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the repository map with the implemented CLI.
cmd/now includes build and sign commands, and signing lives underinternal/sign, but this section only documents root, version, and detect. Add the implemented command and package locations so contributors do not miss the current lifecycle and signing code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` around lines 80 - 85, Update the “Where things are” section in CLAUDE.md to document the implemented build and sign commands under cmd/ and the signing implementation under internal/sign, while preserving the existing root, version, detect, driver, pipeline, documentation, and task references.cmd/build.go (1)
103-105: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Handle the reported writer errors.
errcheckreports both ignoredfmt.Fprintlnresults, which blocks linting.
cmd/build.go#L103-L105: propagate an output-write error fromchooseProject.internal/pipeline/pipeline_test.go#L21-L23: assert the write result or explicitly discard it.🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 103-103: Error return value of
fmt.Fprintlnis not checked(errcheck)
📍 Affects 2 files
cmd/build.go#L103-L105(this comment)internal/pipeline/pipeline_test.go#L21-L23🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/build.go` around lines 103 - 105, The output writes in chooseProject must handle errors: propagate failures from both fmt.Fprintln/Fprintf calls through the function’s existing error path. In internal/pipeline/pipeline_test.go lines 21-23, explicitly assert the write result or discard it to satisfy errcheck.Source: Linters/SAST tools
cmd/sign.go (2)
63-72: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Android dry-run prints a false "Signing configured" confirmation.
setupAndroidSigning's dry-run branch (lines 112-123) always returnsnilforextra, same as its real-work branch (line 146). Back here,len(extra) == 0is checked beforesignDryRun, so for Android the flow always falls into the first branch and prints "Signing configured. Run 'anvil build --release'..." even during--dry-run, right after the "Would set up Android signing:" plan was printed. No changes were actually made, but the message tells the user otherwise.🐛 Proposed fix: check dry-run before the empty-extra branch
- if len(extra) == 0 { - fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") - return nil - } if signDryRun { + if len(extra) == 0 { + return nil + } printPlan(cmd, chosen, d, opts, extra) return nil } + if len(extra) == 0 { + fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") + return nil + } return runPipeline(cmd, chosen.Path, d, opts, extra)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if signDryRun { if len(extra) == 0 { return nil } printPlan(cmd, chosen, d, opts, extra) return nil } if len(extra) == 0 { fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") return nil } return runPipeline(cmd, chosen.Path, d, opts, extra) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/sign.go` around lines 63 - 72, Update the control flow in the signing command around the len(extra) check so signDryRun is handled before the empty-extra confirmation branch. Ensure Android dry-runs call printPlan and return without printing “Signing configured,” while preserving the existing confirmation for non-dry-run cases with no extra actions.
164-173: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Empty Team ID from the interactive prompt is not validated.
Unlike
resolveSecret(lines 196-198), which errors on an empty value, this block accepts whatever the huh form returns without checking for emptiness. If the user submits the form blank,WriteExportOptionssilently writes anExportOptions.plistwith an emptyteamID.🐛 Proposed fix
if err := huh.NewForm(huh.NewGroup( huh.NewInput().Title("Apple Developer Team ID").Value(&teamID), )).Run(); err != nil { return driver.Signing{}, nil, err } + if teamID == "" { + return driver.Signing{}, nil, errors.New("Apple Developer Team ID is required") + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if teamID == "" { if !interactive() { return driver.Signing{}, nil, errors.New("set --team-id or ANVIL_TEAM_ID") } if err := huh.NewForm(huh.NewGroup( huh.NewInput().Title("Apple Developer Team ID").Value(&teamID), )).Run(); err != nil { return driver.Signing{}, nil, err } if teamID == "" { return driver.Signing{}, nil, errors.New("Apple Developer Team ID is required") } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/sign.go` around lines 164 - 173, Validate teamID after the interactive huh form completes in the team-ID resolution block, matching resolveSecret’s empty-value behavior. If the prompt returns an empty value, return an error instructing the user to set --team-id or ANVIL_TEAM_ID before proceeding to WriteExportOptions.docs/ARCHITECTURE.md (2)
8-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the diagram fence language.
Markdownlint reports MD040 because this fenced block has no language identifier. Use
textfor the architecture diagram.🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ARCHITECTURE.md` around lines 8 - 16, Update the architecture diagram fenced block in ARCHITECTURE.md to specify the text language identifier, preserving the diagram content unchanged.Source: Linters/SAST tools
52-52: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the documented Driver contract with
internal/driver.Driver.The supplied implementation defines
Name,Steps,Classify, andArtifacts, but noCapabilitiesmethod. The new pipeline description therefore documents behavior that the actual registry and drivers cannot provide. Update this sketch and the phase-selection text to matchinternal/driver.Driver, or change the implementation and tests to introduce the documented contract.Also applies to: 63-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ARCHITECTURE.md` at line 52, Align the Driver contract sketch and related phase-selection text in ARCHITECTURE.md with internal/driver.Driver by removing or revising the unsupported Capabilities requirement and documenting only the implemented methods: Name, Steps, Classify, and Artifacts. Keep the documentation consistent with the registry and existing drivers rather than changing implementation or tests.internal/detect/detect.go (1)
67-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate filesystem traversal errors.
Line 68 discards permission and I/O errors, so
detectcan successfully report a partial result or no stack. Returnerrbefore inspectingd.Proposed fix
walk := func(path string, d fs.DirEntry, err error) error { - if err != nil || !d.IsDir() { + if err != nil { + return err + } + if !d.IsDir() { return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.walk := func(path string, d fs.DirEntry, err error) error { if err != nil { return err } if !d.IsDir() { return nil }🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 69-69: error is not nil (line 67) but it returns nil
(nilerr)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/detect/detect.go` around lines 67 - 69, Update the walk callback in detect to return any non-nil filesystem traversal err immediately before accessing d.IsDir(), while preserving the existing behavior of skipping non-directory entries. This ensures permission and I/O errors propagate instead of being silently ignored.Source: Linters/SAST tools
internal/tui/plain.go (1)
15-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate output write failures.
A broken writer is silently treated as a successful build. Track errors from every write and return
success && outputOK, otherwiseKindDonecan mask failed CI or pipe output.Proposed fix
func Plain(events <-chan pipeline.Event, out io.Writer) bool { var items []pipeline.Item success := true + outputOK := true + write := func(format string, args ...interface{}) { + if _, err := fmt.Fprintf(out, format, args...); err != nil { + outputOK = false + } + } for e := range events { switch e.Kind { case pipeline.KindPlan: items = e.Items case pipeline.KindStepStart: - fmt.Fprintf(out, "==> %s: %s\n", items[e.Index].Phase, items[e.Index].Name) + write("==> %s: %s\n", items[e.Index].Phase, items[e.Index].Name) case pipeline.KindLine: - fmt.Fprintf(out, " %s\n", e.Line) + write(" %s\n", e.Line) case pipeline.KindStepDone: switch e.Status { case driver.Skipped: - fmt.Fprintf(out, "--- %s: skipped\n", items[e.Index].Phase) + write("--- %s: skipped\n", items[e.Index].Phase) case driver.OK: - fmt.Fprintln(out, " ok") + write(" ok\n") for _, a := range e.Artifacts { - fmt.Fprintf(out, " artifact: %s\n", a) + write(" artifact: %s\n", a) } case driver.Failed: - fmt.Fprintf(out, " failed (exit %d)\n", e.ExitCode) + write(" failed (exit %d)\n", e.ExitCode) success = false } case pipeline.KindDone: success = e.Success } } - return success + return success && outputOK }The CLI consumes this boolean as its build result in
cmd/build.goLine 122.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func Plain(events <-chan pipeline.Event, out io.Writer) bool { var items []pipeline.Item success := true outputOK := true write := func(format string, args ...interface{}) { if _, err := fmt.Fprintf(out, format, args...); err != nil { outputOK = false } } for e := range events { switch e.Kind { case pipeline.KindPlan: items = e.Items case pipeline.KindStepStart: write("==> %s: %s\n", items[e.Index].Phase, items[e.Index].Name) case pipeline.KindLine: write(" %s\n", e.Line) case pipeline.KindStepDone: switch e.Status { case driver.Skipped: write("--- %s: skipped\n", items[e.Index].Phase) case driver.OK: write(" ok\n") for _, a := range e.Artifacts { write(" artifact: %s\n", a) } case driver.Failed: write(" failed (exit %d)\n", e.ExitCode) success = false } case pipeline.KindDone: success = e.Success } } return success && outputOK }🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 23-23: Error return value of
fmt.Fprintfis not checked(errcheck)
[error] 25-25: Error return value of
fmt.Fprintfis not checked(errcheck)
[error] 29-29: Error return value of
fmt.Fprintfis not checked(errcheck)
[error] 31-31: Error return value of
fmt.Fprintlnis not checked(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/plain.go` around lines 15 - 43, Update Plain to track an outputOK flag and capture errors from every fmt.Fprint/Fprintf call to out, marking outputOK false whenever a write fails. Return success && outputOK so pipeline.KindDone cannot mask output failures, while preserving the existing build success handling.Source: Linters/SAST tools
README.md (1)
7-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align README capabilities and status with the implemented milestones.
The README says the command signs and uploads, but upload remains planned. It also says build comes next and signing is later, although both are implemented. Finally, the v1 stack list omits Swift and Kotlin. Update these statements, or clearly mark the planned upload and future capabilities.
Also applies to: 20-22, 46-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 7 - 8, Update the README capability descriptions and milestone/status sections to match the implementation: describe build and signing as available, remove or clearly label upload as planned, and add Swift and Kotlin to the v1 supported stack list. Ensure all related statements remain consistent across the command overview and referenced milestone sections.
1 participant
What does this PR do?
Type of change
Checklist
CHANGELOG.md(if the repo has one)developbranchSummary by CodeRabbit
New Features
anvil detect, including table and JSON output with configurable scan depth.anvil buildwith target, flavor, release, signing, dry-run, and plain-output options.Documentation