Cleanup from a review pass over this branch. No behaviour changes except the
two noted below.
runInstall took app.Snapshot() twice, once inside manifestFor and once for the
cycle check. Snapshot is a deep copy of the registry, and worse than the
copying, the two calls could in principle disagree - the set the cycle check
validated was not provably the set the manifest came from. One snapshot,
passed to both.
appSummary converted a display code back to a stored one with
NormalizeAppCode, which is not that inverse: it leaves "core" as "core", so
the framework needed a branch of its own to stay out of the listing. AppFilter
is the documented inverse and maps it to the empty string, which is not a code
any row is filed under - so the branch goes, and the function now matches
filterAppsByApp twenty lines below it, which was already using AppFilter.
That branch only half-covered what it guarded: a sys_app row carrying an empty
or reserved app_code was still merged into the framework's group by
groupByApp, with only its summary suppressed. loadApps now drops such rows,
which is the one place that settles it for every reader of the map.
requiresInstalled built two parallel slices with a tuple assignment repeated in
three branches; it now picks a reason and appends once. Its last arm was a
catch-all on "not installed", so a status constant added later would have been
described as "did not finish" - a sentence that would be wrong for whatever
reason the constant was added. Unrecognised values now say so. It also takes
the normalised code the caller already has rather than computing it a third
time.
refuseOnDependencyCycle sorted each manifest's Requires before walking them.
Requires is a slice and already has a fixed order, so the sort bought no
determinism - that comes from the sorted outer loop, which walks a map - and
only made a reported cycle harder to line up against the manifest that caused
it. The filtering pass that went with it is covered by the registration check
underneath. The cycle path is trimmed with slices.Index, which also removes a
fallback return that the grey/path invariant made unreachable.
An application's manifest can name others it needs. Until now the list was
stored and never read.
It is checked, not satisfied. Installing the dependencies too would make
"install this application" mean "and everything it happens to name, and
everything those name" - a blast radius the operator did not ask for and
cannot see beforehand. What they get is the list and the order to do it in.
A dependency whose own install failed, or never finished, is not a dependency
that is there. The message says which, because the two send you to different
places: one to install it, the other to look at why it did not take.
The check runs before anything is written, so a refusal cannot cost the
operator the row that told them what they had.
Separately, a cycle anywhere in the registered manifests is refused, whether
or not the application being installed is in it. A cycle between two others is
still an authoring mistake, and the day somebody installs into it - with an
error naming two applications they did not ask for - is the worse time to find
out. The error is the cycle rather than the walk that reached it, and the
walk's order is sorted, so the same set of manifests always reports the same
one. Requires naming an application that is not registered is not a cycle; it
is the database's answer to give, at the time it matters.
Six degradations turn the new assertions red: accepting any dependency,
accepting a row regardless of its status, returning no cycle, not trimming the
reported path to the cycle itself, and running the check after the row has
already been written - the last of which was rebuilt after the first attempt
at it deleted the check rather than moving it, and so went red on the wrong
assertion.
The migration rows answer "did this run". They cannot answer "is this
application installed", and the difference is not academic: an install that
stopped partway leaves every migration reading applied and a row saying the
install never finished. Until now nothing printed that row.
[order] 1.0.0 failed at order-1793800000000
applied order-1793800000000 2026-09-10 21:02:07
The application list is the union of the two sources rather than either one.
Reading it from sys_app alone would drop an application whose migrations ran
under plain `migrate`, which records no row; reading it from the migration
rows alone drops one whose code has been taken out of the binary, which is
when somebody most wants to see it named - that one now gets a group of its
own, empty, saying why.
A database from before sys_app existed prints exactly what it printed before.
`migrate status` has to keep working on a database that has not been migrated
at all, which is when it is most wanted.
Four degradations turn the new assertions red: dropping the sys_app-only
applications from the listing, printing no summary, not narrowing sys_app by
--app, and reporting an unfinished install as an installed one.
`migrate uninstall <code>` removes what an application's install wrote and
leaves the application's own tables alone. Removing an order module is not the
same decision as destroying the orders, and nothing here can tell an operator
who is done with it from one who will reinstall tomorrow.
One transaction, and this one really is one: every statement is DML or a
SELECT, so unlike an install there is no DDL to commit it out from under
itself. Child rows go first, while the ids that identify them can still be
read from their parents, and the api paths are read before the rows carrying
them are deleted.
The two join tables need no ledger and get none. menu_id is a surrogate key,
so a sys_role_menu or sys_menu_api_rule row can only have come from a menu
this application wrote - there is no "looks like it but is not". A column on
sys_role_menu would have been worse than unnecessary: SysRole.Update deletes a
role's rows and writes them back through GORM's many2many, which does not
carry extra columns, so the column would be blanked the first time anybody
edited a role, silently. There is a test that performs that edit and then
uninstalls.
casbin_rule is the opposite case, because its key is business text somebody
else may have written for their own reasons. Policies are removed one at a
time, by exact tuple, and only the ones the ledger says this install created.
A tuple the ledger names that is no longer there is reported, not treated as a
failure - the uninstall wanted it gone and it is gone. Then, with everything
the ledger could speak for already dealt with, a read-only pass lists the
policies still naming this application's paths: those are grants somebody made
by hand, they are about to point at APIs that no longer exist, and they are
not this command's to delete. The two lists stay separate because they mean
different things - one is something of ours that had already gone, the other
is somebody else's grant now pointing at nothing - and merged into one "could
not remove" list neither would be actionable.
sys_migration's rows for the application go too. Without that a reinstall
finds every version applied, runs no migration, seeds nothing, and reports
success. It is the easiest step to leave out, because a migration record does
not look like the application's data.
A sys_app row is not required. `migrate` with no subcommand applies every
registered migration, an application's included, so an application can have
all of its rows without ever having gone through the installer - and that is
the case where nothing else can clean up after it.
Eleven degradations were applied one at a time, each red on the assertion it
was aimed at: skipping either join table, deleting sys_role_menu without its
filter, skipping sys_migration, deleting sys_migration without its filter,
matching policies by path instead of by ledger tuple, dropping the orphan
pass, treating a missing policy as a failure, leaving the ledger behind, soft
deleting sys_menu instead of removing it, and running the whole thing outside
a transaction.
`migrate install <code>` brings one application up to the version its manifest
declares: it runs that application's outstanding migrations and records what
it did in sys_app.
It goes under migrate rather than under the existing `app` command, which
already means "generate the skeleton of a new application" - a directory that
does not exist yet, not an application already compiled into this binary.
Installing one is running its migrations, which is what this command is, so
--domain, resolveDB and the guard that refuses a mistyped code instead of
reporting a successful no-op are all already here.
Three phases, each committing on its own, and they are not one transaction.
An application's versions are separate migration files, and on MySQL a DDL
statement commits the transaction around it - destroying an outer transaction
and every savepoint taken from it. So this does not promise that a
half-installed application cannot happen. It promises one is visible when it
does: phase A writes "installing" before anything that can fail, phase B runs
the migrations, phase C turns that into "installed" or into "failed" with the
version it stopped on.
What is left to apply comes from sys_migration, never from sys_app. sys_app
is a derived view - a summary, and the answer to "which version does this app
think it is at". If it were the authority, an operator who deleted
sys_migration rows by hand would be told an application is installed while its
schema is not, which is worse than not knowing. So "already installed, nothing
to do" needs all three: nothing outstanding, recorded as installed, and the
same version. A row stuck at "installing" - what it reads as after the process
was killed partway - is not installed, and retrying is just running the
command again.
An upgrade is in place and keeps the first install's time; a downgrade is
refused, and refused before phase A writes anything, so a refusal cannot cost
the operator the row that told them what they had. An unparseable recorded
version is refused the same way, while it is still readable.
The report ends by saying the code is not running yet. That is not a
pleasantry: Go links at build time and Vite resolves its import globs at build
time, so installing an application writes its menus, its APIs and its
permissions and cannot make one line of its code run - and the menus appearing
is exactly what makes an operator believe otherwise.
Ten degradations were applied one at a time to check the tests name the
behaviour rather than the shape: deciding the no-op from sys_app alone,
always writing installed_at, allowing the downgrade, keeping the previous
attempt's diagnostics on a row that now says installed, treating "installing"
as installed, truncating last_error by bytes so a Chinese message is cut
mid-rune, not recording the failure at all, skipping code normalization, and
writing phase A before either the downgrade or the version-parse refusal.
Each went red on the assertion it was aimed at. An eleventh was discarded
rather than counted: it failed in the first install's setup, not on the claim.
run() called log.Fatalf on the first migration that failed, which ended the
process from inside the migration engine. Nothing above it could record what
happened - an installer needs to write down which version an attempt stopped
on - and no test could exercise a failing migration at all without taking the
test binary with it, which is why the one test that covers a failed migration
drove the registered function directly and left the scheduler uncovered.
run(), Migrate() and MigrateApp() now return an error, and the exit moved to
the command layer where the exit code is the command's business.
Two of those errors say more than "it failed". A migration that fails comes
back as a *VersionFailure naming the version, because an installer records
that as a diagnostic snapshot - the authoritative answer to where a retry
resumes is always recomputed from sys_migration, never read back, and asking
the database what is still pending answers a different question that merely
has the same answer most of the time. An app code nothing registered under is
now an error rather than a log line, so an installer asking for one app by
name cannot be told that installing an app that does not exist succeeded; the
command layer still rejects a typo before any database work.
exitOnError is what makes the command exit non-zero, and it covers more than
it replaces. Every path out of migrateModel used to return without an exit
code: an unreachable tenant database or a failed AutoMigrate printed a line
and exited 0, so a caller that migrates before starting a server - the deploy
workflow does exactly that - carried on onto a schema that had not been
brought forward. A failing migration function was the only failure reported,
and only as a side effect of the log.Fatalf this commit removes.
Each of these was checked by degrading it and watching the named assertion
go red: returning nil instead of the failure, naming the first version rather
than the one that failed, accepting an unregistered app code, and not exiting.
One gap is left open deliberately. Go allows a call whose only result is an
error to stand as a statement, so `migration.Migrate.Migrate()` still compiles
while dropping what it returns - `go build` passed while migrateModel was
doing exactly that during this change. Both call sites now return the value,
which the compiler does check, but nothing guards against the statement form
coming back. A checksilent rule was considered and dropped: that tool parses
without type information, so it could only match the method name, and a guard
that fires on any type with a Migrate method is noise.
--app rejects a code nothing was registered under, on all three paths. It used
to take a typo as "nothing matched" and report success: migrate said the app
was unknown and still exited 0, while --dry-run and status printed the same
words an up-to-date database produces.
Every import of the module changes, not only the seven packages that
moved out of sdk/pkg: Go requires the major version in the path from v2
on. Both happen in one pass —
go run github.com/go-admin-team/go-admin-core/tools/coreupgrade@v2.0.0 -w -v2 .
go mod tidy
— which is the command the release notes give, run here as a consumer
would run it. 210 imports across 95 files.
The compatibility shims this used are gone in v2, so the paths that
moved had to move: sdk/pkg/captcha, sdk/pkg/jwtauth and its user
package, sdk/pkg/response and sdk/pkg/casbin.
The count of unformatted files is unchanged at 34, none of them touched
by this: the tool reformats a file only if it was already gofmt clean,
so a migration cannot disappear into whitespace.
The pinned core dated from April, before sdk stopped being a separate module,
so the build resolved sdk packages from the old module and core packages from
the new one. Dropping the separate requirement is what makes the two agree
again.
Most of the diff is renames that came with that: the tenant accessors gained a
ByTenant suffix, GetDb now returns one database and GetAllDb the map, and
casbin moved to v3.
The change that matters is four call sites moving from GetMemoryQueue to
GetQueuePrefix. GetMemoryQueue returns a queue fixed at construction, so the
login log, the operate log and the api check ran in process no matter what the
settings file selected — a second instance saw none of it. GetQueuePrefix
returns whatever the configuration built, which is the point of being able to
configure a queue at all.
Verified against core at main: build and vet clean. The two file_store failures
are unchanged from before this branch; they need cloud credentials.
zhangwenjian