From de61afef7460d0dfd66a03257f7ee8d199be521d Mon Sep 17 00:00:00 2001 From: Julien Goux Date: Mon, 21 Sep 2026 21:26:49 +0200 Subject: [PATCH 1/2] docs: capture rewrite and independent review practices --- AGENTS.md | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e4e8dad459..c1e21674dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,9 @@ The skill is authoritative for Effect coding practices when repository instructi Effect linting uses oxlint via `.oxlintrc.effect.json`; run `pnpm lint:effect:check` or `pnpm lint:effect:fix` from the repository root. +Prefer native Effect primitives over custom coordination and lifecycle machinery. An Effect +migration should remove redundant mechanisms, not just wrap them in Effects. + ## Commands, validation, and workflows Package scripts are the source of truth for leaf workspaces; root-owned Turbo coordinates build, @@ -72,6 +75,21 @@ pnpm exec turbo run supabase#build pnpm run test:live ``` +## Independent review + +For substantial redesigns, use an available independent reviewer at the design stage, after a +meaningful implementation boundary, and for the final code and tests. Follow the user's choice +of reviewer and its authorization requirements; an authorized review workflow needs no repeated +permission. Review the relevant changes and contracts, not the unchanged repository each round. + +Usually use two or three focused rounds per checkpoint, stopping early at consensus. Consensus +means every material finding is fixed and verified, refuted with evidence, or accepted as a +tradeoff within the agreed scope, with the reviewer having assessed those resolutions and raised +no remaining material objection. If review does not converge, report the remaining disagreement +and continue work that does not depend on it; do not claim consensus or force agreement. Honor an +explicit request for further rounds. Report unavailable reviews; reviewer agreement never replaces +execution evidence. Judge scope by relevance to the agreed task, not the age of the affected line. + ## Comments Comments exist for the next reader, not as the author's audit trail. Code states what happens; a @@ -113,15 +131,25 @@ internal context in Linear. ## Refactoring -Internal unreleased APIs may be simplified or reshaped; move responsibility to the correct owner -and delete obsolete helpers, shims, and parallel paths instead of preserving compatibility -scaffolding. Protect shipped interfaces and valuable persistent data; update consumers, tests, and -docs when interfaces, ownership, or lifecycle changes. +Derive rewrites from accepted consumer behavior and documented invariants. Use old code as +behavioral evidence, not a required architecture. New abstractions, persisted state, recovery, and +compatibility mechanisms need a current consumer, documented contract, or demonstrated failure. +Prefer focused implementations with explicit limitations; preserve safeguards for data integrity +and resource ownership. + +Internal unreleased APIs may be simplified or reshaped. Name the scope being replaced and delete +its superseded implementation, tests, configuration, and exports. Protect shipped interfaces and +valuable persistent data. Keep lifecycle, identity, and persistence rules inside their owning +package; consumers should use public operations rather than reconstruct those rules. Validate +the owning contract before integrating consumers. Update consumers, tests, and docs, including the +architecture document or ADR that states the invariant, when interfaces, ownership, lifecycle, or +persisted state change. ## Test quality - Write focused tests that read as stories: arrange, act, assert. -- Assert behavior that matters to consumers, not implementation details. Prefer real parsers and observable outcomes over source-text or registry checks. +- Assert behavior that matters to consumers, not implementation details. Prefer real parsers and observable outcomes over source-text or registry checks. Readiness alone does not prove a service works. +- When supporting multiple runtimes or backends, exercise shared consumer scenarios through their public surfaces. - Make assertions meaningful: establish prerequisites, check specific failures, and choose matchers that express the intended contract. - Keep setup concise with small fixtures. Accept some duplication rather than introducing unnecessary test abstractions. - Remove redundant coverage. Push back on review suggestions that add assertions without protecting meaningful behavior. @@ -141,6 +169,9 @@ Keep tests flake-resistant: - Assume file-level parallelism: use unique IDs, roots, process markers, and derived resources; never disable parallelism globally. - Never release and reuse an ephemeral port or assume a released endpoint is a dead backend; own a refusal listener or inject the failure. - Require subprocess readiness and stdout/stderr diagnostics; clean up only exact owned resources. Reproduce and stress flake fixes, then repeat the green case. +- After an operation whose contract releases resources, assert that those processes and listeners + are gone, including detached owners and descendants where applicable. Name any data, port claims, + listeners, or processes the contract retains; client disposal need not stop a detached service. ## Maintaining instructions From 1da05f660e8e3e45a7a33b1e742acd95f4165f96 Mon Sep 17 00:00:00 2001 From: Julien Goux Date: Mon, 21 Sep 2026 21:39:04 +0200 Subject: [PATCH 2/2] docs(repo): generalize engineering guidance --- AGENTS.md | 48 +++++++++++++++--------------------------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c1e21674dd..7cdec80512 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,9 +38,6 @@ The skill is authoritative for Effect coding practices when repository instructi Effect linting uses oxlint via `.oxlintrc.effect.json`; run `pnpm lint:effect:check` or `pnpm lint:effect:fix` from the repository root. -Prefer native Effect primitives over custom coordination and lifecycle machinery. An Effect -migration should remove redundant mechanisms, not just wrap them in Effects. - ## Commands, validation, and workflows Package scripts are the source of truth for leaf workspaces; root-owned Turbo coordinates build, @@ -78,17 +75,12 @@ pnpm run test:live ## Independent review For substantial redesigns, use an available independent reviewer at the design stage, after a -meaningful implementation boundary, and for the final code and tests. Follow the user's choice -of reviewer and its authorization requirements; an authorized review workflow needs no repeated -permission. Review the relevant changes and contracts, not the unchanged repository each round. - -Usually use two or three focused rounds per checkpoint, stopping early at consensus. Consensus -means every material finding is fixed and verified, refuted with evidence, or accepted as a -tradeoff within the agreed scope, with the reviewer having assessed those resolutions and raised -no remaining material objection. If review does not converge, report the remaining disagreement -and continue work that does not depend on it; do not claim consensus or force agreement. Honor an -explicit request for further rounds. Report unavailable reviews; reviewer agreement never replaces -execution evidence. Judge scope by relevance to the agreed task, not the age of the affected line. +meaningful implementation boundary, and for the final code and tests. Usually run two or three +focused rounds per checkpoint and stop at consensus. The reviewer should assess how findings were +fixed and verified, refuted with evidence, or accepted within the authorized tradeoff, with no +material objections remaining; do not force agreement. Report unresolved decisions and continue +only work that does not depend on them. A user-requested further round takes precedence; +an authorized workflow needs no repeated permission, and review never replaces execution evidence. ## Comments @@ -131,25 +123,18 @@ internal context in Linear. ## Refactoring -Derive rewrites from accepted consumer behavior and documented invariants. Use old code as -behavioral evidence, not a required architecture. New abstractions, persisted state, recovery, and -compatibility mechanisms need a current consumer, documented contract, or demonstrated failure. -Prefer focused implementations with explicit limitations; preserve safeguards for data integrity -and resource ownership. - -Internal unreleased APIs may be simplified or reshaped. Name the scope being replaced and delete -its superseded implementation, tests, configuration, and exports. Protect shipped interfaces and -valuable persistent data. Keep lifecycle, identity, and persistence rules inside their owning -package; consumers should use public operations rather than reconstruct those rules. Validate -the owning contract before integrating consumers. Update consumers, tests, and docs, including the -architecture document or ADR that states the invariant, when interfaces, ownership, lifecycle, or -persisted state change. +Internal unreleased APIs may be simplified or reshaped; move responsibility to the correct owner +and delete obsolete helpers, shims, and parallel paths instead of preserving compatibility +scaffolding. Protect shipped interfaces and valuable persistent data; update consumers, tests, and +docs when interfaces, ownership, or lifecycle changes. +Base designs on consumer requirements and documented invariants rather than inherited code +structure. Justify additional complexity with a current need or documented contract; prefer explicit +limitations over speculative machinery while preserving required safeguards. ## Test quality - Write focused tests that read as stories: arrange, act, assert. -- Assert behavior that matters to consumers, not implementation details. Prefer real parsers and observable outcomes over source-text or registry checks. Readiness alone does not prove a service works. -- When supporting multiple runtimes or backends, exercise shared consumer scenarios through their public surfaces. +- Assert behavior that matters to consumers, not implementation details. Prefer real parsers and observable outcomes over source-text or registry checks. Exercise shared contracts across supported implementations. - Make assertions meaningful: establish prerequisites, check specific failures, and choose matchers that express the intended contract. - Keep setup concise with small fixtures. Accept some duplication rather than introducing unnecessary test abstractions. - Remove redundant coverage. Push back on review suggestions that add assertions without protecting meaningful behavior. @@ -168,10 +153,7 @@ Keep tests flake-resistant: - Subscribe before triggering a transition; use observable readiness/completion, never sleeps or polling delays for propagation, startup, cancellation, cleanup, or port release. Timeouts are guards; use controlled clocks or fake timers for timing semantics. - Assume file-level parallelism: use unique IDs, roots, process markers, and derived resources; never disable parallelism globally. - Never release and reuse an ephemeral port or assume a released endpoint is a dead backend; own a refusal listener or inject the failure. -- Require subprocess readiness and stdout/stderr diagnostics; clean up only exact owned resources. Reproduce and stress flake fixes, then repeat the green case. -- After an operation whose contract releases resources, assert that those processes and listeners - are gone, including detached owners and descendants where applicable. Name any data, port claims, - listeners, or processes the contract retains; client disposal need not stop a detached service. +- Require subprocess readiness and stdout/stderr diagnostics; clean up only exact owned resources and verify the cleanup required by the contract. Reproduce and stress flake fixes, then repeat the green case. ## Maintaining instructions