# Framework Lessons — AMIR > **What goes here:** lessons from this project that would apply to the *next* project bootstrapped with the Parsec AI Development Framework. If the lesson is AMIR-specific (e.g. an accounting domain rule), it goes in `DEVIATION_LOG.md` or `DOMAIN_GUIDE.md` instead. --- ## Sprint S01 — 2026-05-07 ### S01-L1. Package version in bootstrap scaffold must match documentation The bootstrap scaffold installed `spatie/laravel-activitylog ^5.0` but `CLAUDE.md`, the domain guide, and agent files all referenced v4. The wrong import path (`Spatie\Activitylog\Traits\LogsActivity`) caused PHPStan failures during the first sprint's pipeline run. **Rule:** After `composer install` in a new project, run `composer info ` for every package referenced in CLAUDE.md and verify the version matches. Update all doc references before the first sprint. ### S01-L2. Middleware that reads session must guard `hasSession()` from Day 1 When a project has both web (session) and API (stateless) routes, any middleware that calls `$request->session()` will crash on API requests with `RuntimeException: Session store not set on request`. This was caught only when the first `routes/api.php` was created in Sprint 1. **Rule:** In bootstrap scaffold, add `hasSession()` guard to `SetTenantContext` (or equivalent context-resolution middleware) before the project has API routes. The cost is zero; the risk of missing it is a blocked sprint. ### S01-L3. Full test suite invocation path must be confirmed at bootstrap `php artisan test` and `php vendor/bin/pest` both exited 255 with zero output when run without a path argument. Tests only ran correctly when given an explicit directory path (`pest tests/Feature/`). The cause appears to be the Browser test suite (Playwright) crashing the runner in the project's CI wrapper. This was not discovered until sprint-close. **Rule:** At project bootstrap, confirm `php artisan test` produces output for all non-browser tests. If not, document the correct invocation command in `CLAUDE.md` §10 (Self-Verification Protocol). Never assume the default invocation works without testing it. ### S01-L4. PHPStan memory limit must be raised in project config PHPStan's default 128M memory limit caused OOM crashes on this project (Laravel 12, ~60 domain files). The workaround `--memory-limit=512M` must be passed manually. This should be set in `phpstan.neon` or a Composer script alias to avoid agent failures. **Rule:** In bootstrap scaffold, add `memory_limit: 512M` to `phpstan.neon` (or project equivalent) so agents can run static analysis without the flag. A crashed analysis gate is worse than a slow one. --- ## Sprint S02 — 2026-05-07 ### S02-L1. Domain event listeners require manual registration when domains are structured as sub-directories When domain code lives in `app/Domain/*/Listeners/` (a common DDD structure), Laravel's auto-discovery does not scan these paths — it only scans `app/Listeners/`. Agents spent a debugging session before discovering the listener wasn't firing. **Rule:** In projects using domain-folder structure (DDD), always document in the bootstrap CLAUDE.md that domain listeners must be registered manually in a ServiceProvider. Consider evaluating `EventServiceProvider::$listen` array or a dedicated DomainServiceProvider pattern at project start so agents don't rediscover this every sprint. ### S02-L2. PostgreSQL type-strict columns reject non-typed placeholder values in queries When using `whereNotIn('uuid_column', ['placeholder-string'])`, PostgreSQL will throw `SQLSTATE[22P02]: invalid_text_representation` because UUID columns validate all values passed to them — including those in NOT IN lists. This is a PostgreSQL-specific behaviour; SQLite (common in test environments) is permissive. **Rule:** In projects using PostgreSQL as the test database (correct choice for production parity), never use placeholder/sentinel strings as values for typed columns. Use conditional early-return on empty arrays instead. Add this pattern to the project's testing-standards guide at bootstrap. ### S02-L3. Immutable audit tables require DB superuser for maintenance operations Enforcing audit log immutability via PostgreSQL rules (`DO INSTEAD NOTHING`) creates a production maintenance gap: the DB user running application queries (and Artisan commands) must have SUPERUSER privilege to bypass the rule via `session_replication_role = replica`. Standard application DB users typically do not have SUPERUSER. **Rule:** If a project uses PostgreSQL rules for audit immutability, document at bootstrap that a separate high-privilege maintenance DB user is required for pruning jobs. Alternatively, use row-level security (RLS) or application-level write-once enforcement that doesn't require superuser to bypass. Resolve before production deployment. ### S02-L4. UI browser verification must be explicitly scheduled or marked as deferred Multiple S02 tasks implemented UI screens but could not verify them in a browser (no dev server running during agent execution). This was logged as medium-risk in each assumption, but there's no systematic gate preventing UI ships without visual verification. **Rule:** Add a `/verify` step 1b: "If UI changes were made, start the dev server and verify in a browser before opening the PR." Mark it explicitly as SKIPPED if skipped, with a reason. Silent skips become tech debt. Consider making the verification step a checklist item in PR descriptions. --- ## Sprint S03 — 2026-05-07 ### S03-L1. `Model::fresh()` bypasses global scopes in Laravel 12 — use `Model::find()` for visibility tests An agent spent significant debugging time (added DB query logging, inspected vendor code) to discover that `Model::fresh()` in Laravel 12 uses `newQueryWithoutScopes()` internally. This means soft-delete and tenant scope checks via `$model->fresh()` silently return the record regardless of scope state. **Rule:** In the project's testing-standards.md, add an explicit note: "For soft-delete and tenant-scope visibility assertions, always use `Model::find($id)` (applies all global scopes) not `$model->fresh()` (applies no scopes). This applies to any Laravel 12 project." ### S03-L2. Upstream action/enum branches must merge to dev before dependent task branches start S03-001 created `PeriodState::Reopened` and `ReopenPeriod` action. Three downstream branches (S03-008, S03-011, PostTransaction extension in S03-001) needed to cherry-pick S03-001's commit because S03-001 had not yet merged to dev. This cherry-pick pattern adds merge conflict risk and was repeated 3 times. **Rule:** When generating sprint task prompts, identify tasks that create shared types/enums/actions that other tasks in the same sprint depend on. Mark those tasks as "must merge to dev first" in the dispatch.sh batch config and in dependent tasks' prompts. A simple `## Prerequisites` section in each prompt ("Branch S0N-XXX must be merged to dev before starting this task") prevents the cherry-pick pattern. ### S03-L3. Frontend file commits should be verified with `git ls-files` on macOS `ReopenPeriod.tsx` was listed in the S03-002 commit message but absent from the git tree. This was only caught at sprint-close when the test suite ran. The likely cause is macOS's case-insensitive HFS+ filesystem where `Pages/` and `pages/` are the same directory. **Rule:** After every commit that includes frontend files (`.tsx`, `.ts`), run `git ls-files resources/js/ | grep ` to confirm the file is tracked. Add this as a verification step in the frontend skill file and PR checklist. ## Sprint S04 — 2026-05-08 ### S04-L1. gh pr create must always specify --base explicitly Sprint S04 had a PR merge to `main` instead of `dev` because `gh pr create` defaulted to the repo's default branch. This required a `git merge origin/main` and conflict resolution on dev, plus a second PR to restore clean state. **Rule:** Every `gh pr create` call in agent prompts and skill files must include `--base dev`. Never rely on the repository default. Add a note to `AGENT_GUIDE.md` under "PR rules": "Always use `--base dev`. The repo default branch is `main`; GitHub will accept PRs against it silently." ### S04-L2. Auth test passwords must be ≥ 8 characters to pass validation Laravel's `AuthenticateRequest` includes `password: ['required', 'string', 'min:8']`. Test passwords shorter than 8 characters (e.g. `'Wrong!'` = 6 chars) fail FormRequest validation silently — the controller action is never reached and assertions about authentication side-effects (failed_attempts counter increment) fail unexpectedly. **Rule:** All auth test fixtures must use passwords ≥ 8 characters. Document in `testing-standards.md` under "Auth test conventions": "Wrong-password tests must use a string ≥ 8 chars that passes FormRequest min:8 validation but does not match the user's stored hash (e.g. `'WrongPassword!'`)." ### S04-L3. Sentry SDK v4 Event::getRequest() returns a plain PHP array When implementing Sentry `before_send` callbacks, `$event->getRequest()` returns `array` not an object. Calling methods on it (e.g. `->getData()`) throws a TypeError. This wasted debugging time and required rewriting the scrub method. **Rule:** Document in the Sentry service class and in the domain guide: `Event::getRequest()` returns a plain PHP array. Use `$request['data']` (array access) and `$event->setRequest($array)` (not `->setData()`). Add this to the Sentry-specific section of `DOMAIN_GUIDE.md` Operations domain. ### S04-L4. current_sprint counter must be advanced at every sprint-close Sprint S04 ran sprint-close after only 7 of 12 tasks were done, because `/sprint-status` returned `"done: 12"` for sprint 2 (current_sprint = 2) not sprint 4. The sprint counter had not been advanced after S02 or S03 sprints. **Rule:** The `/sprint-close` gate must explicitly check that all `sprint == N` tasks are `done` where `N` is the sprint being closed — not `current_sprint`. Add to the sprint-close gate script: "Before advancing, assert `jq '[.tasks[] | select(.sprint == N and .status != \"done\")] | length' == 0`." Also verify `current_sprint` is consistent before running sprint-close. ## Sprint S05 — 2026-05-08 ### S05-L1. Silent correctness bugs require audit-first discovery — not just test coverage Two bugs in S05 (RunDepreciationRun `in_service_date` filter, ImportBankStatement DD-MM-YYYY parsing) produced zero test failures and no visible errors. The code ran, returned success, and produced wrong financial data silently. Only an audit-first sprint discovered them. **Rule:** For every sprint that builds financial calculation or data-import logic, plan a follow-up audit sprint within 4–5 sprints. Do not assume "tests pass → correctness is guaranteed". Audit sprints should run action classes against representative real-world inputs and compare against manually verified expected outputs. ### S05-L2. Carbon-cast date columns fail string equality assertions silently Eloquent columns with `date` or `datetime` casts return Carbon objects. `expect($model->transaction_date)->toBe('2026-05-01')` fails silently because Carbon ≠ string. This cost debugging time during ImportBankStatement test writing. **Rule:** Before writing date assertions in any test, check the model's `$casts` array. If the column is cast to `date`, `datetime`, or `immutable_date`, use `->toDateString()` on the retrieved value: `expect($model->date_col->toDateString())->toBe('2026-05-01')`. Add this pattern to `testing-standards.md` under "Date assertion conventions". ### S05-L3. Audit-logged actions have higher baseline query counts — budget accordingly `PostInvoice` with activitylog v5 wiring reaches ~31 queries for a minimal single-line invoice. Setting a query budget below this causes the budget assertion to fail immediately, forcing a budget increase. Initial budget of ≤30 was too tight. **Rule:** For action classes that trigger `LogsActivity` on multiple models (transaction + lines + invoice), set the query budget at ≥35. For simpler actions with 1–2 audited models, ≥25 is appropriate. Document the expected query breakdown in a test comment when the budget exceeds the standard ≤25 default. --- ## Sprint S06 — 2026-05-08 ### S06-L1. Junction tables with UUID PKs cannot use Eloquent attach() — use raw DB::table()->insert() `tenant_user_memberships` has a `uuid('id')->primary()` column — a UUID PK, not auto-increment. `$user->tenants()->attach($tenant->id)` does not populate the `id` column, causing a NOT NULL violation. This blocked test setup and required discovery via database error logs. **Rule:** When a junction/pivot table uses a UUID primary key (not auto-increment), always use `DB::table('table_name')->insert(['id' => (string) Str::uuid(), ...])` in tests and seeders. Document at bootstrap any pivot table that uses a UUID PK — `attach()` silently breaks without a clear error. ### S06-L2. PHPStan's LazyCollection template is not covariant — avoid method return type annotations on mapped collections Declaring `@return LazyCollection>` on a method that returns the result of `.map(fn(Model $m): array {...})` causes PHPStan to reject the assignment. Multiple alternative annotations (`mixed`, `array`) were tried and all failed. The root issue is that `LazyCollection` is covariant in neither type parameter in PHPStan's analysis. **Rule:** For generator/export classes that build a `LazyCollection` via `.map()` on Eloquent model queries, remove the `@return` annotation from the method signature. Use an inline annotation on the variable before the map call: `/** @var LazyCollection $items */ $items = $query->lazy();`. PHPStan infers the map result correctly from there. ### S06-L3. PR branches must be based off origin/main (not origin/dev) when GitHub default branch is main In this project, all PRs merge to `main` (GitHub default). Feature branches created from `dev` miss any work that was already merged to `main` but not yet merged back to `dev`. This caused a DemoDataSeeder conflict in S06-010: `seedMembers()` was on `main` (from S06-009 PR), but not on `dev`, so the S06-010 feature branch was missing it. **Rule:** For projects where GitHub default branch is `main` and PRs merge there: create feature branches from `origin/main` (not `origin/dev`). Use `git checkout -b feature/name origin/main`. If dev is used for sprint tracking commits, merge `origin/main` into your feature branch before starting (`git merge origin/main`). Document this in the sprint task prompt template. ### S06-L4. Task prompt ACs can have copy-paste errors — agents must identify them before coding Multiple S06 tasks had copy-pasted acceptance criteria from unrelated stories (S06-004 had MEM-TA-03 ACs instead of MEM-TA-01; S06-005 had MEM-TA-04 ACs). Agents implemented against the task title and referenced screen briefs (P-161, P-162) rather than the incorrect ACs, which was the correct behavior, but it required explicit assumption logging. **Rule:** When a task's acceptance criteria do not match the task title, log this as an assumption (type: "task prompt AC copy-paste error") and implement against the title + referenced screen brief / USER_STORIES story ID. Do not block implementation waiting for AC correction — log and proceed. Consider a sprint-start pass that verifies AC-to-title alignment across all sprint tasks before the first `/plan` call. ## Sprint S07 — 2026-05-08 ### S07-L1. SUM(credit - debit) is the correct P&L TB formula — CASE expressions invert the sign An initial implementation of `PenyataPlIntegrity::rawTbNet()` used `CASE WHEN account_type = 'income' THEN credit-debit ELSE debit-credit END`. This formula adds both income and expenses (both produce positive values), yielding `total_income + total_expenses` instead of `total_income - total_expenses`. The correct formula is simply `SUM(credit_cents - debit_cents)` for all P&L accounts — income contributes positive, expenses contribute negative. **Rule:** For raw TB P&L tie-out queries, use `SUM(credit_cents - debit_cents)` across all income and expense accounts. Never flip the sign for expense accounts — they are already negative under this formula. Tests catch this immediately if a surplus + deficit test case is included. ### S07-L2. Systematic AC copy-paste errors persist into Reporting sprint — validate all prompts before sprint start Every one of the 12 S15 task prompts had AC from an unrelated story (RPT-SYS-01, RPT-TA-02). This is the same issue as S06-L4 but more widespread (12/12 tasks affected vs 2/12 in S06). The pattern suggests the sprint backlog generator is not matching AC to task title when building Reporting sprint prompts. **Rule:** Before running the first `/plan` of any sprint, run a quick AC validation pass: for each task, confirm the AC stories match the task title keyword (e.g., "BsGenerator" task should reference BS or report generation stories). Flag mismatches to the founder before starting. The S06-L4 rule already said "log and proceed" — add to it: "block and surface to founder if >50% of tasks in a sprint have mismatched ACs." ## Sprint 8 (S16) — 2026-05-08 ### S08-L1. BelongsToTenant scope produces 404, not 403, on cross-tenant route model binding — tests must use assertNotFound() Tests that check cross-tenant access on tenant-scoped route model binding were written with `assertForbidden()` (403). The actual response is 404 because `BelongsToTenant` global scope filters the model out of the query before the controller runs — route model binding returns "not found", not "forbidden". This is the correct security behavior but the test expectation is consistently wrong on first write. **Rule:** For any route that uses route model binding on a `BelongsToTenant` model, cross-tenant isolation tests MUST use `assertNotFound()` not `assertForbidden()`. Document this in the testing-standards.md skill. Add a reminder comment in the test template: "// cross-tenant: BelongsToTenant scope → 404 (not 403)". ### S08-L2. Config task template leaked into implementation task prompts — verification step said "restart services" for a GL computation task S17-001 and S17-002 (Note auto-population tasks) had the wrong verification step: "Restart the relevant services (queue, scheduler)" — copied from a config/infrastructure task template. This would cause an agent to verify by restarting services instead of writing tests. **Rule:** Validate the `## Verification Step` section of every task prompt against the task's `Type` field before the sprint starts. `type: config` → restart services; `type: default` or `type: feature` → write Pest tests. A mismatch is a template copy-paste error — fix before the agent runs `/plan`. ### S08-L3. Downstream sprints need explicit references to infrastructure created by the current sprint S17 note-population tasks were generated without knowing that S16 would create `PenyataNote`, `NotesFramework`, and `UpdateNoteContent`. Without explicit references, an S17 agent might recreate these classes rather than extending them. **Rule:** When closing a sprint that creates foundational infrastructure (models, pure classes, actions), update the next sprint's task prompts to reference the new classes by name. Add a "## S[N] Infrastructure (already built — do NOT recreate)" section. The `/sprint-close` prompt quality pass is the right time to do this. ## Sprint S17 — 2026-05-08 ### S17-L1. Stale cross-sprint test file is a merge-order dependency S17-012 (end-to-end test) references 11 classes from S17-001..S17-011 that live on separate feature branches. The test cannot run on dev until all those branches merge. PHPStan didn't catch this because tests are excluded from its scope. **Rule:** When a sprint's final test task must reference classes from earlier tasks in the same sprint, document the merge order constraint explicitly in both the PR description and DEVIATION_LOG. Gate the test PR's merge on all dependency PRs being merged first. ### S17-L2. Auto-skip generators need both paths tested Every note generator that conditionally returns null (auto-skip) requires tests for both paths: one test that confirms the note IS created when data exists, and one that confirms null IS returned when it doesn't. Several S17 tests only tested one path initially. **Rule:** For any generator method that returns `PenyataNote|null`, the test file must contain at least one `toBeInstanceOf(PenyataNote::class)` assertion AND at least one `toBeNull()` assertion. Treat this as a test completeness checklist item in the `/verify` protocol. --- ## Sprint S18 — 2026-05-08 ### S18-L1. Larastan 3.9.x does not infer enum/date types from the `casts()` method form The `casts()` method form on Eloquent models (as opposed to the `$casts` property array) does not generate PHPDoc property stubs that Larastan can use to infer enum or date types. Every access to `$model->enum_column->value` or `$model->date_column->format()` triggers a false positive ("Cannot access property $value on string" / "Cannot call method format() on string"). This cost approximately 2–3 PHPStan iteration cycles per affected file in S18. **Rule:** At project bootstrap, add a note to `CLAUDE.md` §5 (Data Conventions): "For Larastan compatibility, use the `$casts = [...]` property array form (not the `casts()` method form) in all Eloquent models." If a model already uses `casts()`, add `@property EnumType $column` PHPDoc annotations for all enum/date columns. This is a one-time fix per model, not a recurring workaround. ### S18-L2. `Storage::fake('s3')` must be documented as a prerequisite for any file-storage action test DomPDF binary output requires S3 storage. Without `Storage::fake('s3')` at test setup, the test environment tries to connect to a live S3 bucket and fails with a connection error. This is easy to miss on first write of a new test type (binary storage). **Rule:** In the project's `testing-standards.md`, add a section "Storage fakes" with the rule: "Any test that invokes an action which writes to S3 (or any storage disk) must call `Storage::fake('disk_name')` at the top of the test method or in `beforeEach`. Do not stub the action class — the storage write is the behavior being tested." ### S18-L3. macOS case-insensitive filesystem silently loses tracked TSX files Three component files (BsUi.tsx, PlUi.tsx — tracked in a previous sprint) were lost when a case-sensitivity cleanup on macOS renamed the `Pages/` directory to `pages/`. Git on macOS case-insensitive HFS+ silently deduplicated the directory rename and removed the old-case files from the tree. The files appeared tracked in git log but were absent from the working tree. **Rule:** After any file operation that involves case changes to a directory name: (1) immediately run `git ls-files resources/js/ | sort` and diff against the expected file list; (2) run `npm run build` to confirm all components resolve; (3) if any file is missing, force-add with `git add -f path/to/File.tsx`. Add this as a checklist item in the frontend skill file and in the `/verify` protocol step 6 (frontend build). --- ## Sprint S21 — 2026-05-08 Sprint S21: no framework-level lessons identified. The P-25 Larastan workaround (JSONB cast column variant documented as P-27) is a project-specific instance of the S18-L1 rule and does not introduce new framework guidance beyond what S18-L1 already states. The auditor role gap (Permissions::seed() vs. RolesSeeder) is AMIR-specific and documented in DEVIATION_LOG.md. --- ## Sprint S25 — 2026-05-08 ### S25-L1. Ship the FakeClient in the same PR as the interface for external API integrations When building a domain that wraps an external HTTP API, the fake client is as important as the interface itself. In S25, `FakeMyInvoisClient` (shipped in the same PR as `MyInvoisClientInterface`) immediately unblocked all 6 subsequent tasks by allowing them to test against the fake without any mocking setup. Projects that defer fake clients force agents to mock interfaces with PHPUnit/Mockery, which is both slower to write and harder to read. **Rule:** In the framework's external-integration skill (`writing-myinvois-integration.md` or similar), mandate: "When creating a new integration interface, ship `Fake[Client]` in the same PR. The fake must: (1) implement the interface, (2) return realistic stubbed responses by default, (3) expose fluent state mutators for failure scenarios (e.g. `rejectSubmission(): static`). Bind in tests via `app()->bind(Interface::class, fn () => new FakeClient)`." ### S25-L2. Compliance migrations as documentation: `COMMENT ON TABLE` is a valid migration technique The v4.6 spec-check migration (S25-013) runs only `COMMENT ON TABLE einv_credentials IS '...'` — zero DDL, pure documentation that appears in the migration log. This creates a timestamped, version-controlled record of compliance baseline that auditors and future agents can find via `\d einv_credentials` in psql. **Rule:** When a regulatory or compliance requirement must be tracked but requires no schema change, add a documentation-only migration that writes a PostgreSQL table comment. The migration file header comment explains the requirement; the body records the baseline. Add this pattern to the project's `writing-migrations.md` skill file. --- ## Sprint S26 — 2026-05-08 ### S26-L1. Extract `*Interface` for any `final class` that will be injected into another class S26-011 (BulkSubmit) tried to use Mockery to mock `SubmitInvoiceToMyInvois` (a `final class`) and immediately hit `Cannot mock final class`. The fix — extracting `SingleInvoiceSubmitterInterface` — was straightforward, but it cost a full rework cycle. Any time a `final` action class is injected as a collaborator in another class, its interface must be extracted in the same PR that creates the injection. **Rule:** In `writing-tests.md` and `writing-pr-descriptions.md`, add a verification checklist item: "For every constructor-injected `final class`, confirm a `*Interface` exists in `app/Domain/[Name]/Contracts/`. If missing, extract it before opening the PR." ### S26-L2. Squash merges overwrite sprint backlog task-done commits When the autonomous pipeline calls `/task-done` (which commits `.sprint-backlog.json` to `dev`), subsequent squash merges from feature branches that contain an older version of `.sprint-backlog.json` overwrite the task-done entries on `dev`. Sprint-close found 7 tasks marked incorrectly. **Rule:** When resolving a `.sprint-backlog.json` merge conflict during a squash merge, always prefer the `dev` version (`git checkout --theirs .sprint-backlog.json` when on the feature branch). The sprint-close gate must re-validate all task statuses against the actual merged PRs before advancing the sprint counter. ### S26-L3. Cross-cutting E2E orchestrators do not belong inside a domain folder The `EinvProdE2e` class exercises SubmitInvoiceToMyInvois (S25), StatusUpdate (S26), and CancelFlow (S26). Placing it in `app/Domain/EInvoice/` would create false coupling — it is not a domain action, it is a cross-cutting integration harness. It was placed in `app/` (root namespace). **Rule:** When an "action" orchestrates multiple domain actions in sequence (for testing or integration purposes), use `app/` root namespace, not `app/Domain/[Name]/`. Document the placement in CODEBASE_MAP.md with a "cross-cutting" note. If this pattern recurs more than twice, introduce `app/Orchestrators/`. ## Sprint 14 (S30) — 2026-05-08 ### S30-L1. Schedule::job() is incompatible with ->runInBackground() `Schedule::job(MyJob::class)` creates a `CallbackEvent` in Laravel 12, not a standard `Job` event. Calling `->runInBackground()` on it throws "Scheduled closures can not be run in the background" during app bootstrap — which silently breaks the entire test suite (every test fails). The agent added `->runInBackground()` by analogy with documentation that applies to `Schedule::command()`, not `Schedule::job()`. **Rule:** Never add `->runInBackground()` after `Schedule::job()`. Add this to the framework's task prompt template for any sprint task involving scheduled jobs: "If using `Schedule::job()`, do NOT add `->runInBackground()` — the job dispatches to the queue and runs async by design." ### S30-L2. gh pr create defaults to the repo default branch (main), not dev When `gh pr create` is called without `--base dev`, it targets the repository's default branch (typically `main`). In AMIR, `main` is the production branch — PRs that land there bypass the sprint gate and can go directly to production. The agent forgot `--base dev` on the very first PR of the sprint. **Rule:** Every project using this framework must include `--base dev` in EVERY `gh pr create` call in the `writing-pr-descriptions.md` skill and the commit-push-pr command templates. Add a check in `pre-commit-guard.sh` (or the PR creation step) that verifies the base branch before creating. ### S30-L3. Centralise multi-tenant bypass — never scatter withoutGlobalScope In projects with a `TenantScope` global scope, ad-hoc `withoutGlobalScope(TenantScope::class)` calls in individual actions create an unaudited, untestable bypass surface. The correct pattern is a single class (`CrossTenantQuery`) that enforces the permission gate, restores context in `finally`, and audit-logs every invocation. **Rule:** For any project with a global tenant scope, establish a `CrossTenantQuery` (or equivalent) class in the first sprint that needs PO/admin cross-tenant access. Mark direct `withoutGlobalScope()` calls as a DO NOT violation in CLAUDE.md from that sprint onward. ## Sprint 13 — 2026-05-08 ### 13-L1. Domain events decouple cross-domain side effects In Sprint 13, cancelling an invoice required a journal reversal in the Accounting domain. The naive approach (call Accounting action directly from EInvoice) creates tight coupling. The event-driven approach (dispatch `InvoiceCancelled` → Accounting listener handles reversal) keeps each domain ignorant of the other's internals. **Rule:** When domain A's state change must trigger side effects in domain B, always use a domain event + listener. Never import domain B's action classes into domain A's actions. Add this as a mandatory check in the `reviewing-code.md` skill. ### 13-L2. Test at action level when dependent HTTP routes are in unmerged PRs Sprint 13's E2E test couldn't use `route('einvoice.cancel')` because that route was in an open PR. Rather than skipping the cancel assertion or waiting for the merge, the test called `app(CancelFlow::class)->execute()` directly — proving the action works without a route dependency. **Rule:** When an E2E test depends on a route that is not yet merged to dev, test the action/service directly and document the assumption in SPRINT_ASSUMPTIONS.md. This is preferable to either deferring the test or creating an unstable test that passes only after the other PR merges. ## Sprint 14 — 2026-05-08 Sprint 14 [date]: no framework-level lessons identified. Sprint ran ahead of counter (recalibration scenario). ## Sprint 15 — 2026-05-08 Sprint 15 [date]: no framework-level lessons identified. ## Sprint 16 — 2026-05-09 ### 16-L1. AC text is the full user story — files_touched is the actual deliverable scope Sprint 16 (S34) had 11 tasks where the AC text in each prompt described the full ONB-TA-xx user story (filtering, export, PDPA redaction, CSV, etc.) rather than the single-task deliverable. Every task was bounded to `files_touched`, which was the correct interpretation, but the disconnect caused friction in every task's Assumptions section. **Rule:** When writing sprint task prompts, the AC field should describe only what the single task delivers. If the AC is the full user story (inevitable when generated from user story definitions), add a `## Scope Boundary` section explicitly stating: "Implement only files_touched scope; document everything else in PR Assumptions." ### 16-L2. Foundational schema PRs should be merged before dependent feature tasks begin S34-005 (branches table) had 7 downstream tasks (S34-006 through S34-012) that all needed the Branch model. Each task had to copy Branch.php from the unmerged feature branch. This is fine for a solo workflow but creates drift risk if the foundation PR changes. **Rule:** In the sprint plan, mark schema/model tasks as hard prerequisites. In the sprint workflow, merge foundational PRs to dev before starting tasks that list them in `deps[]`. For parallel workflows (L4/L5), group dependent tasks sequentially on the same worktree. ### 16-L3. PHP memory_limit needs to be raised for full-suite runs PHPStan and Pest both hit the default 128M memory limit when running the full codebase (723 tests, ~300 PHP files). Neither tool gracefully degrades — they crash mid-run. **Rule:** Set `memory_limit = 512M` in the project's `php.ini` or add `--memory-limit=512M` to phpstan.neon and set `PEST_MEMORY_LIMIT=512M` in the `.env.testing` file so sprint-close runs don't require manual flag overrides. ## Sprint 17 — 2026-05-09 ### 17-L1. `Log::fake()` API removed in Laravel 13 / Monolog 3 Sprint 17 (S38-007) attempted to test a cron job that emits `Log::info()` using `Log::fake()`. The test threw "Call to undefined method Monolog\Logger::fake()" — this API was removed when Monolog 3 became the default. **Rule:** Do not use `Log::fake()` in any test. Test logging behaviour via observable side effects: `Bus::fake()` for dispatch assertions, DB state changes, or exception-absence (`expect(fn() => $job->handle())->not->toThrow()`). If you need to assert a specific log message, register a custom Monolog handler. ### 17-L2. Domain directories must be named in PascalCase from creation Sprint 17 (S38-001) was delayed ~2h51m because the PDPA domain directory was created as `PDPA/` (all-caps) instead of `Pdpa/`. macOS case-insensitive FS caused `PDPA/` and `Pdpa/` to resolve to the same path on disk but be distinct in the git index. A two-step rename via temp name was required. **Rule:** When scaffolding a new domain directory, always use PascalCase (`Pdpa`, not `PDPA`). The bootstrap script and any domain scaffold commands should enforce this. Before committing a new `app/Domain/*/` directory, verify: `ls app/Domain/ | grep -i pdpa` returns exactly one match in the expected PascalCase form. ## Sprint 19 — 2026-05-09 ### 19-L1. `uuidMorphs()` must be documented as a mandatory pattern for UUID-PK notifiables Sprint 19 (S19-06) created a `notifications` table using `morphs('notifiable')`. This generated a BIGINT `notifiable_id` column that was incompatible with UUID v7 User PKs. The error only surfaced at test runtime — no migration error. Required a rollback and re-run. **Rule:** Any framework scaffold that generates a notifications/polymorphic table must default to `uuidMorphs()`, not `morphs()`. Document in project templates and the `writing-migrations.md` skill. ### 19-L2. `fresh()` should be called after `Model::create()` before returning to Resource/API layer Sprint 19 (S19-07) discovered that `SupportTicket::create([...])` returned an instance where enum casts on DB-defaulted columns were not hydrated. `SupportTicketResource` accessed `$this->severity->value` and got `null`. **Rule:** Actions that return a newly created model to an API Resource layer should always call `->fresh()`. This applies to any model with enum-cast columns that have DB defaults. ### 19-L3. Role seeding boilerplate must be documented for every test file that uses Spatie roles Sprint 19 (S19-07) tests failed with `RoleDoesNotExist` on first run because `RefreshDatabase` wipes roles. This is the same class of error seen in prior sprints. **Rule:** The `writing-tests.md` skill and agent test prompts should include the standard boilerplate: "If the test uses `assignRole()`, add `beforeEach(fn() => $this->seed(RolesSeeder::class))` at the top of the test file." ## Sprint 18 — 2026-05-09 ### 18-L1. Observability middleware must catch all Throwable — never let SLI/analytics writes break requests A `RecordHttpRequest` middleware that writes to `http_request_log` was propagating DB exceptions as 500 responses when the Postgres transaction was already aborted by an earlier error in the pipeline. The observability write is a side effect — it must never become the primary failure. **Rule:** Any middleware, job, or listener that writes to an observability store (SLI log, analytics events, audit trail) must wrap its DB write in `try-catch (Throwable $e) { Log::warning(...); }` and never rethrow. ### 18-L2. ULID strings are not valid PostgreSQL UUID values — document the boundary explicitly `Str::ulid()` returns a 26-char base32 string. PostgreSQL UUID columns reject it. The failure cascades as 25P02 errors on all subsequent queries in the same transaction, masking the original error. This was discovered late in the sprint during tests. **Rule:** The `data-conventions.md` guideline must state: "UUID columns (`uuid` type) require RFC 4122 format. Use `Str::uuid()` (PHP) or `crypto.randomUUID()` (TypeScript). ULID is only valid in VARCHAR/TEXT columns." Add this as a prohibited pattern in the testing-standards skill. ### 18-L3. 25P02 cascade debugging: search for the first SQL error, not the last When tests show `SQLSTATE[25P02]: current transaction is aborted`, the reported failing statement is almost always a downstream victim. The root cause is an earlier SQL error that aborted the transaction block. Agents repeatedly tried to fix the 25P02-reporting line instead of searching upstream. **Rule:** Add to `writing-tests.md`: "When debugging a 25P02 error, search the full stack trace for the earliest `SQLSTATE[22xxx]` or `SQLSTATE[23xxx]` error in the same request. That is the root cause. The 25P02 lines are consequences." ### 18-L4. jobs table `created_at` is a Unix integer, not TIMESTAMPTZ The Laravel jobs queue table stores `created_at` as a Unix epoch integer. Comparing it to a datetime string with `WHERE created_at >= '2026-05-09 07:00:00'` fails with `invalid input syntax for type integer`. This is counter to the project's `timestampsTz()` convention on all domain tables. **Rule:** When querying the jobs table, always use `->where('created_at', '>=', now()->subHour()->timestamp)`. Add this note to `writing-journal-entries.md` and any queue-related task prompts.