Skip to content

docs: deslop AGENTS.md without weakening its rules (#921) - #925

Merged
mforce merged 4 commits into
mainfrom
docs/921-deslop-agents
Sep 21, 2026
Merged

mforce merged 4 commits into
mainfrom
docs/921-deslop-agents

Conversation

@mforce

@mforce mforce commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

Rewrites the dense rules in AGENTS.md without changing what any of them require, and adds a rule keeping GitHub issue state out of the file.

9,677 → 8,603 words (−11%), ~1,070 fewer tokens in every agent session. 50 of 96 rules rewritten; 46 deliberately left, each with a reason.

Nothing load-bearing was lost, verified by count rather than by reading: 44 → 44 decision links, 90 → 90 issue references, 4 → 4 runbook links, and every accepted-risk phrase, exception and opt-out intact. Don't throw for expected failures is restored to its negative form.

The Phase context section no longer names a "current phase" — it had said 1.5 for eight days after epic #15 closed. Each entry now states what the code does, with the issue as a pointer, and current priorities point at the milestones and open epics.

Three code-versus-doc mismatches are flagged but not fixed here, since each needs a precision edit rather than a deletion: gh pr edit now works (tested, exit 0 on gh 2.101.0), ci.yml:189 runs the AppHost test leg, and e2e-smoke.yml / k6-baseline.yml drive tools/simulation.

Full accounting — the 46 unchanged rules with reasons, the verification detail and the test census — is in a comment on this PR rather than the body, so the squash commit message stays readable.

Closes #921

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 25f0528c-7d35-4aea-80b6-7595c26b0c83


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mforce

mforce commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Changes requested — this is a partial pass reported as a complete one

The word count is accurate: 9,677 → 9,296 (−381, −3.9%), verified against both file
versions. The edits that were made are good — the #565 Aspire and #776 coverage rewrites
genuinely read better and preserve their links, identifiers and boundaries.

The problem is coverage. The diff is 12 hunks / +37 −41 lines against a file of
96 rule paragraphs, and the rules it skipped are exactly the ones #921 was filed
about. Every one of the twelve longest paragraphs is untouched:

words rule (opening)
355 **A documentation-only PR skips web and image, and the gate is pull_request-only (#782).*…
340 Multi-tenancy: every tenant-owned entity has AccountId, enforced by an EF **global query …
292 Run one serving instance. Every instance runs DurableJobWorker, but its poll and the thre…
279 **A farm code changes only through Account.Rename, reached by the rename-account verb (#732…
267 Do not extend that list from memory — it was twice derived wrongly, both times a process-lo…
239 The four test projects are legs of one fail-fast CI matrix (#775). ci.yml's tests job r…
220 **Help prose naming a control follows that control's LABEL, per locale, and only review checks …
215 Every adapter declares its module reach, and shrinking it is free (#514/#846). `AdapterReac…
215 **Any PR that changes what a user sees attaches screenshots, captured from a stack rebuilt at t…
208 Business dates, row timestamps and list sequences are separate facts (#819). Keep farm oper…
198 A PR closes its issue from the BODY, never from the title. Closes #NNN on its own line in…
190 **A reference from one business module to another is declared in the module ledger (#514/#842).…
168 A PR that ships work another OPEN issue claims amends that issue in the same PR. An issue b…
166 Count a selector's call sites before styling it (#662). Three selectors named across #651/#…
163 AuditEvents carries a durable monotonic ordering key, and it is GLOBAL (#508). Sequence…
163 **A tier declares a privilege a module may not otherwise have, before the surface that needs it…
157 Seed / simulation data is never boot-seeded (#280, #284, #279). Run it explicitly against a…
150 A write-contract change must update its non-CI callers (#394). Coverage is not uniform, and…
137 **Runtime verification uses the repo's verify skill at [tools/verify/](tools/verify/SKILL.m…
119 NuGet versions live in Directory.Packages.props (#684). Central Package Management: every…

74 of 96 rule paragraphs unchanged — 6953 of 8487 rule words (81%).

The #775 CI-matrix paragraph (239 words, one sentence of which runs 60) is the clearest
case: it sits between two bullets this PR rewrote and was stepped over. The pass took
the short, easy bullets — the intro, "Communicating", the Result-pattern line, two
Build/test/run entries — and left every dense paragraph in place.

Acceptance criterion 1 is "Every section of AGENTS.md has been reviewed, not only
recently changed text." Reviewing a rule and leaving it alone is a legitimate outcome, but
not for 81% of the file's rule text when density is the stated problem. If a specific
paragraph genuinely cannot be shortened without weakening it, criterion 5 already asks you
to say so explicitly in the PR — that is the answer for those, not silence.

What to do

  1. Work the table above, longest first. That is where the value of docs: /deslop AGENTS.md without weakening its rules #921 is.
  2. For each, the bar is unchanged: no → link, issue reference, command, identifier,
    exception, accepted risk or enforcement boundary may change meaning. Split long
    sentences, drop stock framing and repetition, keep the rule one paragraph.
  3. Where you decide a paragraph must stay as it is, name it in the PR body with the reason
    — "shortening this would weaken it" is a valid, expected outcome and belongs on record.
  4. Re-report the word count, and this time also report how many of the 96 rule
    paragraphs you changed and how many you deliberately left
    .

One substantive nit in what you did change

- **Result pattern:** lost Don't throw for expected failures in favour of
Return expected failures. Same intent, but the explicit prohibition is the half an agent
scans for when asking "what must I not do". Restore the negative form.

@mforce
mforce force-pushed the docs/921-deslop-agents branch from e3786cd to b9d5343 Compare September 21, 2026 03:04
@mforce

mforce commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Review detail for #925

Moved out of the PR body so the squash commit message stays readable — this repo squashes with PR_BODY as the commit message, and the body was 19,087 characters. The full accounting is preserved here.

The previous pass left the twelve longest rules untouched. This completes the brief's 25-paragraph worklist, longest first, splitting dense sentences while preserving commands, identifiers, links, exceptions, accepted risks and enforcement limits. It also restores the explicit Result-pattern and mutation-check prohibitions.

Only AGENTS.md changes. The original e3786cd work is retained as 5daa018 after the requested rebase onto fb5a8f1; this completion is a separate commit. The deslop edits preserve the rules and guarantees. The follow-up keeps the owner-supplied test census and replaces copied tracker state with technical facts and issue pointers. A new preamble convention prevents reintroducing that failure.

  • Words: 9,677 → 8,603, a reduction of 1,074 (11.1%). The previous head was 9,296 words.
  • Changed: 50 of 96 rule paragraphs, including all twelve longest and all 25 in the ranked brief.
  • Deliberately left: 46 of 96 rule paragraphs, each named with its reason below.

One new rule is added, for 97 total; it is excluded from the original-96 changed/left comparison. This follow-up goes from 8,484 to 8,603 words, including that new convention. Counts use whitespace-separated words and the same 96-paragraph inventory as the owner's review: lines beginning with ** or - **. Comparing origin/main with e3786cd using this inventory reproduces the owner's 22 changed / 74 unchanged and 8,487 baseline rule words. Unchanged material outside that inventory is accounted for separately.

Deliberately unchanged rule paragraphs (46)

Rule Reason
Run full stack (prod-like): The exact Compose command, port and serving arrangement already fit one short sentence.
Run frontend dev: The dev command, URL and proxy target are already minimal.
Debug API (no docker stack): The command, database port, Development context and user-secrets restriction are all needed.
Handler per feature The direct-handler convention, MediatR prohibition and registration location are already concise.
Validation: The validator naming, cardinality and endpoint calls are already a compact instruction.
Endpoints: The route convention, authentication and idempotency requirement leave no redundant clause.
Nullable enabled Both build-breaking constraints are already stated in ten words.
The coupling matrix is generated from the ledger and its three walks (#848). The regeneration command, report validation and observable/unobservable matrix symbols are distinct facts; cutting them would lose the interpretation or workflow.
Flock-scoped writes require a resolved actor (#787). The unresolved-actor refusal, unrestricted-read exception and system-actor access-review requirement must stay together.
AuditEvents is not time-partitioned, on purpose (#505). The query shape explains the deliberate partitioning refusal and the account-partition alternative without repetition.
InitialCreate is frozen; one migration per change (#407). The migration freeze, unregenerable SQL and pre-#407 drop/recreate exception each explain a separate failure.
Base reference data ships as guarded raw-SQL migrations (#283). Whole-set versus per-key guards and the explicit HasData/InsertData prohibition are essential to the seed contract.
The design-time connection is fail-closed (#318). The unset-connection refusal, Production TLS floor and exact loopback opt-out are already compact.
Farm configuration and identity are Owner-only (#729). The exact Owner/Admin gates, permitted reads and Manager restrictions are a necessary permission inventory.
Both JWT keys are checked at boot, and the check is serving-only (#510/#347). Both importable keys, blank-string handling and boot-versus-request timing explain different failure modes.
Credential epoch revocation (#364). Retired epoch zero, malformed claims and the no-cache rule each protect revocation; none is redundant.
First-run admin: bootstrap-admin (#283). The no-Owner precondition, output restriction, no-op and two middleware exceptions are all required.
Break-glass: recover-admin (#265). The Production exception, generated-password prohibition and single-transaction contents are already concise.
Nothing writes an audit event without an actor (#500). The actor requirement and authorization consequence explain why a literal role label is insufficient.
Proxy-trust boot guard (#260). The proxy dependency and direct-TLS-only opt-out must remain at the boot rule.
Production Postgres TLS floor (#261/#262). Every TLS outcome, including unset, the plaintext opt-out and Npgsql spelling exception are needed.
GSS/Kerberos negotiation off by default (#332). Presence-versus-value detection and textual append ordering preserve operator configuration and unknown-key behavior.
Farm timezone + tzdata/ICU (#264). Provisioning validation, UTC default, Owner setup, runtime packages and both-role canary each constrain a different step.
A new Production boot guard must be taught to the sim harness (#370). The caller-update obligation remains valid, but its no-CI claim is stale. Left unchanged for the owner to approve the corrected coverage boundary described below.
A new required config key must also be taught to the AppHost (#565). The configuration obligation remains valid, but its no-CI claim is stale. Left unchanged pending wording that distinguishes model/configuration tests from a full Aspire boot.
Migrate command + prod migration split (#263). The exact verb, no-DDL serving rule, ordering-only guarantee and readiness backstop are already compact.
Container image hardening (#267). Non-root execution, three digest pins, scan severity and the glibc requirement are separate constraints.
Stays here This is the portable operational-contract inventory; removing examples would narrow what belongs here.
Does NOT belong here This is the explicit prohibited deployment-content inventory; shortening would remove exclusions.
Run a local adversarial pass before the first push The brief rule already states the timing and both allowed adversarial methods.
Two misses of the same shape mean the METHOD is wrong The two-miss trigger and required change in method are already concise.
For a pinned/golden value, prove portability The single sentence explains why repeated local success does not prove portability.
Prefer the boring guard The short instruction gives the reason to avoid complexity in trusted guards.
NuGet lock files. Lockfile timing, locked-mode failure and the Dependabot repair workflow are all operational facts.
Pin third-party Actions to a full commit SHA The full-SHA requirement, version comment, compromise references and first-party exceptions must remain together.
Every merge to main Both release stages and their exact image tags are already compact.
Promotion is a server-side retag of the existing digest The retag-only rule, digest-affecting flag and never-rebuild prohibition cannot be shortened safely.
The release stays a draft until its image is promoted The draft/tag behavior explains the failure outcome in one sentence.
The version comes from conventional commits, damped below 1.0.0. Both breaking-change syntaxes, pre-1.0 damping flags and the silent 1.0 transition define the version contract.
A commit-body parse error drops the whole commit The dropped-commit outcome, forbidden body syntax and PR-title hook gap are distinct limits.
The release PR is opened with a GitHub App token, not GITHUB_TOKEN. The token distinction and required permission downscoping explain the otherwise silent privilege expansion.
Never hand-edit .release-please-manifest.json or version.txt The two generated-file prohibitions already take one short sentence.
Deploy by digest, never by tag. All three verification flags, the reference-versus-digest distinction and deploy runbook link are load-bearing.
A PR that changes the UI carries screenshots This deliberate second screenshot reminder is needed where agents open PRs; its pointer avoids duplicating the mechanics.
Keep phase epics in sync The filing/merge actions, epic mapping and insufficient-milestone warning already form a compact workflow.
Keep documentation in sync The dated owner directive has two separate documentation obligations and a review standard.

The earlier pass also changed prose outside the 96-rule inventory: the introduction, communication guidance, real-value storage instruction, host-portability and provider-review paragraphs, guard introduction, CI-security introduction, release introduction and graphify introduction. Those edits are retained.

The test command's census comment is also updated outside the 96-rule inventory.

Other unchanged material reviewed

Material outside the 96-rule inventory Reason
Project description and layout block Stack and directory reference data; no prose reduction is needed.
Section headings and navigation Preserve heading anchors and section order as requested.
Dependency direction and architecture-reading instruction The inward dependency rule, Domain exclusion and required architecture read are already concise.
Local API debug configuration Keep the exact user-secrets command and project key.
No hardcoded passwords/keys Keep the explicit prohibition, scanner and runtime-test-credential instruction.
Pre-commit hook paragraph Preserve every trigger, command, excluded test class and skip option.
Release trust-boundary paragraph beginning “Net, stated at exactly the strength…” Leave byte-for-byte: neither gate blocks substitution of other attested bytes or survives a merge to main; self-merge with zero approvals remains an open path. Shortening risks overstating provenance.
Canonical release-boundary cross-reference paragraph Keep the explicit three-copy synchronization rule; deduplicating it would remove the maintenance instruction.
Package visibility and host pull credentials The deployment-side responsibility and cluckwork-deploy#6 reference already fit one sentence.
GitHub origin / Gitea mirror The remote identities and gh instruction are already minimal.
Domain terms and architecture references Preserve the required reading before renaming/modeling and the state-connection reference.
Graphify query/path/explain instruction Commands, graph-existence condition and scoped-output purpose remain necessary.
Dirty graph output rule Keep the explicit prohibition on skipping and both permitted exceptions.
Graph wiki/navigation rule Both existence condition and broad-review-only restriction are necessary.
Periodic graph update rule Keep the exact command, AST-only/no-cost statement and prohibition on unrelated per-change churn.

Verification

  • Prior deslop-head verification: CI run 35556271071 passed on b9d5343, including integration. CodeQL and other PR checks also completed without failures; Web and Image were skipped under the documentation-only gate.
  • Application tests: 532 passed, including tracked-document guards. Ran dotnet test tests/Cluckwork.Application.Tests/Cluckwork.Application.Tests.csproj --logger 'console;verbosity=normal'. The initial no-restore command produced no test results and is not counted as verification.
  • Per-rule audit: no original backtick-delimited command/identifier, issue reference or link target is missing. Repeated identifiers may occur fewer times where a pronoun retains their meaning.
  • All existing Markdown link targets and section headings are retained; Phase context adds links to milestones, open epics and the existing product-spec path; local target files/directories and the unlinked fix(api): order same-instant audit events by a durable monotonic key, not a random Guid #508 diagnosis path exist. Existing executable command text is unchanged; the new rule adds gh issue view <n> --json state; the test-count/date comment was updated under the owner's follow-up request; command paths and named scripts remain present. Commands that mutate a deployment were checked as references, not executed. The Aspire CLI is not installed on this host; its unchanged command was checked against the linked runbook, not run.
  • Manual before/after review checked negative prohibitions, exclusions, accepted costs and statements of what is not guaranteed. Tests cannot prove prose equivalence.
  • The existing navigation target #deploy-invariant-exactly-one-serving-api-instance-271-338 does not match the current heading ending in #271. It predates this PR and is retained under the explicit instruction to preserve links, issue references and heading anchors.
  • git diff --check passes; the PR diff contains only AGENTS.md.

Tracker-state rewrite and Phase context proposal

The full-file audit found 13 sentences to rewrite: 12 tracker-state assertions plus one ambiguous “options remain open” sentence. Census against 0adad80:

Location Sentences Replacement
Phase context 5 Remove epic #13/#14/#530 closure/completion claims, the #357/#388/#537 closed follow-up list, and the current-Phase-1.5 announcement. Retain every issue pointer.
Single serving instance 6 Remove closed labels on #271/#556 while retaining their lock/endpoint mechanisms. Replace #545/#544/#338 closure language with the shared report lease, IP counter and replay/logout mechanisms. Explain #307's database-coordinated HTTP claim/result protocol without licensing scaling.
Singleton inventory 1 Remove “Both are closed above”; the adjacent #338/#544 caller-wiring facts remain.
CI matrix 1 Describe container reuse and parallelism as optimization options discussed in #775, without asserting an open status.

Separately, the MCP tier sentence now ties dormancy to an unmapped surface and empty namespace rather than “when #806 lands.” It was a code condition, not another tracker-state assertion. The #806 pointer and green/not-red limit remain. Historical defect accounts and conditional instructions to amend or maintain GitHub issues remain; changing tracker state cannot invalidate those instructions or past events.

Phase context proposal for owner review: retain the egg loop, operational features and multi-farm behavior as capabilities, with their original issue pointers. Keep 530-multi-farm-tenancy.md and its accepted costs; make the immediate-suspension boundary explicitly about use. Replace the numbered current-phase announcement with live milestone and open-epic links. Keep the product-spec §6 pointer and epic #15's hardening inventory as scope references, and preserve the glossary/architecture instruction. This removes the hand-maintained phase status without selecting a new phase on the owner's behalf. The previous deferred Phase 1.5 flag and Phase 1.6 completion refresh are superseded by this proposal.

New convention: one unlinked paragraph immediately after “Every rule here is one paragraph.” It identifies the silent failure, names the eight-day phase drift and obsolete Remaining list, directs state queries to gh, and explicitly says to maintain epic checklists in GitHub. “Keep phase epics in sync” is unchanged and complementary: maintain the tracker, do not copy its answers here. The preamble is the right location because this governs the whole file.

Separate code-versus-doc mismatches retained for owner review

These three flags remain; their AGENTS.md paragraphs are unchanged by the tracker-state pass:

  • Simulation boot guards (Sim harness (#243) rotted silently: 4 breakages, no CI ever ran it #370). “Deliberately not in CI” is stale: e2e-smoke.yml runs on matching pull requests and executes bootstrap.sh, verify-harness.sh and reset.sh at lines 109, 114 and 160. The dispatch-only k6-baseline.yml also bootstraps and verifies the simulation harness, then runs it at lines 104–114. The same-PR caller-update rule still applies; docs-only PRs skip this workflow, and slow/canary modes remain dispatch-only. Left unchanged so the owner can approve wording that preserves those limits.
  • AppHost configuration (Developer experience: add an Aspire AppHost for local orchestration and observability #565). “The AppHost is deliberately not in CI” is stale: ci.yml includes the AppHost test leg. AppHostModelTests.Api_and_web_model_the_required_expressions checks configuration expressions. These are model/configuration tests, not a full running Aspire stack, so this finding does not imply that every missing serving config key is caught. Left unchanged pending the owner's coverage wording.
  • PR body editing command. The claim that gh pr edit fails here with a Projects-classic deprecation error is no longer true in this environment: gh pr edit 925 --body-file /tmp/921-staleness-body.md successfully updated this PR during this audit. The documented gh api -X PATCH fallback remains valid. The paragraph is unchanged; the owner should decide whether to remove the failure claim or qualify it by affected CLI versions.

The rest of the file was reviewed for factual claims. No additional definite code-versus-doc mismatch was established. The owner's verified singleton counts, nine ledger owners, links, verify symlinks and documentation-path classifier were accepted without re-deriving them.

Test census and latest validation

The owner-supplied local full-solution census remains 2,887 as of 2026-09-21: Domain 491 + Application 532 + AppHost 10 + Integration 1,854. Prior CI run 35557295013 on 0adad80 reported 2,884 (1,851 integration cases, all passed); the three-case difference is unexplained. That CI run does not independently reproduce the supplied local census.

The tracker rewrite passed all 532 Application tests with dotnet test tests/Cluckwork.Application.Tests/Cluckwork.Application.Tests.csproj --no-restore --verbosity quiet. The local adversarial diff review checked the shared-state mechanisms, transaction-pooling limit, fallback bounds, no-scaling prohibition, and all accepted-risk phrases. Every existing inline identifier, distinct issue reference and link target survives; the repeated #806 pointer is consolidated into one occurrence in its paragraph. The original 44 decision links remain. git diff --check passes; only AGENTS.md changes. CI run 35558931055 passed on 5cf427f, including all four test legs. CodeQL and the other PR checks finished without failures; Web and Image were skipped by the documentation-only gate.

Closes #921

@mforce

mforce commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Follow-up filed as #928 — moving the inline narrative out of the 48 rules that already link to a decision record (4,689 words, 54% of the file). That issue also carries the three code-versus-doc mismatches this PR flagged and deliberately left.

@mforce
mforce merged commit e44d435 into main Sep 21, 2026
16 checks passed
@mforce
mforce deleted the docs/921-deslop-agents branch September 21, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: /deslop AGENTS.md without weakening its rules

1 participant