Skip to content

feat: migrate install onto fluttersdk_artisan + layout fixes#71

Open
anilcancakir wants to merge 11 commits into
mainfrom
feat/cli-migration-to-fluttersdk-artisan
Open

feat: migrate install onto fluttersdk_artisan + layout fixes#71
anilcancakir wants to merge 11 commits into
mainfrom
feat/cli-migration-to-fluttersdk-artisan

Conversation

@anilcancakir

Copy link
Copy Markdown
Contributor

Syncs 0 local commit(s) to main.

Commits

- bin/magic_starter.dart: DELETED (hard cut per locked decision)
- lib/src/cli/cli.dart: DELETED (dead re-export barrel)
- lib/src/cli/starter_artisan_provider.dart: ServiceProvider returning
  5 commands (starter:install/publish/configure/doctor/uninstall)
- lib/src/cli/commands/*.dart: migrated 5 commands to extend
  ArtisanCommand with CommandBoot.none + namespaced names; thread
  ArtisanContext through private helpers; inlined _promptConfirm
  (artisan has no interactive prompt API yet)
- pubspec: drop magic_cli, add fluttersdk_artisan + path override,
  remove 'executables: magic_starter', bump 0.0.1-alpha.14 -> .15
- lib/magic_starter.dart: barrel export
InstallCommand was renamed to InstallArtisanCommand in fluttersdk_artisan
to free the unprefixed name for plugin packages. magic_starter's five
CLI commands had no compile-time dependency on the old class name, but
their imports and command-list ordering were touched by the parallel
fluttersdk_artisan submodule bump. No behavioral change in
magic_starter:install / configure / publish / uninstall / doctor.
Re-pin to the magic release carrying fluttersdk_wind 1.0.0. Verified locally
via a temporary path override (dart analyze clean, tests green); a clean
pub get resolves once magic 1.0.0-alpha.14 is published to pub.dev.
Lets developers wire sibling fluttersdk packages via a local pubspec_overrides.yaml
without committing it (committing path overrides would break CI, which has no sibling paths).
… mcpTool

Pin fluttersdk_artisan ^0.0.7 and drop committed dependency_overrides. Rewrite install onto ArtisanInstallCommand + install.yaml (hybrid: feature-gated transactional writes staged before injectProvider per the no-rollback ordering contract; stateful helper injects run post-commit). Add read-only starter_doctor mcpTool. Purge magic_cli/runWith from the doctor/configure/publish/uninstall command tests (migrated to the ArtisanContext.bare harness), drop an unnecessary import, update docs/rules/CHANGELOG.
Add Flutter-free lib/cli.dart + rename StarterArtisanProvider to MagicStarterArtisanProvider so fluttersdk_artisan plugin discovery generates a compilable _plugins.g.dart in the consumer app.
…point

The notifications feature shelled out to 'dart run magic_notifications install', but post artisan-migration magic_notifications has no standalone bin entrypoint (and notifications:install requires a user-specific OneSignal --app-id). Replace the broken auto-run + its test seam with a clear instruction to run 'dart run <app>:artisan notifications:install --app-id=<id>' via the host artisan.
…nject

_injectTranslationAssetIntoPubspec used contains('assets:') to detect an existing assets list, which matched the commented '# assets:' example that 'flutter create' ships. With no real assets key present it then fell through to appending a SECOND top-level 'flutter:' section -> duplicate key -> invalid YAML that breaks all flutter tooling (analyze, run, build) in the consumer app. Detect a real (uncommented) assets: key via regex instead; add a regression test using the real flutter-create pubspec shape.
…r cascade

MagicStarterAppLayout wrapped its whole Scaffold in a LayoutBuilder purely to
read the responsive breakpoint via wScreenIs. wScreenIs reads MediaQuery.size,
so the LayoutBuilder was unnecessary, and it made itself the build-scope root:
a setState from the auth/refresh listeners during a route transition (or a
viewport resize crossing the lg breakpoint) rebuilt dirty widgets in the wrong
scope, producing a cascade of non-fatal framework exceptions (LayoutBuilder
slot == null, 'dirty widget in the wrong build scope', GlobalKey
_elements.contains, RenderFlex overflow) that broke the body render until an
app restart. Worst under rapid navigation and at narrow widths.

Compute isDesktop directly in build() via wScreenIs(context, 'lg'); MediaQuery.of
registers a normal dependency so width changes still rebuild the shell, without
the layout-phase build boundary. Behavior is identical; the fragility is gone.

Verified: app_layout tests green, analyze clean, and a fresh uptizm dogfood that
previously logged 11-15 exceptions under aggressive Settings<->Dashboard
navigation now logs 0.
Two further fixes on top of the LayoutBuilder removal, both root-causing the
render-lifecycle exception cascade (_elements.contains / markNeedsLayout on an
already-dirty relayout boundary / double detach) that surfaced the red
ErrorWidget when navigating between settings sub-views under accumulated
navigation, on mobile and desktop:

1. Key the route content by path: KeyedSubtree(key: ValueKey(currentPath)).
   The persistent ShellRoute kept ONE scroll container (scrollPrimary) and
   reused its child slot across different route views; swapping one scrollable
   view for another then tore down render objects in a confused order. A
   per-route key forces a clean unmount-then-mount. This is the root fix.
2. Drop the _scaffoldKey GlobalKey<ScaffoldState>; open the drawer via
   Builder + Scaffold.of(context) instead. A GlobalKey on a shell Scaffold
   that rebuilds every navigation is a reparenting hazard.

These are debug-mode asserts (stripped in release) but they break the dev
experience via the red ErrorWidget. Verified: app_layout tests green, analyze
clean, and the exact uptizm repro (Monitors->StatusPages->Profile->AI settings
->Appearance->Notifications, plus 3 rounds of aggressive nav and a full
breakpoint resize stress) that logged 12-15 exceptions now logs 0 at both
desktop (1440) and mobile (390) widths.
Copilot AI review requested due to automatic review settings June 16, 2026 15:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates Magic Starter’s CLI surface from the standalone magic_cli binary to the host app’s fluttersdk_artisan dispatcher, introduces a manifest-backed install flow (install.yaml), and includes a stability fix in MagicStarterAppLayout to avoid layout-scope rebuild issues during navigation.

Changes:

  • Replace magic_cli commands/entrypoint with ArtisanCommand/ArtisanInstallCommand + a new MagicStarterArtisanProvider (plus a read-only MCP tool descriptor).
  • Make install partially manifest-driven (install.yaml) while keeping feature-gated logic in a fluent override; update tests accordingly.
  • Adjust MagicStarterAppLayout responsiveness and route-body mounting to prevent rebuild/layout exceptions.

Reviewed changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test/ui/widgets/magic_starter_social_divider_test.dart Removes direct src import; relies on public export surface.
test/cli/starter_artisan_provider_test.dart Adds tests for the new artisan provider and MCP tool curation.
test/cli/commands/magic_starter_uninstall_command_test.dart Updates uninstall tests to drive handle(ctx) via ArtisanContext.
test/cli/commands/magic_starter_publish_command_test.dart Updates publish tests to use ArtisanContext and new command names.
test/cli/commands/magic_starter_install_command_test.dart Updates install tests for artisan context, manifest parsing, and op ordering.
test/cli/commands/magic_starter_doctor_command_test.dart Updates doctor tests for artisan migration + report marker changes.
test/cli/commands/magic_starter_configure_command_test.dart Updates configure tests to use ArtisanContext flag maps.
README.md Migrates documentation to dart run <app>:artisan starter:* usage.
pubspec.yaml Bumps version and replaces magic_cli with fluttersdk_artisan.
lib/src/ui/layouts/magic_starter_app_layout.dart Reworks responsive build and keys route content for safer remounting.
lib/src/cli/starter_artisan_provider.dart Adds MagicStarterArtisanProvider and MCP tool descriptor.
lib/src/cli/commands/magic_starter_uninstall_command.dart Migrates uninstall command to ArtisanCommand + handle(ctx).
lib/src/cli/commands/magic_starter_publish_command.dart Migrates publish command to ArtisanCommand + signature DSL.
lib/src/cli/commands/magic_starter_install_command.dart Migrates install to ArtisanInstallCommand, adds manifest resolution and staged ops.
lib/src/cli/commands/magic_starter_doctor_command.dart Migrates doctor command to ArtisanCommand, updates output markers and exit codes.
lib/src/cli/commands/magic_starter_configure_command.dart Migrates configure command to ArtisanCommand, updates flag parsing and output.
lib/src/cli/cli.dart Removes old magic_cli-based CLI barrel.
lib/magic_starter.dart Exports updated public surface (now includes CLI provider export).
lib/cli.dart Adds a dedicated pure-Dart CLI barrel exporting the artisan provider.
install.yaml Adds manifest for static install scaffolding (provider injection).
CLAUDE.md Updates contributor docs for artisan-based CLI usage and architecture.
CHANGELOG.md Documents breaking CLI migration, manifest install, and MCP tool addition.
bin/magic_starter.dart Removes the standalone CLI entrypoint.
.gitignore Ignores pubspec_overrides.yaml to prevent committing local path wiring.
.claude/rules/cli.md Updates internal CLI rules to reflect the artisan migration.

Comment on lines +187 to 188
await installer.commit(dryRun: isDryRun(ctx), force: force);

Comment on lines +646 to +648
FileHelper.writeFile(targetPath, rendered);
ctx.output
.warning('Overwritten: lib/app/providers/app_service_provider.dart');
Comment thread lib/magic_starter.dart
Comment on lines 1 to 6
// Magic Starter plugin exports

export 'src/cli/starter_artisan_provider.dart';
export 'src/facades/magic_starter.dart';
export 'src/magic_starter_manager.dart';
export 'src/providers/magic_starter_service_provider.dart';
Comment thread README.md

This generates `lib/config/magic_starter.dart`, injects `MagicStarterServiceProvider` into `lib/config/app.dart`, and wires the `magicStarterConfig` factory into `lib/main.dart`.

(If `magic_starter` is not yet registered as a provider in your host app, register `StarterArtisanProvider` in your app's `artisan.providers` list first.)
Comment thread CHANGELOG.md
## [Unreleased]

### Removed
- **Breaking: Standalone CLI entrypoint** — removed `bin/magic_starter.dart` and `dart run magic_starter:*` commands. Commands now surface via the host app's artisan dispatcher. Migrate: `dart run magic_starter:install` becomes `dart run <app>:artisan starter:install` (register `StarterArtisanProvider` in your app's `artisan.providers` list).
Comment thread CHANGELOG.md
- **Install is manifest-driven** — static scaffolding (config publish, provider injection) now driven by `install.yaml` manifest; dynamic logic (feature toggles, interactive mode) handled by fluent override in `MagicStarterInstallCommand`.

### Added
- **Read-only MCP tool** — `starter_doctor` diagnostic command exposed as a read-only MCP tool via `StarterArtisanProvider.mcpTools()`.
Comment thread CLAUDE.md
assets/stubs/ # Stub templates for code generation
```

**CLI Surface**: Commands surface via the host app's artisan registry, not a standalone bin/. The host app registers `StarterArtisanProvider` in its `artisan.providers`, exposing 5 commands and 1 read-only MCP tool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants