Windows Modern Standby suspends the app while the Helper's Core keeps running, so a frame that was half written when the host stopped draining timed out and closed the connection, which ended the Core with it. A timed-out write on a half-written frame is now resumed for as long as the stall lasts, and only a hard error still closes. go-winio reports the expiry as its own ErrTimeout, so the check goes through Timeout(). Changelog: Keep the core running while Windows sleeps with the app suspended
39 KiB
Rules
These are repository coding and testing conventions. Codex command permission rules belong in .codex/rules/*.rules; see .agents/agent-config.md before adding those.
Dart and Flutter Style
The lint set lives in lint_options.yaml at the repo root. The root analysis_options.yaml and every local plugin under
plugins/* include it, so application and plugin code are held to the same rules. Add or change a rule there, not in an
individual analysis_options.yaml; those files carry only their own analyzer.exclude entries.
lint_options.yaml enforces these non-default rules:
prefer_single_quotes: true: always use single quotes.require_trailing_commas: true: use trailing commas in multi-line argument lists.sort_child_properties_last: true:child:must be the last named parameter.avoid_print: true: do not useprint()calls.prefer_const_constructors: trueandprefer_const_declarations: true.prefer_final_locals: trueandprefer_final_in_for_each: true.always_declare_return_types: true.only_throw_errors: true: throw anExceptionorError, never a bareString.
Failures whose whole content is a message meant for the user throw
MessageException from lib/common/exception.dart. Its toString() is the bare
message, which is what globalState.safeRun surfaces in the dialog, so the
user-facing text is unchanged from the older throw someMessage idiom while the
throw stays catchable as an Exception and carries a stack trace. Assert on it
with isA<MessageException>().having((e) => e.message, 'message', ...), not on a
raw string.
Corner Radius
Rounded corners are superellipses everywhere, not circular arcs. Use the superellipse API at each layer:
- Shapes:
RoundedSuperellipseBorderinstead ofRoundedRectangleBorder. - Clips:
ClipRSuperellipseinstead ofClipRRect. - Container decorations:
ShapeDecoration(shape: RoundedSuperellipseBorder(...))instead ofBoxDecoration(borderRadius: ...); borders move to the shape'sside, and aContainerwithclipBehaviorstill clips to the shape path. - Canvas:
canvas.drawRSuperellipse(RSuperellipse.fromRectAndRadius(...))instead ofdrawRRect.
Passing BorderRadius.circular(x) as the borderRadius argument of these APIs is expected — it only
carries the corner magnitude; the rendered geometry stays a superellipse.
APIs that accept only BorderRadius keep circular corners, with the superellipse supplied by an
enclosing clip or shape where one is needed: InkWell.borderRadius, OutlineInputBorder,
ScrollbarThemeData.radius, and smooth_sheets' MaterialSheetDecoration. Fully round pills
(BorderRadius.circular(999) or half the shortest side) may stay circular — both geometries coincide
there.
CI gates formatting: dart format --output=none --set-exit-if-changed lib test tool plugins setup.dart runs before flutter analyze.
Generated directories are excluded from analysis:
build/**lib/l10n/intl/**lib/**/generated/**plugins/**
Comments
Comments are part of the code and some of them are load-bearing. What this repository limits is the useless and the frequent. Every comment that restates the code devalues the ones carrying real information, until readers skim past all of them — a file with three comments that matter is more readable than one with thirty.
The two halves are enforced differently, because only one of them can be counted. Whether a comment is useless is a
judgment, and it lives in the rules below and in review. Whether comments are too frequent is arithmetic, and
comment-density measures it.
What a Comment Must Earn
- Write one when it carries something the code cannot: a non-obvious constraint, an upstream or platform behavior being worked around, a reason the next reader would otherwise get wrong, a coupling to a value defined somewhere else.
- Never restate what the code already says. If the comment and the line below it convey the same thing, the comment is noise.
- Never narrate. Not what this diff changed, not what the code used to be, not the order you did things in. Version history belongs in git and change rationale belongs in the commit message.
- Never annotate line by line or statement by statement. A block that seems to need a comment per step needs better names or a smaller decomposition instead — reach for structure before prose.
- Prefer encoding the intent in structure and naming. A named mixin, type, or method that makes the invariant hard to break beats a paragraph asking the next reader not to break it.
- Delete commented-out code, stale version notes, and comments that only restate the code, whenever you edit the file that contains them. This does not need approval.
- These are not comments and are exempt from every rule here, including the density count: analyzer and linter
directives (
// ignore:,// ignore_for_file:,// coverage:ignore), license and copyright headers, code-generation markers, and comments inside vendored upstream code such aslib/widgets/open_container.dart.
Density
tool/check_comment_density.sh fails a file whose added lines are more than 5% standalone comment lines, ignoring
diffs under 20 added lines so small edits are never caught. The number is calibrated on this repository's own history:
healthy changes sit at or under 3.6%, while the core fix that prompted the gate ran 22.4%.
It is a ceiling on frequency, not a target to fill. Do not read core/'s inherited mihomo density as a quota either —
it is forked upstream code, not a house style.
Three gates run the same script, and they do not have equal force. The PostToolUse hooks in .claude/settings.json
and .codex/config.toml run after the tool and hand offending lines back as feedback; neither can undo the completed
write. The comment-density pre-commit hook fails the commit, and it is the only hard gate that covers every tool.
Behavior is pinned by tool/check_comment_density_test.sh, which CI runs directly.
When a change genuinely warrants more, raise the ceiling for that run rather than working around it:
COMMENT_DENSITY_MAX=20 git commit, or SKIP=comment-density git commit to step past it entirely.
Where Knowledge Belongs
A comment is one of three destinations, and often the weakest. Pick by where the constraint would be violated, not by how important it feels.
- Assertable behavior goes in a test. A test is the only form that cannot drift, because it fails when the behavior it describes is broken. Prefer it over both a comment and a document whenever the fact can be checked in code.
- Repository-wide defaults, ownership, and invariants go in
.agents/*.mdor a.agents/skills/*/SKILL.md. They are violated from many files, so they must reach every future agent at session start. A comment in one file cannot do that. - A fact that is true only at one call site, and is not visible from that call site, is the one thing a comment does
better than a test or a document. Its value is being in the reader's line of sight at the moment of the edit.
lib/common/constant.dartcarries one: the delay-test concurrency cap is bound todelayTestConcurrencyincore/common.go, and whoever changes that number must see the constraint on the same screen. This is the case a comment is for; the bar is that no test and no.agents/entry could hold the fact instead.
Both failure directions are real. Moving a local constraint into .agents/ hides it from the person editing the line;
leaving a repo-wide policy as a comment reaches only the reader of that one file.
Core API Safety
core/Clash.Metais a fork of mihomo, and changes to it are budgeted for features, not repairs. Fixing a bug there is low priority even when the bug is real and the fix is small: every patch is one more thing to carry across an upstream rebase. Solve it on the FlClash side of the boundary and note the mihomo behaviour you are working around. Reach into the submodule only for a feature that has nowhere else to live, or when the problem is one the FlClash patches themselves introduced — and say which of the two it is in the commit message.- Do not expose direct filesystem deletion APIs through Core or helper IPC; use a scope-specific cleanup API instead.
- The desktop IPC socket in
plugins/rust_apiadmits a Unix peer only when its effective uid is the app's own or root: the socket lives in/tmp, and the Core connects as root whenever it runs setuid for TUN or under the Linux Helper. A launch mode that runs the Core as any other user has to widenauthorize_peerinipc/platform.rs, and Windows keeps its identity check on the Dart side, which compares the named-pipe peer PID with the Core it launched. - Keep the shared
CoreMethodCall/CoreMethodResponseJSON envelope structurally identical across Dart, Go, JNI, and desktop IPC. Do not double-encodearguments,result, or event batches. core/message.gocarries three event queues, and the split is load-bearing: state (loaded, geo-update), delay, and bulk (log, request). Delay and bulk evict their own oldest entry under backpressure; state usesenqueueState, which never evicts, because a droppedgeoUpdate{updating:false}leavesisUpdatingProviderstuck at true in the UI untilUpdatingActionsweeps it as stale minutes later. Do not merge the tiers or give state eviction semantics.enqueueStatedrops silently on a full queue and must stay that way: reaching it means the host stopped reading, whichlogDeliveryErroralready reports, and reporting it from the message layer feeds the same batcher.jni_get_stringinandroid/core/src/main/cpp/jni_helper.cppmallocs and hands ownership to Go, which frees throughfree_string_func.quickSetuprelies on that: it reads its*C.chararguments inside a goroutine, after the JNI wrapper has already returned. Switching the wrapper toGetStringUTFChars/ReleaseStringUTFChars, or freeing on the C side, turns that read into a use-after-free.- Go goroutines reach Java through
ATTACH_JNI(), which attaches once withAttachCurrentThreadAsDaemonand detaches from apthread_keydestructor at thread death. Do not restore a detach-per-call:protectruns once per outbound socket andonResultonce per event batch, and attach/detach takes ART's thread-list lock each time. - Every JNI call into Kotlin must be followed by
jni_clear_exception. A pending exception left in place aborts the process on the next JNI call on that thread, so a throw inprotect/resolveUid/resolvePackage/onResultbecomes a crash in unrelated code. For the same reason every one of those wrappers checks itstun_interface/invoke_interfacefornullptrfirst: ART aborts on a call through a null object, and a callback released byTunHandler.clearwhile a connection is still resolving is exactly that. - The Android bridge resolves an owner in two steps —
resolve_uidthenresolve_package— because mihomo fillsmetadata.Uidfrom a procfs lookup that Android Q closed off. Collapsing them back into one call that returns only a package name is what left every connection reporting uid 0, soUIDrules matched nothing. - The desktop delivery path in
core/server.gomust not report failures throughlogError. A log event is published to the log subscriber, batched, and handed back tosend, so a send failure reported that way feeds itself; uselogDeliveryError, which writes to stderr and latches until a frame gets through or the next connection is installed. A write that fails without putting a byte on the wire — host backpressure hittingipcWriteTimeout, or a payload abovemaxIPCFrameSize— drops that one frame and keeps the connection: the stream is still framed correctly, and tearing it down here ends the read loop, and with it the Core process. A half-written frame whose write merely timed out is resumed for as long as the stall lasts: Windows Modern Standby suspends the app while the Helper's Core keeps running, so the host can stop draining for hours and still come back, and go-winio reports the expiry as its ownErrTimeoutrather thanos.ErrDeadlineExceeded, which is whysendchecksTimeout(). The wait has no cap on purpose, andsendholdswriteMuthroughout, so every other frame — method responses and the single batcher goroutine behind the event queues — waits behind the stalled one. Memory is bounded by the queues; what gives is delivery: the state queue fills andenqueueStatestarts dropping, which is the case theUpdatingActionsweep above exists for. Nothing on the Dart side restarts the Core over a stall:CoreRpcClienttimes each pending request out on its own and hands the callernull(ano_responseexception for message methods), and the sweep clears core-scope updating state minutes later. Only a half-written frame that fails outright desynchronizes the stream, and that is the one casesendcloses on. - Core method handlers in
core/hub.goare synchronous. Anything that must not block the dispatcher is spawned bysafeGo/safeGoDetachedincore/method.go, which recover; a baregoin a handler puts a panic outside every recovery and kills the process, which on Android is the whole application. The//exportentry points incore/lib.godo not reachhandleMethodCall, so each one carries its own recovery. dialer.DefaultSocketHookandprocess.DefaultPackageNameResolverare installed exactly once, byinstallHooksincore/lib.go, and never cleared. mihomo checksDefaultSocketHookfor nil once and dereferences it again when the socket is created (component/dialer/socket_hook.go), so clearing it while a dial is in flight calls a nil func value. Stopping the TUN swapsactiveTunHandlerinstead.tunnel.AllProxies()returns a shared, cached map — never modify it. The cache is invalidated byinvalidateAllProxiesontunnel.UpdateProxiesand validated against each provider'sVersion(), so a rebuild costs one read per provider rather than one per proxy. Anything else added totunnel/patch.gothat derives from the proxy set needs both signals: the external controller can reload the config throughhub/route/configs.gowithout going through FlClash'sapplyConfig, so a hook on the FlClash side alone would miss a profile switch.- Core state that mirrors mihomo state goes stale at the next
applyConfig, which replaces every proxy, provider and rule. Read the tunnel instead of caching a snapshot of it:lookupExternalProviderkept one that was rebuilt only when the host asked for the provider list, and the host asks after a successful setup and not after a failed one, so an update ran against a provider the tunnel no longer held — downloading, writing to disk and reporting success against nothing. - Selection writes take
selectMu, notconfigMu. mihomo'sSelector.Sethas no lock of its own, so the writes need mutual exclusion against each other and againstpatchSelectGroup— but not against a whole config apply, which is whatconfigMumade a proxy switch wait for, provider downloads included.patchSelectGrouptakesselectMuunderconfigMu, fixing the order asconfigMu→selectMu. - The delay-test semaphore is acquired with a slice of the caller's budget (
budget/delayTestQueueShare), not unconditionally and not with the whole deadline. Queueing and probing come out of one budget, so a test handed all of it can spend it waiting and reachURLTestwith nothing left, reporting a proxy it never contacted as unreachable. The probe keeps the caller's original deadline, so whatever the queue did not use is still its own. - A delay test that the Core does not answer is a fault of the Core or the channel, never a verdict on the proxy:
handleTestDelayreturns inside its own budget on every path.asyncTestDelaytherefore returns null instead of a-1delay, andProxiesActionleaves the last measurement in place and abandons the rest of the run. Writing a timeout there is what made a reachable node read as unreachable whenever the host deadline beat the Core's. - Delay-test progress lives in
pendingDelayTestsProvider, not as a sentinel value inDelayDataSource. A delay of 0 used to mean "testing", which let a result and the state of a test overwrite each other and left cards spinning forever when the Core restarted. The run owns its keys and releases them in afinally, so nothing depends on a reply arriving; core status leavingconnectedcancels every run in flight. - Anything on the mihomo side that is reached from both a user-triggered core method and mihomo's own background
scheduler needs its in-flight guard on the FlClash side.
updater.UpdateMMDBand its siblings have none — only the batchUpdateGeoDatabasesdoes — and two concurrent runs close the mmap'd database twice, sohandleUpdateGeoDataclaims per resource andupdater.GeoUpdateHookreleases. - A failed
applyConfigrolls the tunnel back to the default config, and that rollback is the whole recovery:handleSetupConfigreturns the error and stops there. Do not add a teardown on top of it — stopping the listeners takes the app offline over a profile the user can still switch away from, and the error already reaches the host, which is what surfaces the failure (MessageExceptionon the Flutter side, the config-error toast on Android). Keep an emptyconfig.yamlout of the failure path — it is how the app says "no profile selected", andloadConfigresolves it to the defaults. - Package
initin the Android library runs while the.sois being loaded, so a panic there takes the application down before it can report anything.platform/limit.goarms an fd-pressure probe and degrades to never blocking when it cannot; keep that shape for anything elseinitsets up that correctness does not depend on. - Every
android && cgofile incore/is compiled only by the NDK-backed CI step in thegojob. Keep the build constraints asandroid && cgo/!(android && cgo): a barecgoconstraint makesgo build ./...fail incore/on any developer machine, because the files it pulls in need the NDK.
Lifecycle Rules
- Crash recovery is owned by
BootGuard(lib/common/boot_guard.dart), and the signal it acts on is the persistedBootRecord, never a crash reporter. The record is stampedstartingbeforestartCoreandrunningonce_initAppfinishes, so only a launch that died before reachingrunningis a failed launch — a crash after hours of runtime, or a process the system reclaimed in the background, is not one.ApplicationExitInfocan only veto a failure (user stop, low memory, signal, package change), never create one, and its timestamp is consumed throughhandledExitAtso the same exit cannot be counted twice. BootGuardis an Android-only mechanism and gates itself: on every other platformevaluate,markRunningandmarkClosedreturn without touching preferences. The desktop has neither of the two attribution sources — noApplicationExitInfoequivalent and no Crashlytics toggle — so a bare sentinel there would read a window closed on the disclaimer dialog, or a session ended by shutdown, as a failed launch. Keep the platform check inside the guard; callers inbootstrap.dartandSystemActionstay unconditional.FirebaseCrashlytics.didCrashOnPreviousExecution()corroborates a failure and must not trigger one. Its marker file is cleared by a background initialization thatdataCollectionArbiterblocks while collection is disabled, which is the default here, so one real crash makes it return true on every later launch. That latch is what made the app clear the selected profile on every cold start; it is also why the probe is only read whencrashlyticsis enabled, keeping Firebase uninitialized until the user consents.- Recovery is graded: the first failed launch only skips
initStatus, andcurrentProfileIdis cleared only fromcrashRecoveryClearThresholdconsecutive failures on. Do not let a single interrupted launch write to the config. - Desktop process ownership belongs to
DesktopCoreLifecycle; do not start/killFlClashCorefrom providers, widgets, managers, or ad hoc exit callbacks. Acquire and release it through aCoreProcessLease. CoreController.close()and platformclose()implementations are terminal and idempotent. Application shutdown must stay centralized inSystemAction/SystemExitCoordinator.- Android start/stop MethodChannel calls are optimistic UI commands. Keep latest-wins arbitration in native
ServiceState; do not add a Flutter completion callback that creates a second lifecycle owner. - Android service callbacks are not automatically user intent. Route explicit Quick Settings, Always-on VPN, and revoke
actions through
ServiceStateand keepServiceControlleras the sole binding/run-time owner. - Every
BroadcastReceiver.goAsync()path must finish itsPendingResultexactly once. A watchdog may release the broadcast lease, but must not cancel, reverse, or otherwise redefine the service operation. - Presentation smoothing such as
CoreStatusButton's connecting hold must remain local display state. It must not delay or overwritecoreStatusProvider, and a real failure must bypass/cancel the hold immediately. Tray.hide()is idempotent on all three desktop platforms and returns native state to "showwas never called".AppTray.shutdown()latches, so no laterupdate()/updateTitle()can resurrect the icon once shutdown begins. Keep it that way; a resurrected icon outlivesexit(0)as a Windows ghost icon, becausesetPreventClose(true)meansWM_DESTROYnever runs.- The
trayplugin owns call ordering, idempotency, serialization, and unchanged-payload suppression. Application code declares desired state through oneTray.show(TraySpec)call and must not add platform branches to work around ordering. Platform branches inlib/common/tray.dartare only for deliberate product differences (macOS speed title and group submenus); queryTray.instance.capabilitiesfor ability differences. - Every native
showreturns whether the tray now reflects the payload, and reportsfalseinstead of showing a broken icon.Traycaches the payload signature only ontrue, so a rejectedshowis retried by the next update rather than suppressed until restart. Any test that mocks thetraychannel must returntruefromshow. - Reading the Wi-Fi SSID is opt-in work, not ambient state.
ConnectivityManagerreads it only whileexcludeSSIDsis non-empty, because that list is its only consumer throughsuspendProvider, and the read costs a blocking platform call plus a location permission on Android and macOS. A second consumer must widen that gate, not drop it. - On Android the
wifi_ssidpermission is onlygrantedonceACCESS_BACKGROUND_LOCATIONis held too (Q+). Fine location alone reads the SSID in the foreground but returnsUNKNOWN_SSIDonce the app is backgrounded, which is where on-demand spends most of its time.getSsiditself only needs fine location, so the split stays inside the plugin. - No
wifi_ssidimplementation may answergetSsidon the platform thread. Android resolves through a network callback with a timeout, Linux throughg_task_run_in_thread, and macOS throughssidQueueafter checking the location authorization, because CoreWLAN reacheswifidover XPC and a wedged daemon would freeze the window. Windows is the outstanding exception:WlanQueryInterfacestill runs inline, since replying off-thread there needs a window-message hop that the plugin API does not provide. - The delayed DNS re-check
NetworkObserveModule.onLosingposts is deliberately left un-deduplicated. The runnable re-readsnetworkInfosand does nothing whenonLostalready dropped the network,updateDnsreturns early when the resolved list is unchanged, andstop()clears the handler queue, so a network that reportsonLosingrepeatedly costs one comparison per event. Holding the pendingRunnabletoremoveCallbacksit changes no behaviour and adds state that has to stay in sync with the map.
Testing Rules
The core/ directory is excluded from automated coverage accounting. Do not add coverage instrumentation or coverage
collection for code under core/. CI still runs CGO_ENABLED=0 go test . and go vet . to compile/check the Go wrapper,
plus an NDK-backed GOOS=android vet that covers the android && cgo files the first two exclude; verify cross-language
protocol behavior through shared Dart contract tests under test/core/ and native platform build checks.
A Go test in core/ that reaches sendMessage — directly, or through handleStartLog or updater.GeoUpdateHook —
leaves events in the process-wide batcher, which flushes them up to messageBatchInterval later into whichever
connection captureFrames has installed by then. Either keep the event out of the batcher or end the test with
settleMessageBatcher. That batcher runs for the whole test binary and reads conn under connMu, so install a test
connection with swapConn; a bare assignment races every event it happens to be delivering, and go test -race catches
it in an unrelated test.
Use CoreController.test(mock) to inject a mocked CoreHandlerInterface. Call CoreController.resetInstance() in tearDown to clean up the singleton between tests.
Register fallback values for freezed params used with any() matchers.
tool/check_coverage.dart enforces a total floor passed by CI plus per-group floors declared in _groupFloors. Raise a
group's floor when new tests lift it; do not lower one to make a run pass.
Every measured group needs a floor. A group the report measures but _groupFloors does not declare fails the run, so
adding a top-level directory under lib/ means adding its floor in the same change. Set a new floor at or just below
the coverage the directory actually has; the point is to stop a slide, not to backfill tests before the directory can
land.
Prefer coreHandlerProvider.overrideWithValue(CoreController.scoped(fake)) over CoreController.test(fake) in new and
touched tests. CoreController.test claims the process-wide singleton, which makes a global read and a provider read
resolve to the same fake, so it cannot fail on a call site that still reaches for the global.
Construct the Android lib handler with CoreLib.scoped(fakeService). The service global is gated on Platform.isAndroid
and is therefore null on every test host, so a CoreLib() built from it silently takes the null-service fallback on every
path. CoreLib.scoped binds an explicit Service instead; reset the singleton with CoreLib.resetInstance() in tearDown.
Three globals in lib/common reach real host state and carry a @visibleForTesting seam to stand in front of it:
AutoLaunch.launcher, listNetworkInterfaces and LinkManager.uriLinkStream. Replace the launcher in particular — every
enable/disable/isEnabled writes the actual autostart entry (a LaunchAgents plist, a .desktop file or a registry
key), so a test that skips the seam registers the test binary on the machine that ran it. updateStatus returns early
under kDebugMode, which is always true beneath flutter test, so its remaining branches cannot be reached from a test
at all; test/common/launch_test.dart pins the early return instead.
pumpAndSettle never returns on a page holding EditorPage: the code editor blinks its caret forever, so frames keep
being scheduled. Pump explicitly instead. encodeYamlTask and its neighbours in common/task.dart hand work to a real
isolate through compute, which only runs outside the fake-async zone, so a test awaiting one needs
tester.runAsync(...) between the pumps — see test/views/profile_preview_test.dart.
The @visibleForTesting database setter in lib/database/database.dart deliberately does not close the instance it
replaces. Tests inject NativeDatabase.memory(), which holds no file handle, and Database.close() is async while the
setter is not, so closing there would either be unawaited or force the seam to become async for no gain. A test that
does open a file-backed database owns closing it.
system.isAndroid / isMacOS / isWindows / isLinux read dart:io Platform and cannot be overridden, unlike
debugDefaultTargetPlatformOverride. A branch behind one of them is only ever exercised on a host that matches it, so CI
(ubuntu-latest) and a macOS working copy measure different coverage for the same test. Assert host-agnostic behavior,
and leave headroom under a group floor that covers such a branch.
A platform decision that drives layout takes isDesktop/isMacOS as parameters and reads system only at the call site,
so every platform's outcome is reachable from one host. getWindowHeaderHeight and showsWindowHeader in
lib/common/layout.dart own the window header rule for both WindowHeaderContainer and overlayTopOffset — they must
agree, or the content is offset by a header that is not there. WindowHeaderBar takes its height and slots as arguments,
which is what lets test/manager/window_header_test.dart measure the Windows caption bar on a macOS host.
A Stack under loose constraints sizes to its non-positioned children, and falls back to constraints.biggest only when
it has none — so a bar that fills the window must keep every slot positioned. One non-positioned child is enough to
collapse WindowHeaderBar to that child's width: the macOS title did exactly that, leaving the app name over the traffic
lights and the raw window painted black beside it, while Windows, whose slots are all positioned, stayed correct.
WindowHeaderLayout positions the header across the top for the same reason, and the macOS group asserts the bar's
width, not just its height — a height-only assertion cannot see this.
The same seam carries the two other places a platform decision changed what was built: AppTray holds isMacOS/
isWindows as state — AppTray() fills them from system, AppTray.forPlatform is the test seam — and OnDemandView
takes nullable isAndroid/isMacOS that fall back to the host. Every test names the platform it means, because
debugDefaultTargetPlatformOverride does not move system and a suite that leaves it to the host asserts the macOS
shape on a developer machine and the Linux one on CI. WindowHeaderContainer builds the caption buttons on every
non-macOS host, so a test mounting it needs TestApp for AppLocalizations and a window_manager channel mock that
answers isMaximized/isAlwaysOnTop with a bool.
Auto-dispose providers need a container-level hold before a test reads them back. proxyGroupProvider, ruleProvider,
itemsProvider and friends mix in AutoDisposeNotifierMixin, so a container.read that no widget is currently watching
rebuilds the provider from its override and silently discards whatever the code under test wrote. Add
container.listen(theProvider, (_, _) {}) in the harness, as overwrite_stage_flow_test.dart does. The staging flow also
re-arms its debounce when it clears the stage, so drain it (pump past the duration, then unmount) or the binding fails
the test on a pending timer.
A field that constructs its own ValueNotifier, TextEditingController, ScrollController, FocusNode, TabController,
PageController, AnimationController or StreamController must be released in the same file.
test/lint/disposable_field_test.dart enforces this by scanning lib/, because no lint covers it: close_sinks only sees
sinks, and nothing in the standard set tracks ChangeNotifier disposal. A field that genuinely outlives its owner goes in
that test's _allowed set with the reason, not left bare. Controllers received as widget parameters belong to the caller
and are out of scope.
An IconButton whose icon is an icon needs a tooltip. It is the button's only accessible name — without it TalkBack and
VoiceOver announce nothing and the desktop build shows no hover hint. test/lint/icon_button_tooltip_test.dart enforces
it and skips exactly two shapes: an icon: holding a Text, which is already a visible label, and
views/dashboard/widgets/core_status_button.dart, which takes its label from an enclosing Tooltip (a second test fails
if that wrapper disappears). Reuse an existing string before adding one; a label that depends on state goes on the button
inside the ValueListenableBuilder, not outside it, or the tooltip cannot follow the icon. A row of window buttons hidden
behind system.isMacOS is unreachable from a macOS test host, so extract it — WindowHeaderActions is the pattern.
A tooltip needs an Overlay ancestor at build time, not at hover time: RawTooltip builds an OverlayPortal, so a
button whose tooltip has nowhere to go throws "No Overlay widget found" and takes its whole subtree down with it.
Everything buildManagerStack wraps around MaterialApp.builder's child sits above the app Navigator and therefore
above the only Overlay in the tree. A manager that renders a tooltip — WindowHeaderLayout and its caption buttons are
the case that broke Windows while macOS, which renders a bare title there, stayed clean — hosts its own with
Overlay.wrap, spanning the window so the tooltip is not clipped to the widget that owns it. A test only reproduces this
by mirroring that topology: build the widget from MaterialApp.builder, never from home:.
A public top-level declaration that nothing outside its own file references is dead, and no lint catches it:
unused_element covers only private ones, and a barrel export keeps a dead file compiling and off every
"unused import" report. test/lint/dead_file_test.dart scans lib/ for files whose declared names — types plus the
final appPath = AppPath() singletons next to them — appear nowhere else, counting generated code as a consumer (a
riverpod notifier is reached through its generated provider) and barrels as neither. Files publishing only extensions or
typedefs are skipped: those are reached through the types they attach to, never by name.
A State.dispose() override must not await before super.dispose(). StatefulElement.unmount calls dispose() and then
immediately asserts that super.dispose() already ran, so an await defers the call past the assert and every teardown
throws "…State.dispose failed to call super.dispose." in debug and profile builds. Declare the override as void dispose() and hand async teardown to unawaited(...); Future<void> dispose() async compiles and is the shape that
invites the bug.
Use ProviderContainer directly for simple Riverpod provider tests. The generated Riverpod update() method takes a callback:
notifier.update((state) => newValue);
When testing freezed models with nested objects, always round-trip through jsonEncode and jsonDecode. Direct fromJson(toJson()) fails for nested freezed types because toJson() stores child objects directly instead of maps.
For async widgets, put visual cleanup in finally when the action may throw. Focused widget tests should cover success,
failure, disposal, and any timer boundary that changes visible state.
Commit Messages
Subjects follow Conventional Commits and are enforced by the commit-msg hook in .pre-commit-config.yaml, which runs
tool/check_commit_msg.sh:
<type>[(scope)][!]: <description>
- Types:
feat,fix,docs,style,refactor,perf,test,build,ci,chore,revert. - Scope is optional and lower case; use a comma to list several, as in
fix(core,android). !before the colon marks a breaking change.- Descriptions start in lower case, omit the trailing period, and keep the whole subject within 100 characters.
Identifiers and acronyms keep their own casing, as in
fix(ui): AppBar text is truncated. Merge/Revertsubjects andfixup!/squash!commits are exempt.- No
Co-authored-bytrailer crediting a coding agent, whatever that tool's own convention says. The history records who owns the change, not which tool typed it; human co-authors are still fine. The hook rejects the known agent identities.
Write what the change does, not that something changed: perf(views): stop redoing per-frame work in build, not
Optimize more details.
Install the hooks once with pre-commit install --hook-type pre-commit --hook-type pre-push --hook-type commit-msg.
Changelog Trailers
tool/changelog.dart builds the user facing changelog from the commit history, so the trailers below are the copy that
ships to users. The subject stays the developer facing summary and is only the fallback.
feat(profiles): support per-profile override script
Changelog: Per-profile override scripts
Changelog:is the English entry.Changelog: skipdrops the commit from the changelog entirely.- The changelog is English only. Translation trailers were removed on purpose: they pushed release copy into the commit
history, so any
Changelog-<locale>:orBreaking-<locale>:now fails the hook. Translate after the fact if ever needed, not in the commit message. Changelog-Type:moves an entry into another group, for example to promote arefactorthat users will notice. Valid values arebreaking,feat,fix,perf,revert.BREAKING CHANGE:is required whenever the subject carries!, and its text becomes the breaking entry. A!commit therefore needs two lines of copy: the footer for the breaking entry andChangelog:for the normal one.
feat, fix, perf, revert and breaking commits are collected by default; every other type is dropped unless it
carries a Changelog: trailer. Commits missing a trailer reuse their subject, and the hook says so without blocking.
Generated Code
Do not manually edit generated files under:
lib/l10n/l10n.dartlib/models/generated/lib/providers/generated/lib/database/generated/lib/l10n/intl/
After schema, model, or provider changes, run build generation and include focused tests when behavior changes.
lib/l10n/l10n.dart is the one file that still imports package:flutter/material.dart.
intl_utils hardcodes that import in its own template, so regenerating rewrites it and
there is nothing to fix here; test/lint/design_package_test.dart exempts the generated
l10n paths for that reason. It is harmless because the file only needs Locale,
BuildContext, Localizations and LocalizationsDelegate, which the legacy library and
material_ui both re-export from the same package:flutter/widgets.dart. Everything a
human writes takes Material from material_ui; cupertino_ui is banned outright and
survives only as a transitive dependency of material_ui.
Strings live in arb/intl_{en,zh_CN,ja,ru}.arb — flat JSON, no @ metadata. Add a key to all four, then regenerate with
dart run intl_utils:generate, which rewrites lib/l10n/. A key present in only some locales silently falls back to
English at runtime, so add the translation rather than leaving it out.
Some labels are not reached through the generated AppLocalizations getters at all. Intl.message(<runtime string>)
builds the key from an enum name or a stored string — action_${HotAction.name}, ${DynamicSchemeVariant.name}Scheme,
NavigationItem.description. The analyzer sees nothing, and a failed lookup returns the key itself, so a stale key ships
as routeMode_config in the UI rather than throwing. test/lint/dynamic_message_key_test.dart expands those families
from the real enums and fails when a derived key is missing from any locale; every Intl.message site in lib must be
registered there, so a new dynamic key cannot be added without also declaring what builds it.