Fix critical bugs from Todo App dogfood - #159
Merged
Merged
Conversation
Two agents built the same todo app — one from the starter, one from
scratch. Their journals and retrospective surfaced bugs, DX gaps, and
architectural questions. This commit translates every finding into
actionable checklists:
1. Critical bug fixes (make:key, parsed body, symfony/filesystem,
ExceptionHandler, migrate timestamps, migrations path)
2. Auth config reform (IdentityProvider interface, no closures)
3. CommandResult & flash messages (redirect support, transport-agnostic
command outcomes, StringStream)
4. Auth exceptions & redirect (transport-agnostic exceptions,
AuthRedirectMiddleware, DTO-specific error templates)
5. Routing & handler resolution (path doubling, class_exists fix,
catch narrowing, root path documentation)
6. Bootstrap self-wiring (container constructor, kernel bootstrap)
7. Investigation fixes (CSRF JS htmx v4, env(), doc fixes)
8. Unify CLI dispatch & colon routing (DTO+Handler for all 19
built-in commands, retire BuiltInCommand, colon naming convention)
9. Starter app update (absorb all framework changes)
10. Documentation (from-scratch guide, config schemas, login recipe,
testing guide, domain boundaries)
Also adds deferred items to Long-Distance Future: final class testing,
cross-domain model access, PdoConnection PDO instance, template cache
dev mode, {{ end }} closer, database seeding, Forge dynamic WHERE.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RuneKernel now runs bootstrap in two phases. Phase 1 always runs the early bootstrappers (Hourglass, Environment, Configuration) — the minimum needed to read config and determine the command. Phase 2 checks config/bootstrap.php under the 'cli' key for per-command bootstrapper overrides. Framework defaults: make:key, list, and help map to empty arrays so they skip Security, Database, Auth, etc. This fixes the chicken-and-egg where make:key couldn't run because Bootstrap\Security throws on missing APP_KEY. App config merges over framework defaults. Early bootstrappers are deduplicated if listed in a per-command override. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The per-command defaults for make:key, list, and help need CliRouting to register the BuiltInRegistry and command services. CliRouting is safe under minimal bootstrap — it only needs Configuration (already ran) and all heavy command factories are lazy. Defaults: [CliRouting] instead of []. Heavy bootstrappers (Security, Database, Auth, Cache, Logger) remain skipped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three scenarios: make:key prints a valid key with no .env, --write flag persists the key to .env, and a normal command (migrate) still fails on missing APP_KEY via the full bootstrap chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Server::request() now parses the body for non-GET/non-POST methods when Content-Type contains application/x-www-form-urlencoded. POST continues using $_POST per PSR-7 spec. GET and methods with other content types return null parsed body. Fixes the dogfood issue where PUT/PATCH/DELETE form submissions silently lost their data, forcing hx-post workarounds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Parchment uses Symfony\Component\Filesystem in Reader, Writer, TempFile, and FileSystem, but composer.json only required symfony/finder. Works in dev via transitive deps; fails on a clean install. Also runs a full composer update to sync the lock file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bootstrap\Exceptions had two bugs: (1) the handle*() methods used $container->get() which throws on missing services, making the null checks dead code — replaced with has() + get(). (2) handleShutdown() had no try-catch — shutdown handlers must never throw. Also registers Glitch\Handler as the default ExceptionHandler, ErrorHandler, and ShutdownHandler with has() guards. From-scratch users get real error handling out of the box; apps override by registering their own implementation before bootstrap. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch from 14-digit second-resolution timestamps (YmdHis) to 17-digit millisecond-resolution (YmdHisv) for migration version identifiers. Two migrations created in the same second no longer collide on the version primary key. Updated MigrateCreateCommand, MigrationParser regex, MigrationRepository schema (VARCHAR(14) → VARCHAR(17) for MySQL/PostgreSQL), all test fixtures, and test assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Migration commands now default to database/migrations/ instead of migrations/. The path is configurable via the database.migrations_path key in config/database.php — the value is relative to the project root. When not configured, the default applies. All four migration commands (migrate, migrate:rollback, migrate:status, migrate:create) accept a $migrationsPath constructor param, wired by Bootstrap\CliRouting from the config. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Six critical bugs surfaced by the Todo App dogfood builds, all fixed:
make:keyno longer requiresAPP_KEYto exist.RuneKernelnow supports configurable per-command bootstrap lists viaconfig/bootstrap.php, with built-in defaults formake:key,list, andhelpServer::request()now parsesapplication/x-www-form-urlencodedbodies for PUT, PATCH, and DELETE requests (previously only POST via$_POST)symfony/filesystemdependency — Added explicit require (was only available transitively)ExceptionHandlernot registered — Fixedhas()guards inBootstrap\Exceptionsand registeredGlitch\Handleras the default handler so from-scratch users get error handling out of the boxYmdHisv)database/migrations/, configurable viadatabase.migrations_pathin configTest plan
composer checkpasses (cs-fix, cs-check, phpstan, phpunit — 2828 tests)Server::request()body parsing (7 tests)RuneKernelper-command bootstrap (7 tests)make:keyworks withoutAPP_KEYExceptionHandlerbootstrap tests🤖 Generated with Claude Code