Make the plugin references true again, and stamp them for the releases going out - #152
Merged
Merged
Conversation
These files are the only agent-facing guide to the plugins: all three packages exclude CLAUDE.md and .claude/ from publication, so an agent adopting one from pub.dev lands here and nowhere else. All three were stale against shipped code and the deeplink one handed out an example that does not compile. reference stamp shipped magic_deeplink v0.0.2 0.1.0 magic_notifications v0.0.3 0.2.0 magic_starter alpha.23 alpha.27 The deeplink reference was the worst of the three. It documented the old single-argument handle(Uri), navigated with a bare Route.to (Flutter's own Route class, not the MagicRoute facade) passing an 'extra' parameter that does not exist, described the removed Future.delayed initial-link read, and carried no platform setup at all, which is the one thing an adopter cannot work out alone: without the entitlement, the intent filter and the two switches that disable Flutter's own deep linking, the plugin compiles and never fires. It also told the reader to ensure provider order in app.dart. That is false: boot runs after every provider has registered (application.dart:353), which is exactly why the deeplink push bridge works whichever side of notifications it sits on. The gotcha is replaced by the fact. SKILL.md gains one ordered install recipe so no agent has to reconstruct it: pub add, plugin:install, <plugin>:install, <plugin>:doctor, then whatever the manifest cannot do.
Both references asserted the opposite of what the code did. The deeplink one said provider order "does not matter" for the push bridge, and the notifications one said a listener attached before any driver exists "still receives them". The first is what produces the defect and the second is the reasoning error behind it: the manager's click stream does exist from construction, but it is a BROADCAST stream, and a broadcast stream drops what it publishes to nobody. That window is real on a cold start, where the tap that launched the app is drained inside `driver.initialize()` before a late-booting bridge has subscribed. `magic_notifications` now buffers those clicks, so either order works; the deeplink row says what to do on a build that does not, and names the installer behaviour that decides which order a consumer gets in the first place. Measured on a physical iPhone: the same push opened the right screen warm and the home screen cold.
Each of these is a place the reference sends an adopter somewhere wrong, and all four fail silently in the app rather than loudly at build time. Web was described as "a deliberate no-op" in the summary line, which reads as "no deep links on web" and stops people. The DRIVER is inert; the feature is not. The push bridge is wired outside the isSupported gate, so a tapped web push routes exactly as it does on mobile, and the address-bar half is GoRouter's, conditional on a path url strategy and a host rewrite that this package neither owns nor can check. handleUri and getInitialLink do not wait for a frame. Both in-package callers do, so a consumer driving the chain by hand from boot inherits nothing and loses the link into a router that is not built yet. That is how the push path itself failed until this week. The iOS App Group and Notification Service Extension were absent from the notifications reference entirely, as they were from the package. Push works without them, so the missing confirmed delivery, rich media and badge counts read as the product rather than as an install left half done. Also the Debug cold-start caveat, which cost a whole diagnostic round here: iOS will not launch a debug Flutter build from a link, and OneSignal documents that a force-closed Debug app never registers the click listener.
magic_notifications is going out as 0.3.0 rather than the 0.2.0 this stamp was written against: the release carries the cold-start click buffer plus the bulk preferences row, and that row needs two translation keys an adopter has to add, which is a soft break rather than a patch. The package asserts this from its own suite (skill_reference_stamp_test.dart reads its pubspec version and this file's stamp), so leaving it at 0.2.0 keeps that suite red and blocks the publish.
This was referenced Sep 9, 2026
Merged
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Docs-only change, and a substantial accuracy win over what was there - the corrected Major
Minor
TestsNothing in Checks I ran
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… release" This reverts commit b6550fc.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Since my last review the only change is one commit, Major
Minorstamp vs. the described blocking order — the PR states TestsUnchanged: this repo carries no test over these stamps; the assertions live in the plugin repos and read this checkout from outside. Checks I ran
|
…0 release" This reverts commit 29ca7e8.
Three findings from the kodizm review, each confirmed against the source before acting. **A table rendered as literal text.** The `handleUri` prose and its `endOfFrame` snippet were inserted between two halves of the DeeplinkManager API table, so `onLink`, `driver` and `reset()` sat after a paragraph with no header or delimiter above them. GFM does not start a table there; it swallows them as a lazy paragraph continuation, and the three rows an agent needs most render as pipe-delimited text. Prose and snippet moved below the complete table. **"Provider ORDER does not matter for plugin wiring" was too broad, and this skill is where that sentence does the most damage.** It is true of BINDINGS, because every register() runs before any boot(). It is false of everything a provider DOES in boot(), because `Application.boot` is a sequential await over the list (application.dart:378-381). The sentence contradicted this same file's own opening, which tells the reader AppServiceProvider must precede AuthServiceProvider so setUserFactory lands before auth restore. It also contradicted the defect this whole reference pass came out of: magic_notifications published a cold-start push tap from its boot() and magic_deeplink subscribed in its own, so with notifications first the tap went into a broadcast stream with no listener and the app opened on its initial route. Silent both ways, and decided by install order, because artisan's installer appends each provider to the end of the list. The section now scopes the claim to bindings and lists the three order-sensitive shapes, including that one. **The SKILL.md version line did not move** though the file gained a section and three references were rewritten. CLAUDE.md's post-change sync item 4 asks for it, and since the mirror to fluttersdk/ai rides the release rather than the push, the stamp is how a consumer tells a stale copy from a fresh one. 0.1.13 -> 0.1.14. Also reverts the stamp revert. Dropping plugin-notifications.md to v0.2.0 left the file describing the click buffer while claiming to document a version that is already on pub.dev WITHOUT it, which is the exact drift this PR exists to close; the reviewer caught it independently. The stamp tracks the package's own declared version, which is 0.3.0 on its branch.
anilcancakir
added a commit
to fluttersdk/magic_notifications
that referenced
this pull request
Sep 9, 2026
…tter read
Two holes in the cold-start buffer this branch added, both found by review and
both confirmed against the source before acting. The buffer asked "has anybody
read this stream", and the question that matters is "is anybody subscribed".
**Reading a stream and listening to it are different moments, and only the
second can receive anything.** `onPushClicked` set `_pushClickedHeard` on the
getter and scheduled the drain there, so a consumer that captures the stream,
awaits its router and subscribes afterwards flipped the flag, had the microtask
drain into a broadcast controller with no subscriber, and lost the launch tap in
the exact way this buffer exists to prevent.
**`forgetDrivers()` could not close the second hole.** It reset the flag but
cannot cancel a subscription somebody else holds, and it is public through
`Notify.forgetDrivers()`. An app calling it on sign-out kept its boot-time
subscription, so the manager buffered again for a listener that was already
there and never drained, because nothing reads the getter twice. Every tap after
a sign-out was swallowed in silence.
Both close on the same change: the buffer fills when
`_pushClickedController.hasListener` is false and drains from that controller's
own `onListen`, which fires on each 0 -> 1 transition. The drain refuses an
empty controller, so a subscriber that cancels inside the turn it subscribed
leaves the buffer for the next one instead of having it added to nobody. The
window therefore reopens on any later gap between subscribers rather than
closing for the life of the process, which is the honest shape: a tap that
arrives while nothing is listening is a tap nobody received.
The replay is also re-judged rather than replayed blind. A tap buffered while
the device was subscribed as one person and drained after `want()` moved it to
another handed the deep-link bridge the previous identity's payload, which is
precisely what the receive-side guard exists to stop; `forgetDrivers` clears the
buffer, `want` does not.
Three tests, each verified to fail against the previous implementation and pass
against this one: read-then-subscribe-later, a live subscriber surviving
`forgetDrivers`, and a buffered tap dropped after the identity moves.
Also from the same review:
- `notifications:doctor` identified a Notification Service Extension by a
`.appex` suffix, which every app extension has. A project shipping a widget
and no NSE reported it present and only nagged about App Groups: a false
green on the one check whose justification is that its subject looks exactly
like the product working. It now requires the `.appex` AND an `Info.plist`
under `ios/` declaring `com.apple.usernotifications.service`, since the
extension point lives in the target's own plist and the target can be named
anything. Runner's own plist is excluded. Covered by a test that fails
against the old check.
- `skill_reference_stamp_test.dart` called `.first` on the reference's lines,
which throws `Bad state: No element` on an empty file and replaces the
carefully worded reason with a StateError naming nothing.
Verified: `flutter analyze` clean, `dart format` clean, 623 tests pass. The one
failure is the stamp gate, which reads the sibling magic checkout and is red
until fluttersdk/magic#152 lands; it skips in CI.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Since Major
Resolved since last review
The two Minors from earlier (the missing TestsUnchanged: no test in this repo covers these stamps, and no Dart file changed in this range. Checks I ran
|
anilcancakir
added a commit
to fluttersdk/magic_deeplink
that referenced
this pull request
Sep 9, 2026
…ling correct manifests
Three defects from the kodizm review, all in the new `deeplink:doctor`, all
confirmed before acting and all covered by a test verified to fail against the
previous code.
**The doctor said "All checks passed!" and "Needs attention" in the same run.**
`getWarnings()` read `associations['warnings']` alone, so `checkWiring`'s finding
never reached `handle()` or the summary. A project with perfect platform files,
a resolving domain and no `registerHandler(` anywhere under `lib/` printed
`Dart Wiring: Needs attention` in the report body, `All checks passed!`
underneath it, and exited 0; without `--verbose` the warning text appeared
nowhere at all. The one finding that means "no link will ever reach a handler"
was the one the summary dropped. Warnings are now collected from every section
and prefixed by section name, so a section that learns to warn is not silently
lost the way this one was.
**Two false failures on correct manifests, from regexes standing in for the
element tree this command claims to parse.**
- `_metaDataValue` matched only `<meta-data ... />`. Android accepts
`<meta-data ...></meta-data>` identically, and a manifest using it was told
its meta-data was missing.
- `directChildren` was the application body minus `<activity>` blocks only, so
a `<meta-data>` inside a `<service>`, `<provider>` or `<receiver>` counted
as a direct child of `<application>`. With the correct value on the activity
and a same-named one inside a provider, the project was failed and the early
return meant the activity was never even looked at.
Every container that can sit under `<application>` is now stripped, and
`activity-alias` leads the alternation deliberately: `<activity\b` matches
`<activity-alias` too, because a word boundary sits between the `y` and the `-`,
so the old pattern hunted for a `</activity>` belonging to some later element
and swallowed everything between. An alias carries intent filters, so its body
now counts as an activity's rather than being discarded, and an autoVerify
filter declared on one is found.
This file's own rule is that failing a correct project is worse than
under-reporting, and it was breaking it three ways.
Also from the review, without a code change:
- `DeeplinkServiceProvider._deliver` now documents the gap between "the end of
a frame" and "the router exists". magic builds the router at
`foundation/magic.dart:112`, AFTER the `await boot()` above it, so through
every provider's boot `MagicRouter._router` is null. If a provider
registered later yields the event loop in its own boot and the scheduler
serves the frame in that window, delivery resumes against a null router and
`MagicRoute.to` throws. Not closed here because settling it needs a device
rather than an argument, and the path was measured working on a physical
iPhone; the try/catch makes it a logged error rather than a silent loss. The
docblock names the symptom to look for.
- `.claude/rules/flutter.md` still described the old `handle(Uri)` signature
and prescribed `Future.delayed(Duration.zero, ...)`, the two patterns this
release removes, and `tests.md` still prescribed the `forgetHandlers()` /
`forgetDriver()` pair rather than `reset()`. Those files load whenever an
agent touches `lib/**/*.dart`, so they were teaching the shape this release
exists to replace.
- `CLAUDE.md`'s command table and file tree omitted `deeplink:doctor`.
- `handle`'s required `DeeplinkSource` moved to a `Breaking Changes` heading.
It breaks every hand-written handler, and `install.yaml`'s own post-install
message says handlers are written by hand; `^0.0.3` consumers do not
auto-upgrade and will meet it as a compile error.
Not addressed: `handle()` runs the full check set five times over
(`getMissingRequirements`, `getWarnings`, then `generateReport` re-running all
three), which re-reads every platform file and re-walks `lib/` each pass.
Harmless on a small app, and worth a memo pass rather than a rushed one.
Verified: `flutter analyze` clean, `dart format` clean, 159 tests pass. The one
failure is the stamp gate, red until fluttersdk/magic#152 lands and skipped in CI.
…leased Second review pass. The table fix was half done: the three rows were moved above the prose but a blank line still sat between them and the row before, so they were a standalone block with no header or delimiter and GFM still rendered them as literal pipe text. One line deleted; a scan of the whole file for a row whose predecessor is neither a row nor a delimiter now finds none. The remaining caveat pointed the wrong way rather than being missing. `plugin-deeplink.md:352` and SKILL.md both said `deeplink:doctor` was on the default branch and not in 0.1.0, which was true when written and stopped being true when that release folded the command in. A reference stamped v0.1.0 telling its reader the command is not in v0.1.0 is worse than silence. `plugin-starter.md:5` naming `magic_notifications ^0.2.0` is now correct and needs no edit: the pin bump that would have made it wrong came back out of magic_starter#126.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Since Resolved since last review
Still open, non-blocking
TestsUnchanged: no test in this repo covers these stamps or reference bodies; the stamp assertions live in the plugin repos and read this checkout from outside. Checks I ran
|
Contributor
Author
|
On the one still-open item, Verified in the checkout: The distinction the train now follows, since it caused two red jobs today before it was drawn: a package may declare its OWN next version on its branch, but never raise a caret on a sibling ahead of that sibling's release. |
anilcancakir
added a commit
to fluttersdk/magic_deeplink
that referenced
this pull request
Sep 10, 2026
…0.1.0 (#17) * feat(deeplink): select the driver arm at compile time and emit the modern AASA Wave 1 of the deep-link plan. The driver was one file importing dart:io unconditionally and answering isSupported false behind a kIsWeb branch. It is now a conditional-export trio: an io arm backed by app_links, an explicit no-op web arm, and a stub default. The web arm is inert on purpose, because app_links_web reads the boot-time location.href once and never reacts to navigation while GoRouter already owns the address bar. The AASA generator now emits Apple's modern appIDs plus components shape with no apps key, because TN3155 warns that mixing the two schemas may produce unexpected behaviour. Its config parser also tolerates a Dart generic annotation before a list literal, which is what uptizm writes and what silently produced no assetlinks.json at all. * feat(deeplink): carry provenance to the handler, and make boot honour its own config Wave 2 of the deep-link plan. Four defects that all live in the same seam. The handler contract took only a Uri, and the OneSignal bridge threw away every payload key but the link, so a handler could not tell a crafted OS link from a server-authored push and could not read the keys a consumer acts on. DeeplinkSource is now a required argument on handle() and on handleUri(), and the bridge passes the whole payload alongside it. Required rather than defaulted, because a handler that forgot to ask would treat the two alike. boot() gated on the driver NAME alone: nothing read deeplink.enabled, which the documentation has always presented as the off switch, and nothing read driver.isSupported, so a web build still constructed a driver for a mechanism the browser does not have. Both are read now, and an explicit false wires nothing at all. A cold-start link arrived twice. The provider subscribed to the driver stream AND called getInitialLink(), which the Android plugin serves independently, so one tap ran the whole handler chain twice. The stream is the surviving path, matching app_links' own documented usage. Delivery waited on a zero-duration timer while MagicRoute.to throws until the router is built, and called handleUri unawaited with no catch, so a slow boot lost the link to an unhandled async error. It now awaits the end of the first frame, captured once, and reports a failure instead of escaping. DeeplinkManager gains a test-reachable reset(), because its cached initial link and its never-closed broadcast controller both outlived forgetDriver() and leaked one test's state into the next. * docs(deeplink): describe the package as it now behaves Wave 3 of the deep-link plan. Three documented claims the code has never supported. install.yaml told an adopter to call Deeplink.register, and there is no Deeplink facade, so the post-install message handed out code that does not compile. The README said the route handler maps paths automatically, and the provider has never registered one. The configuration guide presented deeplink.enabled as the off switch, which only became true in this release. The installation guide also never carried the platform prerequisites at all, so an adopter who followed it exactly got nothing and had no way to see why. It now carries the associated-domains entitlement, the autoVerify intent filter with data elements for both http and https, and the two switches that keep Flutter's own deep linking out of the way. The iOS plist key is applied through install.yaml; the Android one is documented rather than automated, because the artisan installer inserts meta-data into application and Flutter reads that key from activity, so automating it would ship a silent no-op. Both architecture pages and CLAUDE.md described the old single-argument handler contract and the removed initial-link read. doc/basics/cli.md still documented a bin entrypoint removed in 0.0.1. * chore(release): 0.1.0 A minor rather than a patch: the DeeplinkHandler contract gained a required DeeplinkSource argument and the AASA generator changed schema, so a consumer implementing a handler has to change with it. * refactor(drivers): drop error handling for a case the platform split removed The io arm's try/catch around Platform.isAndroid existed so the single-file driver could survive a web build. This file now compiles only where dart.library.io resolves, so Platform is real and those getters cannot throw. * feat(cli): add deeplink:doctor, so an adopter can tell the install worked The manifest installer can publish a config file and inject a provider. It cannot put an <intent-filter> inside a specific <activity>, and artisan's XmlEditor writes <meta-data> into <application> while Flutter reads flutter_deeplinking_enabled from <activity>. So the half that decides whether a deep link works at all is hand-written, and every way of getting it wrong is silent: the link opens the browser, with no exception and no log. deeplink:doctor checks that half. Config parses through the SAME parser the generator uses, so the two cannot disagree; no value is still a scaffold placeholder; the entitlement's applinks host equals deeplink.domain; both Flutter deep-linking switches are off, with the Android one read off the element tree rather than grepped, because a meta-data on the wrong element greps identically and is inert; the intent filter carries both http and https; and the generated association files agree with the config. --remote adds the two HTTPS fetches. Every check has a broken fixture proving it fails. It also says what it cannot prove. No local check shows a device matching a link: swcutil needs root and Android verifies at install time. Also corrects doc/basics/cli.md, which still printed the legacy appID plus paths AASA the generator stopped emitting, and exports the command from the top-level CLI barrel beside its two siblings. * docs(handlers): resolve the manager under the key the provider actually binds The example asked the container for 'deeplink'; the provider binds 'deeplinks' (deeplink_service_provider.dart:56), and the other three docs say so. An adopter copying this line got a container miss. * chore(release): gate the agent-facing reference against this package's version The reference an agent adopting this package reads lives in the magic repo, not here: .pubignore keeps CLAUDE.md and .claude/ out of the published archive, so pub.dev ships doc/ and README.md and nothing else an agent is pointed at. That reference is versioned by magic's releases rather than by this package's, and it drifted far enough to document a contract that no longer compiles. A test compares its first-line stamp against this pubspec's version. It cannot run in CI, which clones no siblings, so it skips there instead of failing; releases are cut locally and that is where it fires. release.md carries the same requirement in prose for anyone running the suite elsewhere. * fix(deeplink): hold a tapped push until the first frame Measured on a physical iPhone against a real server-sent notification: the same push opened the right screen when the app was already running and landed on the home screen when it was not. The OneSignal SDK replays the tap that launched the app while it initialises, which is before anything has been drawn and therefore before magic's router can accept a navigation. The link was handed over, went nowhere, and the app finished booting onto its own initial route. Nothing failed loudly, so a cold tap read as the feature working badly rather than as a defect. The OS-link path in DeeplinkServiceProvider has waited for `endOfFrame` since 0.1.0; the push path never has, and the two have been asymmetric since the push bridge started working in 0.0.3. The handler now takes that same future once per `setup` and awaits it before routing, and drops a delivery still in flight behind the frame when `dispose` lands. The two routing tests that already existed had to move to `testWidgets`, because a plain `test` pumps no frame and `endOfFrame` never completes there. Two new tests pin the behaviour: one asserts nothing routes until the frame, one asserts a pending delivery is dropped after dispose. Both go red when the await is removed. * feat(doctor): check that the Dart half of the install landed Every section this command had checks a platform file, and all of them pass on a project where the package is a dependency and nothing more: the entitlement, the intent filter and both association files correct, the domain resolving, and no link opening the app because no provider was ever registered. That is what a half-applied install leaves behind, and what a human gets by adding the dependency by hand, so a green report that cannot see it is the most misleading answer the command can give. Three checks, in the report's existing shape. The provider in lib/config/app.dart and the config factory in lib/main.dart are hard failures, because either alone makes the feature inert. A missing registerHandler is a WARNING: a consumer may register handlers somewhere this scan cannot recognise, and failing a correct project is worse than under-reporting on an unusual one. Line comments are stripped before every search, for the same reason the Android checks strip XML ones: a commented-out registration reads identically to a live one, and that is exactly what a half-finished install leaves. The "fully configured project" fixture had no Dart wiring at all, which is why five existing tests went red; it now writes the provider, the config factory and a handler, each independently omittable so a test can leave out exactly one. * docs(deeplink): say what web actually does, and point the install at the doctor Three corrections, each one a place an adopter stops or goes wrong. The platform table said web deep links are unsupported, full stop. They are not: the push bridge is wired outside the driver's isSupported gate, so a tapped OneSignal web push with the tab open reaches the same handler chain as on mobile. What web has no DRIVER for is the address bar, and that half is GoRouter's, conditional on two things this package never mentioned: the path url strategy, and a host rewrite to index.html. Neither fails loudly, and a push clicked with no tab open arrives through that second route rather than the first. `deeplink:doctor` appeared only in the CLI reference, so the document an adopter follows to install never told them how to check the install. It now closes with the doctor, the --remote half, and what no local check can prove. Both install snippets still pinned ^0.0.3 against a 0.1.0 pubspec. * chore(release): fold the cold-start fix and the doctor into 0.1.0 0.1.0 was cut earlier today and never published (pub.dev still serves 0.0.3), so the two entries that landed after it belong in that release rather than in a second one: a version nobody can depend on yet is not a version worth splitting. The changelog now carries one 0.1.0 heading with Fixed, Added and Changed, in the order the rest of this file already uses. CLAUDE.md's version line was still 0.0.3, two releases behind. Not published. The publish is the owner's to run, and it has to follow magic_notifications 0.3.0 rather than precede it: uptizm pins both, and the starter between them pins notifications too. Note the local suite is red on skill_reference_stamp_test until the sibling magic checkout sits on a branch carrying the v0.1.0 stamp; the reference itself is already stamped on feature/plugin-skill-references, and the test skips in CI, which clones no siblings. * fix(doctor): stop dropping the one warning that matters, and stop failing correct manifests Three defects from the kodizm review, all in the new `deeplink:doctor`, all confirmed before acting and all covered by a test verified to fail against the previous code. **The doctor said "All checks passed!" and "Needs attention" in the same run.** `getWarnings()` read `associations['warnings']` alone, so `checkWiring`'s finding never reached `handle()` or the summary. A project with perfect platform files, a resolving domain and no `registerHandler(` anywhere under `lib/` printed `Dart Wiring: Needs attention` in the report body, `All checks passed!` underneath it, and exited 0; without `--verbose` the warning text appeared nowhere at all. The one finding that means "no link will ever reach a handler" was the one the summary dropped. Warnings are now collected from every section and prefixed by section name, so a section that learns to warn is not silently lost the way this one was. **Two false failures on correct manifests, from regexes standing in for the element tree this command claims to parse.** - `_metaDataValue` matched only `<meta-data ... />`. Android accepts `<meta-data ...></meta-data>` identically, and a manifest using it was told its meta-data was missing. - `directChildren` was the application body minus `<activity>` blocks only, so a `<meta-data>` inside a `<service>`, `<provider>` or `<receiver>` counted as a direct child of `<application>`. With the correct value on the activity and a same-named one inside a provider, the project was failed and the early return meant the activity was never even looked at. Every container that can sit under `<application>` is now stripped, and `activity-alias` leads the alternation deliberately: `<activity\b` matches `<activity-alias` too, because a word boundary sits between the `y` and the `-`, so the old pattern hunted for a `</activity>` belonging to some later element and swallowed everything between. An alias carries intent filters, so its body now counts as an activity's rather than being discarded, and an autoVerify filter declared on one is found. This file's own rule is that failing a correct project is worse than under-reporting, and it was breaking it three ways. Also from the review, without a code change: - `DeeplinkServiceProvider._deliver` now documents the gap between "the end of a frame" and "the router exists". magic builds the router at `foundation/magic.dart:112`, AFTER the `await boot()` above it, so through every provider's boot `MagicRouter._router` is null. If a provider registered later yields the event loop in its own boot and the scheduler serves the frame in that window, delivery resumes against a null router and `MagicRoute.to` throws. Not closed here because settling it needs a device rather than an argument, and the path was measured working on a physical iPhone; the try/catch makes it a logged error rather than a silent loss. The docblock names the symptom to look for. - `.claude/rules/flutter.md` still described the old `handle(Uri)` signature and prescribed `Future.delayed(Duration.zero, ...)`, the two patterns this release removes, and `tests.md` still prescribed the `forgetHandlers()` / `forgetDriver()` pair rather than `reset()`. Those files load whenever an agent touches `lib/**/*.dart`, so they were teaching the shape this release exists to replace. - `CLAUDE.md`'s command table and file tree omitted `deeplink:doctor`. - `handle`'s required `DeeplinkSource` moved to a `Breaking Changes` heading. It breaks every hand-written handler, and `install.yaml`'s own post-install message says handlers are written by hand; `^0.0.3` consumers do not auto-upgrade and will meet it as a compile error. Not addressed: `handle()` runs the full check set five times over (`getMissingRequirements`, `getWarnings`, then `generateReport` re-running all three), which re-reads every platform file and re-walks `lib/` each pass. Harmless on a small app, and worth a memo pass rather than a rushed one. Verified: `flutter analyze` clean, `dart format` clean, 159 tests pass. The one failure is the stamp gate, red until fluttersdk/magic#152 lands and skipped in CI.
anilcancakir
added a commit
to fluttersdk/magic_notifications
that referenced
this pull request
Sep 10, 2026
The extension-point check I added last commit was right about WHAT identifies a Notification Service Extension and wrong about where to look for it. Reviewed, reproduced, and confirmed by hand before fixing. Walking all of `ios/` recursively and reading each `Info.plist` as UTF-8 crashes on any real project. `ios/Pods` is full of vendored frameworks whose plist is a BINARY plist; `readAsStringSync` throws a FileSystemException on one, nothing here caught it, and `getWarnings()` is called unguarded from `handle()`, so `notifications:doctor` and the `notifications_doctor` MCP tool both blew up instead of printing a warning. It only bit a project whose pbxproj already names a `.appex`, which is exactly this check's audience, and OneSignal's own iOS SDK arrives as an XCFramework through CocoaPods. Verified against a real binary plist written with plistlib: `readAsStringSync` throws, `utf8.decode(bytes, allowMalformed: true)` does not. The same walk followed links, so it descended `ios/.symlinks/plugins/*` into the pub cache and all of Pods. A dependency shipping an NSE template plist then read as THIS app's extension, which is the false green the extension-point check was added to remove. It also paid a full Pods walk on every run. Both close together: the walk starts at the immediate children of `ios/`, skips the directories that are never an app target (`Runner`, `Pods`, `.symlinks`, `build`, `Flutter`), never follows a link, and decodes tolerantly. Excluding Runner by NAME rather than by a `'/Runner/'` substring also fixes the third finding: that test did nothing on Windows, where the separator is a backslash. Two tests, both verified to fail against the previous commit: a binary plist under `ios/Pods` (asserting `returnsNormally` as well as the warning, since the regression was a crash) and a dependency's NSE template that must not count. Verified: analyze clean, format clean, 625 pass. The one failure is the stamp gate against the sibling magic checkout, which sits on another branch here; fluttersdk/magic#152 is merged, so master carries v0.3.0.
anilcancakir
added a commit
to fluttersdk/magic_notifications
that referenced
this pull request
Sep 10, 2026
* chore(release): gate the agent-facing reference against this package's version The reference an agent adopting this package reads lives in the magic repo, not here: .pubignore keeps CLAUDE.md and .claude/ out of the published archive, so pub.dev ships doc/ and README.md and nothing else an agent is pointed at. That reference is versioned by magic's releases rather than by this package's, and it drifted far enough to document a contract that no longer compiles. A test compares its first-line stamp against this pubspec's version. It cannot run in CI, which clones no siblings, so it skips there instead of failing; releases are cut locally and that is where it fires. release.md carries the same requirement in prose for anyone running the suite elsewhere. * fix(notifications): hold a cold-start click until something listens `onPushClicked` is a broadcast stream, and a broadcast stream drops what it publishes to nobody. On a cold start there is a window where nobody is there yet: `onesignal_flutter` buffers the tap that launched the app and drains it in a microtask scheduled from `addClickListener`, which this manager calls inside `driver.initialize()`, which its own provider awaits in `boot()`. A consumer whose provider list puts notifications BEFORE the package that bridges clicks into deep links subscribes only afterwards, so the launch tap is published into an empty stream. No exception, no log, and the app finishes booting onto its own initial route. Which order a consumer ends up with is decided by the order the two packages happened to be installed in, because artisan's installer appends each provider to the END of the list. uptizm has the safe order by accident, which is why a device measurement passed while the defect was live. The manager now holds clicks until the first listener and replays them once, mirroring `onesignal_flutter`'s own shape: filled only until the first-ever listener, drained once, never refilled, and bounded so a build that never listens does not grow a list for the life of the process. Refilling for a late subscriber would re-navigate an app somebody has since moved through, which is worse than the failure this closes. `forgetDrivers` clears both fields so a buffered click cannot replay into the next test. * style(tests): take the formatter's wrapping on the stamp test reason * docs(changelog): record the cold-start click buffer * feat(doctor): report the iOS pieces no pub package can install OneSignal's iOS setup needs an App Group and a Notification Service Extension, and this package never mentioned either: no check, no doc line, nothing in the install manifest. Push works without them, which is the reason it matters. A build with no extension delivers notifications normally and quietly reports no confirmed deliveries, no rich media and no badge counts, so the absence reads as the product working rather than as an install left half done. Neither can be automated, because both add or change an Xcode target and a pub package cannot. So the doctor warns and the installation doc carries the manual steps. Warnings rather than failures: an app that never wants rich notifications is a legitimate build, and a doctor that fails one stops being read. The two halves are checked separately because they fail independently. An extension with no shared App Group gives rich media and still no confirmed delivery, since the container is how the extension hands what it saw back to the app. Splitting getWarnings into configWarnings plus the platform ones came with it: rendering every warning under "Config Validation" filed an Xcode target's absence as a config finding and printed it twice, and the closing line claimed push "cannot send yet" over something that does not stop a single notification. The doc also gains the cold-start caveat OneSignal documents and that cost real time here: on iOS in Debug a force-closed app opened from a notification never registers the click listener, so the cold path can only be tested from a profile or release build. * chore(release): cut 0.3.0 Minor rather than patch, following this package's own cadence (0.0.3 -> 0.1.0 -> 0.2.0, minor for a feature release). The unreleased section carries the cold-start click buffer, which is the fix, and the bulk preferences row, which is a feature that needs two translation keys an adopter has to supply: a raw key renders where they do not, so a patch would slide a soft break into every consumer on `^0.2.0` without a bump. That choice costs two edits elsewhere, and forgetting the first is how a release train breaks: `magic_starter` pins `magic_notifications: ^0.2.0` and uptizm pins the same, so both have to follow before either resolves. Version lives in four places besides the manifest and the package gates two of them itself: `install_command_test.dart` compares `magicNotificationsVersion` against the pubspec, and `skill_reference_stamp_test.dart` compares the sibling magic repo's reference stamp. The second is red locally until magic's `feature/plugin-skill-references` lands, and skips in CI, which clones no siblings. Not published. The publish is the owner's to run. * Revert "chore(release): cut 0.3.0" This reverts commit b69bf13. * Reapply "chore(release): cut 0.3.0" This reverts commit dbb4e7a. * fix(notifications): gate the click buffer on hasListener, not on a getter read Two holes in the cold-start buffer this branch added, both found by review and both confirmed against the source before acting. The buffer asked "has anybody read this stream", and the question that matters is "is anybody subscribed". **Reading a stream and listening to it are different moments, and only the second can receive anything.** `onPushClicked` set `_pushClickedHeard` on the getter and scheduled the drain there, so a consumer that captures the stream, awaits its router and subscribes afterwards flipped the flag, had the microtask drain into a broadcast controller with no subscriber, and lost the launch tap in the exact way this buffer exists to prevent. **`forgetDrivers()` could not close the second hole.** It reset the flag but cannot cancel a subscription somebody else holds, and it is public through `Notify.forgetDrivers()`. An app calling it on sign-out kept its boot-time subscription, so the manager buffered again for a listener that was already there and never drained, because nothing reads the getter twice. Every tap after a sign-out was swallowed in silence. Both close on the same change: the buffer fills when `_pushClickedController.hasListener` is false and drains from that controller's own `onListen`, which fires on each 0 -> 1 transition. The drain refuses an empty controller, so a subscriber that cancels inside the turn it subscribed leaves the buffer for the next one instead of having it added to nobody. The window therefore reopens on any later gap between subscribers rather than closing for the life of the process, which is the honest shape: a tap that arrives while nothing is listening is a tap nobody received. The replay is also re-judged rather than replayed blind. A tap buffered while the device was subscribed as one person and drained after `want()` moved it to another handed the deep-link bridge the previous identity's payload, which is precisely what the receive-side guard exists to stop; `forgetDrivers` clears the buffer, `want` does not. Three tests, each verified to fail against the previous implementation and pass against this one: read-then-subscribe-later, a live subscriber surviving `forgetDrivers`, and a buffered tap dropped after the identity moves. Also from the same review: - `notifications:doctor` identified a Notification Service Extension by a `.appex` suffix, which every app extension has. A project shipping a widget and no NSE reported it present and only nagged about App Groups: a false green on the one check whose justification is that its subject looks exactly like the product working. It now requires the `.appex` AND an `Info.plist` under `ios/` declaring `com.apple.usernotifications.service`, since the extension point lives in the target's own plist and the target can be named anything. Runner's own plist is excluded. Covered by a test that fails against the old check. - `skill_reference_stamp_test.dart` called `.first` on the reference's lines, which throws `Bad state: No element` on an empty file and replaces the carefully worded reason with a StateError naming nothing. Verified: `flutter analyze` clean, `dart format` clean, 623 tests pass. The one failure is the stamp gate, which reads the sibling magic checkout and is red until fluttersdk/magic#152 lands; it skips in CI. * fix(doctor): stop the NSE walk crashing on a binary plist in ios/Pods The extension-point check I added last commit was right about WHAT identifies a Notification Service Extension and wrong about where to look for it. Reviewed, reproduced, and confirmed by hand before fixing. Walking all of `ios/` recursively and reading each `Info.plist` as UTF-8 crashes on any real project. `ios/Pods` is full of vendored frameworks whose plist is a BINARY plist; `readAsStringSync` throws a FileSystemException on one, nothing here caught it, and `getWarnings()` is called unguarded from `handle()`, so `notifications:doctor` and the `notifications_doctor` MCP tool both blew up instead of printing a warning. It only bit a project whose pbxproj already names a `.appex`, which is exactly this check's audience, and OneSignal's own iOS SDK arrives as an XCFramework through CocoaPods. Verified against a real binary plist written with plistlib: `readAsStringSync` throws, `utf8.decode(bytes, allowMalformed: true)` does not. The same walk followed links, so it descended `ios/.symlinks/plugins/*` into the pub cache and all of Pods. A dependency shipping an NSE template plist then read as THIS app's extension, which is the false green the extension-point check was added to remove. It also paid a full Pods walk on every run. Both close together: the walk starts at the immediate children of `ios/`, skips the directories that are never an app target (`Runner`, `Pods`, `.symlinks`, `build`, `Flutter`), never follows a link, and decodes tolerantly. Excluding Runner by NAME rather than by a `'/Runner/'` substring also fixes the third finding: that test did nothing on Windows, where the separator is a backslash. Two tests, both verified to fail against the previous commit: a binary plist under `ios/Pods` (asserting `returnsNormally` as well as the warning, since the regression was a crash) and a dependency's NSE template that must not count. Verified: analyze clean, format clean, 625 pass. The one failure is the stamp gate against the sibling magic checkout, which sits on another branch here; fluttersdk/magic#152 is merged, so master carries v0.3.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three agent-facing plugin references under
skills/magic-framework/references/refreshed against the source they document, plus the version stamps moved to the releases about to go out:magic_notifications v0.3.0andmagic_deeplink v0.1.0.Why
These files are what an adopter's agent actually reads.
.pubignoreexcludesCLAUDE.mdand.claude/from the published packages, so the reference in this repo is the contract, and it is versioned by this repo's releases rather than by the plugins', which is how it drifts without anyone noticing. Measured once: the notifications reference was stamped v0.0.3 against a shipped 0.2.0, with six breaking entries in the CHANGELOG in between.The load-bearing correction: both references told adopters that service-provider order does not matter. It did. A consumer whose provider list put notifications before
magic_deeplinkdropped the tap that cold-started the app, becauseonPushClickedis a broadcast stream and a broadcast stream drops what it publishes to nobody. Which order a consumer ended up with was decided by the order the two packages happened to be installed in, so it was a coin flip rather than a misconfiguration. The packages now buffer that click, so the advice is true again, but it was published as reassurance while being false.Blocking
magic_notificationsandmagic_deeplinkeach assert their stamp from their own suite (skill_reference_stamp_test.dartreads the sibling checkout and skips in CI). Both are red locally until this merges, so this goes first in the release train:magic_notifications0.3.0magic_starter(pin follows to^0.3.0) alpha.27magic_deeplink0.1.0