feat(zfa): add setup command + make init wire dependencies (#275) - #277
Conversation
Add `zfa setup <name>` — bootstraps a new Flutter/Dart app with the standard zuraffa dependency set wired in (zuraffa[_flutter], zorphy_annotation, dev:build_runner/mocktail/flutter_lints, analyzer override), creates build.yaml with zorphy builder registration, writes default .zfa.json, and scaffolds the domain directory structure. Make `zfa init`/`zfa initialize` wire dependencies by default (per issue #275) so the "only zfa commands" contract is viable on a fresh project. Add `--deps-only` and `--no-deps` flags. Register `init` as an alias. Introduce `DependencyWirer` (lib/src/core/dependencies/dependency_wirer.dart) as the shared dependency-wiring logic: standardSet, findMissing, addOverrideToPubspec, wire, isFlutterProject, ensureProjectStructure. Uses `dart pub add` for regular/dev deps (preserves pubspec formatting) and direct YAML editing for dependency_overrides. Closes #275
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI adds ChangesProject bootstrap and initialization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
lib/src/core/dependencies/dependency_wirer.dart (4)
63-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
skippedis never populated, sodidNothingis always false.
wire()only fillsaddedandfailed.findMissingremoves already-present dependencies before wiring, so no code path adds toskipped.didNothingtherefore always returns false and can mislead callers. Populateskippedinwire()with the already-present dependency names, or remove both members.🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 63 - 85, Update wire() to populate WireResult.skipped with dependency names that are already present, using the pre-wiring dependency state before findMissing removes them, so didNothing accurately reports runs with no additions. Preserve the existing added and failed results; alternatively remove skipped and didNothing together if skipped cannot be populated.
100-102: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDefault git ref points to a moving branch.
defaultGitRef = 'development'makes every bootstrapped project track a mutable branch. Builds are not reproducible, and a broken commit ondevelopmentbreaks new projects immediately. Consider a tagged release ref as the default, and expose the ref as an option for users who want the development branch.🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 100 - 102, Update the defaultGitRef constant to use a stable tagged release rather than the mutable development branch, and add an option in the dependency-wiring configuration or API for callers to explicitly select development or another ref. Preserve the existing ref usage while allowing users to override the stable default.
317-328: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the pubspec write and record success after it completes.
added.add(spec.name)runs beforewriteAsString. If the write throws (read-only file, permission error), the exception escapeswire(). NeitherSetupCommand.runnorInitializeCommand.executecatches it, so the user sees a raw stack trace. The pub-add loop above already handles failures. Apply the same handling here and move theaddedentries after a successful write.🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 317 - 328, Update the override-writing flow in wire() to guard pubspecFile.writeAsString with the same failure handling used by the pub-add loop, preventing write errors from escaping as raw exceptions. Move added.add(spec.name) so override names are recorded only after the write succeeds, while preserving the existing override processing and success output.
249-250: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
package:pathfor filesystem joins.
'$root/pubspec.yaml','$root/build.yaml', and'$root/$dir'hardcode the POSIX separator. The repository already depends onpackage:pathand usespath.joininlib/src/commands/initialize_command.dart. Usep.joinhere so paths stay consistent and portable.Also applies to: 414-437
🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 249 - 250, Replace the string-interpolated filesystem paths in the dependency-wiring logic, including the `pubspecFile`, `build.yaml`, and `$root/$dir` constructions, with `p.join` calls from `package:path`. Add or reuse the existing path alias import, preserving the current root and directory values while making joins platform-independent.lib/src/commands/initialize_command.dart (1)
72-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
exit(1)insideexecutebypasses the CLI error path.
CliRunnersupportsexitOnCompletion: falseandrunCapturing, which run commands in a zone and collect output. A directexit(1)terminates the process, so neither mechanism can observe these two validation failures, and tests cannot cover them.CliRunner.runalready convertsUsageExceptioninto an exit code of 64 and prints the usage. Throw instead of callingexit.Also applies to: 84-90
🤖 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 `@lib/src/commands/initialize_command.dart` around lines 72 - 75, Replace the direct exit(1) calls in execute for both mutually exclusive option validation branches with UsageException throws, preserving the existing error messages so CliRunner can handle the failures and produce the expected usage and exit code.test/commands/setup_command_test.dart (1)
68-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
InitializeCommandtests validate a duplicated parser, not the real one.
_buildInitializeParsercopies theArgParserthatInitializeCommand.executebuilds internally. The two definitions can drift. If a flag is renamed or removed inlib/src/commands/initialize_command.dart, every test here still passes. The group therefore gives no protection for the new--deps-onlyand--no-depsflags.Expose the parser from the production class and use it in the test.
♻️ Proposed refactor
In
lib/src/commands/initialize_command.dart:class InitializeCommand { static ArgParser buildParser() => ArgParser() ..addOption('entity', abbr: 'e', defaultsTo: 'Product', help: '...') // ... remaining options, moved out of execute() ..addFlag('help', abbr: 'h', help: 'Show help', negatable: false); Future<void> execute(List<String> args) async { final parser = buildParser(); // ... } }In this test file:
-/// Builds the same ArgParser that InitializeCommand.execute() uses. -ArgParser _buildInitializeParser() { - return ArgParser() - ..addOption('entity', abbr: 'e', defaultsTo: 'Product') - ..addOption('output', abbr: 'o', defaultsTo: 'lib/src/domain/entities') - ..addFlag('force', abbr: 'f', negatable: false) - ..addFlag('dry-run', negatable: false) - ..addFlag('deps-only', negatable: false) - ..addFlag('no-deps', negatable: false) - ..addFlag('verbose', abbr: 'v', negatable: false) - ..addFlag('help', abbr: 'h', negatable: false); -} +ArgParser _buildInitializeParser() => InitializeCommand.buildParser();Also applies to: 128-139
🤖 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 `@test/commands/setup_command_test.dart` around lines 68 - 106, Expose the production parser through a reusable `InitializeCommand.buildParser()` method by moving the `ArgParser` construction out of `execute()` while preserving all existing options and flags. Update the tests in the `InitializeCommand` group to call `InitializeCommand.buildParser()` and remove the duplicated `_buildInitializeParser` definition, so they validate the parser used by `execute()`.lib/src/commands/setup_command.dart (1)
232-233: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
--platformsand--orgare ignored silently in the Dart branch.The Dart path builds
['create', '-t', 'package', appName]and drops both options.dart createhas no equivalent for either. Print a warning when the user passes--platformsor--orgtogether with--dart. Also extend the--orghelp text with "(ignored with --dart)", which the--platformshelp already states.🤖 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 `@lib/src/commands/setup_command.dart` around lines 232 - 233, Update the Dart branch in setup command argument construction to warn when --platforms or --org were provided with --dart, while keeping those unsupported options out of the dart create arguments. Extend the --org option help text with “(ignored with --dart)” to match the existing --platforms guidance.test/core/dependencies/dependency_wirer_test.dart (1)
500-520: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for
ensureProjectStructure.The suite asserts the contents of
standardDirsandbuildYamlContentbut never runsensureProjectStructure. That method acceptsprojectRoot, so a temp-directory test can verify thatbuild.yamland each directory are created, that existing files are not overwritten, and thatdryRun: truewrites nothing. That covers the only file-creating path reachable without a network.🤖 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 `@test/core/dependencies/dependency_wirer_test.dart` around lines 500 - 520, Add tests for DependencyWirer.ensureProjectStructure using a temporary projectRoot: verify build.yaml and every standard directory are created, existing files remain unchanged, and dryRun: true creates nothing. Keep the tests isolated with temporary-directory setup and cleanup, covering the file-creation path without network access.
🤖 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 `@lib/src/cli/cli_runner.dart`:
- Around line 221-235: Update the CORE COMMANDS help text for the initialize
entry to state that init is an alias of initialize, matching
_InitializeCommand’s name and aliases declarations; leave the command behavior
unchanged.
In `@lib/src/commands/setup_command.dart`:
- Around line 183-195: Update _createApp’s --force deletion flow to resolve and
print the target’s absolute path and file count before deleting it, and require
interactive confirmation when stdin is a terminal; abort deletion if
confirmation is declined. Preserve dry-run behavior without deleting, and revise
the --force help text to explicitly state that the target directory is deleted.
- Around line 123-129: Update DependencyWirer.wire handling in
lib/src/commands/setup_command.dart#L123-L129 to retain the WireResult, include
its failed entries in the step 5 summary, and set exitCode to 1 when isSuccess
is false. In lib/src/commands/initialize_command.dart#L103-L109, after the
existing warning block, set a non-zero exit code or return before entity
scaffolding when the WireResult is unsuccessful.
In `@lib/src/core/dependencies/dependency_wirer.dart`:
- Around line 203-217: Update the append branch for overrideIdx == -1 to
construct the new dependency_overrides section with the header at column zero
from the outset. Remove the replaceFirst-based whitespace correction and
preserve the existing newline and blank-line separation behavior.
- Around line 293-300: Update the dependency-wiring process around the
pubAddSpecs loop and its pub get command to select the executable based on
isFlutter: use flutter for Flutter projects and dart otherwise, while preserving
the existing pub add/get arguments and error handling.
In `@test/commands/setup_command_test.dart`:
- Around line 117-124: Rename the test around SetupCommand.argParser to state
that the parser accepts --flutter and --dart together, matching its assertions
and inline comment. Add a separate CommandRunner test covering the
mutual-exclusion branch in SetupCommand.run, asserting that supplying both flags
throws UsageException.
---
Nitpick comments:
In `@lib/src/commands/initialize_command.dart`:
- Around line 72-75: Replace the direct exit(1) calls in execute for both
mutually exclusive option validation branches with UsageException throws,
preserving the existing error messages so CliRunner can handle the failures and
produce the expected usage and exit code.
In `@lib/src/commands/setup_command.dart`:
- Around line 232-233: Update the Dart branch in setup command argument
construction to warn when --platforms or --org were provided with --dart, while
keeping those unsupported options out of the dart create arguments. Extend the
--org option help text with “(ignored with --dart)” to match the existing
--platforms guidance.
In `@lib/src/core/dependencies/dependency_wirer.dart`:
- Around line 63-85: Update wire() to populate WireResult.skipped with
dependency names that are already present, using the pre-wiring dependency state
before findMissing removes them, so didNothing accurately reports runs with no
additions. Preserve the existing added and failed results; alternatively remove
skipped and didNothing together if skipped cannot be populated.
- Around line 100-102: Update the defaultGitRef constant to use a stable tagged
release rather than the mutable development branch, and add an option in the
dependency-wiring configuration or API for callers to explicitly select
development or another ref. Preserve the existing ref usage while allowing users
to override the stable default.
- Around line 317-328: Update the override-writing flow in wire() to guard
pubspecFile.writeAsString with the same failure handling used by the pub-add
loop, preventing write errors from escaping as raw exceptions. Move
added.add(spec.name) so override names are recorded only after the write
succeeds, while preserving the existing override processing and success output.
- Around line 249-250: Replace the string-interpolated filesystem paths in the
dependency-wiring logic, including the `pubspecFile`, `build.yaml`, and
`$root/$dir` constructions, with `p.join` calls from `package:path`. Add or
reuse the existing path alias import, preserving the current root and directory
values while making joins platform-independent.
In `@test/commands/setup_command_test.dart`:
- Around line 68-106: Expose the production parser through a reusable
`InitializeCommand.buildParser()` method by moving the `ArgParser` construction
out of `execute()` while preserving all existing options and flags. Update the
tests in the `InitializeCommand` group to call `InitializeCommand.buildParser()`
and remove the duplicated `_buildInitializeParser` definition, so they validate
the parser used by `execute()`.
In `@test/core/dependencies/dependency_wirer_test.dart`:
- Around line 500-520: Add tests for DependencyWirer.ensureProjectStructure
using a temporary projectRoot: verify build.yaml and every standard directory
are created, existing files remain unchanged, and dryRun: true creates nothing.
Keep the tests isolated with temporary-directory setup and cleanup, covering the
file-creation path without network access.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dfcba71c-c9ff-4ffa-845d-64f4dbdc5b19
📒 Files selected for processing (6)
lib/src/cli/cli_runner.dartlib/src/commands/initialize_command.dartlib/src/commands/setup_command.dartlib/src/core/dependencies/dependency_wirer.darttest/commands/setup_command_test.darttest/core/dependencies/dependency_wirer_test.dart
| BOOTSTRAP: | ||
| setup <name> Create a new Flutter/Dart app with zuraffa deps wired in | ||
| init Wire zuraffa dependencies + scaffold a test entity | ||
|
|
||
| CORE COMMANDS: | ||
| make <Name> Canonical architecture/code generation command | ||
| feature <Name> Wrapper over `make --preset=feature` | ||
| initialize Initialize a test entity | ||
| initialize Alias of init — wire deps + scaffold a test entity | ||
| entity Create and manage Zorphy entities | ||
| config Manage ZFA configuration | ||
| doctor Check your environment and v5 migration readiness | ||
| schema Output JSON schema | ||
| validate <file> Validate JSON configuration | ||
| migrate <target> Migrate v5 artifacts to v6 (state, gql, di) | ||
| build Run build_runner to generate code from annotations |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The alias description is inverted.
_InitializeCommand declares name => 'initialize' and aliases => ['init']. The help text at line 228 states that initialize is an alias of init, which reverses the relationship. zfa help and zfa initialize --help will disagree. Describe init as the alias of initialize.
📝 Proposed fix
BOOTSTRAP:
setup <name> Create a new Flutter/Dart app with zuraffa deps wired in
- init Wire zuraffa dependencies + scaffold a test entity
+ init Alias of initialize — wire deps + scaffold a test entity- initialize Alias of init — wire deps + scaffold a test entity
+ initialize Wire zuraffa dependencies + scaffold a test entity📝 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.
| BOOTSTRAP: | |
| setup <name> Create a new Flutter/Dart app with zuraffa deps wired in | |
| init Wire zuraffa dependencies + scaffold a test entity | |
| CORE COMMANDS: | |
| make <Name> Canonical architecture/code generation command | |
| feature <Name> Wrapper over `make --preset=feature` | |
| initialize Initialize a test entity | |
| initialize Alias of init — wire deps + scaffold a test entity | |
| entity Create and manage Zorphy entities | |
| config Manage ZFA configuration | |
| doctor Check your environment and v5 migration readiness | |
| schema Output JSON schema | |
| validate <file> Validate JSON configuration | |
| migrate <target> Migrate v5 artifacts to v6 (state, gql, di) | |
| build Run build_runner to generate code from annotations | |
| BOOTSTRAP: | |
| setup <name> Create a new Flutter/Dart app with zuraffa deps wired in | |
| init Alias of initialize — wire deps + scaffold a test entity | |
| CORE COMMANDS: | |
| make <Name> Canonical architecture/code generation command | |
| feature <Name> Wrapper over `make --preset=feature` | |
| initialize Wire zuraffa dependencies + scaffold a test entity | |
| entity Create and manage Zorphy entities | |
| config Manage ZFA configuration | |
| doctor Check your environment and v5 migration readiness | |
| schema Output JSON schema | |
| validate <file> Validate JSON configuration | |
| migrate <target> Migrate v5 artifacts to v6 (state, gql, di) | |
| build Run build_runner to generate code from annotations |
🤖 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 `@lib/src/cli/cli_runner.dart` around lines 221 - 235, Update the CORE COMMANDS
help text for the initialize entry to state that init is an alias of initialize,
matching _InitializeCommand’s name and aliases declarations; leave the command
behavior unchanged.
| } else { | ||
| await DependencyWirer.wire( | ||
| isFlutter: isFlutter, | ||
| dryRun: false, | ||
| projectRoot: appName, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Dependency-wiring failures do not reach the process exit code. Both commands call DependencyWirer.wire, and neither converts a failed WireResult into a non-zero exit code. wire() prints warnings for each package it could not add, then both commands print a success message and exit 0. A CI job cannot distinguish a complete bootstrap from one with no dependencies wired.
lib/src/commands/setup_command.dart#L123-L129: assign the return value ofDependencyWirer.wireto a variable, listfailedentries in the step 5 summary, and setexitCode = 1whenisSuccessis false.lib/src/commands/initialize_command.dart#L103-L109: after the existing warning block, set a non-zero exit code, or return before entity scaffolding whenisSuccessis false.
📍 Affects 2 files
lib/src/commands/setup_command.dart#L123-L129(this comment)lib/src/commands/initialize_command.dart#L103-L109
🤖 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 `@lib/src/commands/setup_command.dart` around lines 123 - 129, Update
DependencyWirer.wire handling in lib/src/commands/setup_command.dart#L123-L129
to retain the WireResult, include its failed entries in the step 5 summary, and
set exitCode to 1 when isSuccess is false. In
lib/src/commands/initialize_command.dart#L103-L109, after the existing warning
block, set a non-zero exit code or return before entity scaffolding when the
WireResult is unsuccessful.
| if (targetDir.existsSync()) { | ||
| if (!force) { | ||
| print('❌ Directory already exists: $appName'); | ||
| print(' Use --force to overwrite, or pick a different name.'); | ||
| return false; | ||
| } | ||
| if (!dryRun) { | ||
| await targetDir.delete(recursive: true); | ||
| print(' Removed existing $appName (--force)'); | ||
| } else { | ||
| print(' Would remove existing $appName (--force)'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
--force deletes an existing directory without confirmation.
_createApp calls targetDir.delete(recursive: true) as soon as --force is set. The help text says "Overwrite the target directory", which does not tell the user that the whole tree is removed first. A typo in the app name that matches an existing project causes permanent data loss. The name validation prevents traversal, so the risk is confined to the working directory, but the deletion is still unrecoverable.
Print the resolved absolute path and the file count before deletion, and require an interactive confirmation when stdin is a terminal. Update the --force help text to state that the directory is deleted.
🤖 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 `@lib/src/commands/setup_command.dart` around lines 183 - 195, Update
_createApp’s --force deletion flow to resolve and print the target’s absolute
path and file count before deleting it, and require interactive confirmation
when stdin is a terminal; abort deletion if confirmation is declined. Preserve
dry-run behavior without deleting, and revise the --force help text to
explicitly state that the target directory is deleted.
| if (overrideIdx == -1) { | ||
| // Append a new dependency_overrides section. | ||
| var result = content; | ||
| if (!result.endsWith('\n')) { | ||
| result = '$result\n'; | ||
| } | ||
| // Ensure a blank line separates the new section from the previous one. | ||
| if (!result.endsWith('\n\n')) { | ||
| result = '$result\n'; | ||
| } | ||
| return '$result dependency_overrides:\n $key: $value\n'.replaceFirst( | ||
| ' dependency_overrides', | ||
| 'dependency_overrides', | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
replaceFirst can corrupt the appended section.
The append branch builds the section with a leading space and then removes it with replaceFirst(' dependency_overrides', 'dependency_overrides'). replaceFirst scans from index 0, so it strips the first occurrence anywhere in the file. If the existing pubspec contains the substring dependency_overrides earlier (for example in a comment such as # see dependency_overrides above), that text is mutated and the appended header keeps its leading space. An indented dependency_overrides: line is not a top-level key, so the override is silently ignored or the file fails to parse.
Build the string without the leading space instead.
🐛 Proposed fix
- return '$result dependency_overrides:\n $key: $value\n'.replaceFirst(
- ' dependency_overrides',
- 'dependency_overrides',
- );
+ return '${result}dependency_overrides:\n $key: $value\n';📝 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 (overrideIdx == -1) { | |
| // Append a new dependency_overrides section. | |
| var result = content; | |
| if (!result.endsWith('\n')) { | |
| result = '$result\n'; | |
| } | |
| // Ensure a blank line separates the new section from the previous one. | |
| if (!result.endsWith('\n\n')) { | |
| result = '$result\n'; | |
| } | |
| return '$result dependency_overrides:\n $key: $value\n'.replaceFirst( | |
| ' dependency_overrides', | |
| 'dependency_overrides', | |
| ); | |
| } | |
| if (overrideIdx == -1) { | |
| // Append a new dependency_overrides section. | |
| var result = content; | |
| if (!result.endsWith('\n')) { | |
| result = '$result\n'; | |
| } | |
| // Ensure a blank line separates the new section from the previous one. | |
| if (!result.endsWith('\n\n')) { | |
| result = '$result\n'; | |
| } | |
| return '${result}dependency_overrides:\n $key: $value\n'; | |
| } |
🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 203 - 217,
Update the append branch for overrideIdx == -1 to construct the new
dependency_overrides section with the header at column zero from the outset.
Remove the replaceFirst-based whitespace correction and preserve the existing
newline and blank-line separation behavior.
| for (final spec in pubAddSpecs) { | ||
| final args = _buildPubAddArgs(spec); | ||
| try { | ||
| final result = await Process.run( | ||
| 'dart', | ||
| ['pub', 'add', ...args], | ||
| workingDirectory: root, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does dart pub get work in a Flutter project with flutter sdk dependency using the standalone Dart SDK?
💡 Result:
No, running dart pub get in a Flutter project using a standalone Dart SDK generally does not work if the project has a dependency on the Flutter SDK [1][2]. When your project includes a dependency on the Flutter SDK (e.g., in pubspec.yaml under dependencies: flutter: sdk: flutter), the pub resolver requires access to the Flutter SDK to satisfy that dependency [3][2]. A standalone Dart SDK does not contain the Flutter SDK, causing dependency resolution to fail with an error stating that the Flutter SDK is not available [2]. While it is technically possible for dart pub get to succeed if you manually set the FLUTTER_ROOT environment variable to point to a valid Flutter SDK installation on your machine, this is not the supported or recommended approach [4][5]. Using the standalone tool may also bypass critical Flutter-specific processes, such as code generation hooks for plugins, which are handled by the flutter command [5]. For Flutter projects, you should always use flutter pub get [1][6]. If you are working in a workspace that contains both Dart and Flutter projects, ensure you run the appropriate command for the specific project directory [1][7].
Citations:
- 1: https://dart.dev/tools/pub/cmd
- 2: https://stackoverflow.com/questions/59089521/flutter-users-should-run-flutter-pub-get-instead-of-pub-get
- 3: https://dart.dev/tools/pub/dependencies
- 4: "Flutter users should use flutter pub get" when fetching packages for a Dart project in a Pub Workspace that also uses Flutter Dart-Code/Dart-Code#5973
- 5: make the logic to locate the Flutter SDK more robust dart-lang/pub#2307
- 6: https://docs.flutter.dev/packages-and-plugins/using-packages
- 7: support hiding subdirectories from all pub commands dart-lang/pub#4083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)dependency_wirer\.dart$|pubspec\.yaml$' || true
echo
echo "dependency_wirer outline:"
ast-grep outline lib/src/core/dependencies/dependency_wirer.dart --view expanded || true
echo
echo "Relevant dependency_wirer sections:"
sed -n '230,355p' lib/src/core/dependencies/dependency_wirer.dart
echo
echo "Search for isFlutter definitions:"
rg -n "isFlutter|flutter: sdk: flutter|process\\.name|flutter pub|dart pub|Process\\.run" lib/src/core/dependencies/dependency_wirer.dartRepository: arrrrny/zuraffa
Length of output: 5874
Use the Flutter pub executable for Flutter projects.
When isFlutter is true, pub add and pub get run through the standalone dart executable. Flutter projects declare flutter: sdk: flutter, and that dependency requires the Flutter SDK resolver, so these commands can fail and leave wiring in failed. Run flutter pub add and flutter pub get when isFlutter is true.
🐛 Proposed fix
+ final pubExecutable = isFlutter ? 'flutter' : 'dart';
for (final spec in pubAddSpecs) {
final args = _buildPubAddArgs(spec);
try {
final result = await Process.run(
- 'dart',
+ pubExecutable,
['pub', 'add', ...args],
workingDirectory: root,
); final getResult = await Process.run(
- 'dart',
+ pubExecutable,
['pub', 'get'],
workingDirectory: root,
);🤖 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 `@lib/src/core/dependencies/dependency_wirer.dart` around lines 293 - 300,
Update the dependency-wiring process around the pubAddSpecs loop and its pub get
command to select the executable based on isFlutter: use flutter for Flutter
projects and dart otherwise, while preserving the existing pub add/get arguments
and error handling.
|
@coderabbitai autofix |
|
An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
@coderabbitai autofix |
|
An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Apply the still-applicable CodeRabbit findings from the merged #277 (zfa setup + init wiring) review: - cli_runner: fix inverted alias help text (init is the alias of initialize) - setup/initialize: propagate failed dependency wiring to a non-zero exit code (retain WireResult, list failed entries, stop before entity scaffolding) - setup: require interactive confirmation before --force deletes the target directory (print absolute path + entry count), revise --force/--org help text - setup: warn when --platforms/--org are passed with --dart - dependency_wirer: build the appended dependency_overrides section at column zero instead of replaceFirst (avoid corrupting earlier text) - dependency_wirer: use 'flutter' pub executable for Flutter projects - dependency_wirer: populate WireResult.skipped so didNothing is accurate; guard the pubspec write and record overrides as added only after a successful write; use package:path joins - initialize: throw UsageException instead of exit(1) for mutually-exclusive flags and missing pubspec - tests: use InitializeCommand.buildParser() (drop duplicated parser), add UsageException coverage for --flutter/--dart exclusion, add ensureProjectStructure tests Skipped: defaultGitRef -> stable tag (no released tag contains the zuraffa_flutter sub-package; pinning would break Flutter wiring).
Summary
Implements issue #275: adds a
zfa setupcommand for bootstrapping new Flutter/Dart apps and makeszfa initwire the standard zuraffa dependency set automatically.Changes
New:
zfa setup <name>commandflutter create --empty(with--platformsand--orgpassthrough) ordart create -t package(--dart)zuraffa_flutter(Flutter) orzuraffa(Dart) — from git (development ref)zorphy_annotation— from gitbuild_runner,mocktail,flutter_lints(Flutter only)dependency_overrides:analyzer: 14.1.0build.yamlwith zorphy + json_serializable builder registration.zfa.json--dry-run,--force,--verboseModified:
zfa init/zfa initialize--deps-onlyflag (wire deps without scaffolding entity)--no-depsflag (legacy behavior — entity only)initas an alias forinitialize(was mentioned in help text but not actually registered)build.yamland domain directory structure exist.zfa.jsonexistsNew:
DependencyWirerclass (lib/src/core/dependencies/dependency_wirer.dart)Shared dependency-wiring logic used by both
setupandinit:standardSet({isFlutter})— returns the canonical zuraffa dependency listfindMissing(pubspecContent, {isFlutter})— pure function: detects missing deps via YAML parsingaddOverrideToPubspec(content, key, value)— pure function: inserts/appends dependency_overrides entrieswire({isFlutter, dryRun, projectRoot})— I/O: runsdart pub addfor regular/dev deps, edits pubspec.yaml for overridesisFlutterProject(pubspecContent)— detects Flutter SDK dependencyensureProjectStructure({projectRoot, dryRun})— creates build.yaml + domain dirsUses
dart pub add(preserves pubspec formatting, runs pub get atomically) for regular/dev deps. Uses direct YAML editing fordependency_overrides(whichdart pub adddoesn't support).Tests
test/core/dependencies/dependency_wirer_test.dart— 44 tests covering standardSet, findMissing, isFlutterProject, addOverrideToPubspec, DependencySpec equality/toString, WireResult, buildYamlContent, standardDirstest/commands/setup_command_test.dart— 13 tests covering SetupCommand flags, InitializeCommand flags, CLI registrationVerification
dart analyze— no issues on new/modified filesdart test— 57/57 new tests passzfa setup test_app --dry-run,zfa setup test_pkg --dart --dry-run,zfa init --deps-only --dry-runall produce correct outputUsage
Notes
cli_edge_cases_test.dartfailures (when run alongsidexray_deck_cli_test.dart) are caused by CWD contamination inxray_deck_cli_testand are not introduced by this PR — they fail identically on the clean development branch.Closes #275
Summary by CodeRabbit
New Features
setupcommand to bootstrap Flutter or Dart projects.initializewith dependency-only and dependency-skip options.Documentation
Tests