diff --git a/.agents/skills/agent-guidelines-audit b/.agents/skills/agent-guidelines-audit new file mode 120000 index 0000000..9e33ff8 --- /dev/null +++ b/.agents/skills/agent-guidelines-audit @@ -0,0 +1 @@ +../../AgentGuidelines/.agents/skills/agent-guidelines-audit \ No newline at end of file diff --git a/.github/workflows/ci-pr.yml b/.github/workflows/ci-pr.yml index 58f0bd6..6d1630e 100644 --- a/.github/workflows/ci-pr.yml +++ b/.github/workflows/ci-pr.yml @@ -18,7 +18,7 @@ jobs: uses: actions/checkout@v7 - name: Lint Swift Sources - run: AgentGuidelines/Scripts/swift_format.sh lint-strict Sources Tests + run: AgentGuidelines/Scripts/swift_format.sh lint-strict Package.swift Sources Tests test: name: Test diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ced6e30..ae4d4aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: uses: actions/checkout@v7 - name: Lint Swift Sources - run: AgentGuidelines/Scripts/swift_format.sh lint-strict Sources Tests + run: AgentGuidelines/Scripts/swift_format.sh lint-strict Package.swift Sources Tests test: name: Test diff --git a/AGENTS.md b/AGENTS.md index 685e70a..ec84a3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,6 +10,7 @@ The package is persistence-, UI-, and application-architecture agnostic. Host ap Read only the guides relevant to the task: +- [Agent workflow](AgentGuidelines/Guidelines/AgentWorkflow.md) - [Swift](AgentGuidelines/Guidelines/Swift/Swift.md) - [Swift style](AgentGuidelines/Guidelines/Swift/SwiftStyle.md) - [Swift format](AgentGuidelines/Guidelines/Swift/SwiftFormat.md) @@ -17,6 +18,7 @@ Read only the guides relevant to the task: - [Documentation](AgentGuidelines/Guidelines/Documentation.md) - [Logging](AgentGuidelines/Guidelines/Logging.md) - [Packages](AgentGuidelines/Guidelines/Packages.md) +- [Development workflow](AgentGuidelines/Guidelines/Development.md) - [CI/CD](AgentGuidelines/Guidelines/CICD.md) - [Git repositories and SSH-first cloning](AgentGuidelines/Guidelines/Git/Repositories.md) - [GitHub pull requests](AgentGuidelines/Guidelines/GitHub/PullRequests.md) @@ -42,6 +44,24 @@ Redux, SwiftData, SwiftUI, and application-localization guidance do not apply to - Update tests, DocC, README examples, and release notes when public behavior changes. - Use logging subsystem `com.thatfactory.cloudsavekit`, category `sync`, and canonical package emoji `☁️`. + +## Code Review Rules + +Review for release-blocking defects introduced or materially exposed by the pull request. A clean review means no unresolved P0/P1 findings; it does not mean exhaustive or perfect software. + +A blocking finding must identify a concrete, reachable path in a supported use case or the documented threat model that can cause a credible security-boundary bypass, durable data loss or corruption, a crash or deadlock, loss of availability, violation of an explicit acceptance criterion, or a serious compatibility regression. + +For every blocking finding, state the severity, preconditions, execution path, impact, evidence, and actionable remediation. Group manifestations that share the same root cause into one finding. + +Treat P2/P3 observations as non-blocking, including defense-in-depth, theoretical completeness, unsupported use cases, malformed state that trusted code cannot produce, behavior by components outside the threat model, style preferences, and speculative refactoring. Record a useful lower-severity observation once as deferred, declined, duplicate, or follow-up work; do not keep the review loop open for it. + +In an initial review, report substantiated blockers together. A follow-up review is limited to unresolved P0/P1 findings, changes since the last reviewed commit, and code directly affected by those changes. Do not restart an unrestricted review of unchanged code. A new follow-up finding must be a P0/P1 defect introduced by the remediation or genuinely hidden by the previous blocker. + +Automatic Codex review is the initial review. Do not request a manual Codex review unless the repository owner explicitly asks. Never request another review after each remediation commit. Within the normal review budget, at most one owner-authorized, delta-scoped verification review may be requested under [the pull-request review workflow](AgentGuidelines/Guidelines/GitHub/PullRequests.md). + + ## Codex review scope For consumer pull requests, do not substantively review `AgentGuidelines/**` after exact tagged-tree provenance has been verified. Verify its `VERSION`, compare its tree with the matching central tag, and verify the required `.gitattributes` rule. If provenance does not match exactly, review the subtree contents and stop the merge. Report substantive guideline feedback against the central `agent-guidelines` pull request. + +The marked block is intentional controlled duplication of the shared review policy. The tracked, synchronized subtree is reviewed centrally in `thatfactory/agent-guidelines`; the root-level instructions ensure the review contract and subtree scope are loaded even when Codex starts from the repository root. diff --git a/AgentGuidelines/.agents/skills/agent-guidelines-audit/SKILL.md b/AgentGuidelines/.agents/skills/agent-guidelines-audit/SKILL.md index 1f33702..2110942 100644 --- a/AgentGuidelines/.agents/skills/agent-guidelines-audit/SKILL.md +++ b/AgentGuidelines/.agents/skills/agent-guidelines-audit/SKILL.md @@ -1,6 +1,6 @@ --- name: agent-guidelines-audit -description: Audit completed repository work against the consumer's applicable agent-guidelines, local AGENTS.md instructions, requested scope, and declared validation workflow. Use after implementing changes and before claiming completion, handing work to the user, preparing, opening, or updating a pull request, declaring merge readiness, or preparing a release. Do not use for simple answers, read-only exploration, or work that is still actively being implemented. +description: Audit completed repository work and checked-in consumer integration against applicable agent-guidelines, local AGENTS.md instructions, requested scope, and declared validation workflow. Use after implementing changes and before claiming completion, handing work to the user, preparing, opening, or updating a pull request, declaring merge readiness, or preparing a release. Do not use for simple answers, read-only exploration, or work that is still actively being implemented. --- # Agent Guidelines Audit @@ -14,6 +14,9 @@ Perform a final, evidence-based compliance pass. Treat the applicable guidelines 3. Read the shared guides referenced by those instructions that apply to the changed files and workflow. 4. Inspect `git status`, the complete diff, and relevant untracked files. Preserve unrelated user changes. 5. Check the consumer's `AgentGuidelines/VERSION` and provenance when the task changes or depends on the synchronized subtree. Do not update it implicitly. +6. When the repository contains an `AgentGuidelines/` subtree, run `python3 AgentGuidelines/Scripts/validate_consumer_setup.py` from the consumer root. The validator detects Swift-format adoption from the root `AGENTS.md`; add `--require-swift-format` only when the repository must adopt it before that link is present. Treat failures as integration drift to fix or report before handoff. + +Do not inspect or require the user's global Codex instructions. They are user-level state outside the repository audit boundary; validate the checked-in root `AGENTS.md` contract instead. ## Audit the implementation @@ -28,6 +31,7 @@ Review the actual change rather than only checking whether files exist: - Check logging ownership, subsystem, categories, emoji, privacy, severity, metadata stability, and noise controls when logging changed. - Check durable documentation, package configuration, CI/CD, Xcode project configuration, security-sensitive changes, and physical-device limitations when they are in scope. Compare documented Swift and concurrency settings with the effective application and test-target settings; flag both redundant isolation annotations and missing annotations at compiler-verified boundaries. - Search for stale type names, superseded files, direct APIs forbidden by the new architecture, empty folders, and references to removed behavior. +- For pull-request or merge readiness, apply the root `## Code Review Rules`: confirm the review covers the current head, no allowed review round is pending, every thread has a disposition, and no unresolved P0/P1 blocker remains. Treat P2/P3 observations as non-blocking and never request another Codex review unless the repository owner explicitly authorizes it. ## Validate the evidence @@ -39,6 +43,14 @@ Run the repository's declared non-destructive checks in proportion to the change - repository-specific validators; - `git diff --check`. +When the shared Swift-format guide applies: + +- For implementation work, run `AgentGuidelines/Scripts/swift_format.sh format-and-lint` over every changed or applicable checked-in Swift source root before tests. For review-only work, use `lint-strict` so the audit does not mutate files. +- Confirm the root `.swift-format` and `.editorconfig` symlinks resolve to the synchronized shared configurations. +- Confirm pull-request and protected-branch CI run the shared wrapper with `lint-strict` in a dedicated non-mutating job. Reject `format` or `format-and-lint` in CI and verify the listed paths cover the repository's checked-in Swift roots. +- For Xcode projects, verify every independently buildable app or test target has the target-scoped pre-compilation phase described by the guide, including its `CI=true` bypass. +- For Swift packages, format `Package.swift`, `Sources`, `Tests`, and other checked-in Swift roots that exist before running `swift test`. Do not require `swift build` or `swift test` themselves to rewrite source; formatting and testing are consecutive, independently visible checks. + Use fresh successful evidence already produced in the same task instead of rerunning expensive checks without reason. Distinguish automated compilation and simulator evidence from hardware, signing, deployment, or manual validation that automation cannot prove. ## Resolve findings @@ -53,6 +65,7 @@ Use fresh successful evidence already produced in the same task instead of rerun Summarize: - the instruction and guideline areas audited; +- consumer-integration validation and any drift found; - findings fixed during the audit; - validation commands and outcomes; - any deliberate deviations, unavailable evidence, or remaining blockers. diff --git a/AgentGuidelines/.agents/skills/agent-guidelines-audit/agents/openai.yaml b/AgentGuidelines/.agents/skills/agent-guidelines-audit/agents/openai.yaml index dc4aab7..f7ecc03 100644 --- a/AgentGuidelines/.agents/skills/agent-guidelines-audit/agents/openai.yaml +++ b/AgentGuidelines/.agents/skills/agent-guidelines-audit/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Agent Guidelines Audit" - short_description: "Audit completed work against shared guidelines" - default_prompt: "Use $agent-guidelines-audit to audit this completed change before handoff." + short_description: "Audit work and shared-guideline integration" + default_prompt: "Use $agent-guidelines-audit to audit this completed change and its consumer guideline integration before handoff." diff --git a/AgentGuidelines/AGENTS.md b/AgentGuidelines/AGENTS.md index c6eef34..f82a61c 100644 --- a/AgentGuidelines/AGENTS.md +++ b/AgentGuidelines/AGENTS.md @@ -37,6 +37,22 @@ This public repository is the versioned source of truth for reusable ThatFactory --squash ``` + +## Code Review Rules + +Review for release-blocking defects introduced or materially exposed by the pull request. A clean review means no unresolved P0/P1 findings; it does not mean exhaustive or perfect software. + +A blocking finding must identify a concrete, reachable path in a supported use case or the documented threat model that can cause a credible security-boundary bypass, durable data loss or corruption, a crash or deadlock, loss of availability, violation of an explicit acceptance criterion, or a serious compatibility regression. + +For every blocking finding, state the severity, preconditions, execution path, impact, evidence, and actionable remediation. Group manifestations that share the same root cause into one finding. + +Treat P2/P3 observations as non-blocking, including defense-in-depth, theoretical completeness, unsupported use cases, malformed state that trusted code cannot produce, behavior by components outside the threat model, style preferences, and speculative refactoring. Record a useful lower-severity observation once as deferred, declined, duplicate, or follow-up work; do not keep the review loop open for it. + +In an initial review, report substantiated blockers together. A follow-up review is limited to unresolved P0/P1 findings, changes since the last reviewed commit, and code directly affected by those changes. Do not restart an unrestricted review of unchanged code. A new follow-up finding must be a P0/P1 defect introduced by the remediation or genuinely hidden by the previous blocker. + +Automatic Codex review is the initial review. Do not request a manual Codex review unless the repository owner explicitly asks. Never request another review after each remediation commit. Within the normal review budget, at most one owner-authorized, delta-scoped verification review may be requested under [the pull-request review workflow](Guidelines/GitHub/PullRequests.md). + + ## Validation Run: diff --git a/AgentGuidelines/CHANGELOG.md b/AgentGuidelines/CHANGELOG.md index e622148..1600a67 100644 --- a/AgentGuidelines/CHANGELOG.md +++ b/AgentGuidelines/CHANGELOG.md @@ -2,6 +2,25 @@ All notable changes to this project are documented in this file. +## [0.0.18] - 2026-08-18 + +### Changed + +- Corrected the standard README badge order to place DocC/documentation before license, updated date, revision, CI badges, and release/publishing status. + +## [0.0.17] - 2026-08-18 + +### Added + +- A version-marked consumer Code Review contract that defines P0/P1 release blockers, non-blocking P2/P3 observations, and bounded follow-up review scope. +- A deterministic consumer-setup validator for review-contract drift, subtree review scope, `.gitattributes`, local guide links, audit-skill wiring, and Swift-format adoption. + +### Changed + +- Expanded the global Codex instruction template and pull-request workflow to prioritize concrete release risk, group shared root causes, and stop review loops after blockers are resolved. +- Extended the completion-audit skill to verify consumer integration, review convergence, Swift-format configuration, local execution, and non-mutating CI coverage. +- Documented explicit Swift package formatting before tests and added a strict Swift-format CI template with package path coverage. + ## [0.0.16] - 2026-08-13 ### Changed diff --git a/AgentGuidelines/Guidelines/AgentWorkflow.md b/AgentGuidelines/Guidelines/AgentWorkflow.md index ebf6f13..8068dd6 100644 --- a/AgentGuidelines/Guidelines/AgentWorkflow.md +++ b/AgentGuidelines/Guidelines/AgentWorkflow.md @@ -42,6 +42,14 @@ Bound the combined output of grouped operations so that every result can be insp Do not expand the investigation merely because additional operations can be executed concurrently. +## Bounded iteration + +Before starting an iterative review, remediation, or model-assisted refinement loop, define its objective, blocking threshold, round budget, and stop condition. New non-blocking observations do not reset the budget or widen the original objective. + +Do not translate feedback directly into both a change and another review request. Classify the feedback, group items with the same root cause, batch accepted corrections, and rerun only the validation or bounded review needed to verify them. + +Stop when the stated acceptance condition is satisfied. Zero possible comments, improvements, or edge cases is not a valid completion criterion. For pull-request review severity, state tracking, and round limits, follow [GitHub pull requests](GitHub/PullRequests.md). + ## Efficiency Avoid unnecessary repeated model and tool cycles when several independent operations are already known. diff --git a/AgentGuidelines/Guidelines/Development.md b/AgentGuidelines/Guidelines/Development.md index 8f93d90..c733ca1 100644 --- a/AgentGuidelines/Guidelines/Development.md +++ b/AgentGuidelines/Guidelines/Development.md @@ -25,6 +25,8 @@ Before claiming implementation is complete, handing work to the user, preparing, If the skill is not discoverable in a subtree consumer, read and follow its [SKILL.md](../.agents/skills/agent-guidelines-audit/SKILL.md) directly. The audit is a final verification gate, not a substitute for reading and applying the relevant guidelines during implementation. Resolve in-scope findings and rerun affected checks before handoff. Do not broaden the requested scope merely to satisfy the audit. +For subtree consumers, the audit runs `python3 AgentGuidelines/Scripts/validate_consumer_setup.py` to detect drift in the root Code Review contract, Codex subtree-review scope, `.gitattributes`, local guide links, and repository skill symlink. When the root `AGENTS.md` links the shared Swift-format guide, the validator also requires the shared configuration symlinks and strict non-mutating CI adoption. User-level global Codex instructions are outside this repository audit. + ## Logging Applications own their orchestration, lifecycle, and product-domain diagnostics. Follow the shared [logging guide](Logging.md) and rely on each dependency to log its own implementation. Do not duplicate or reformat package-internal operations in the application log. diff --git a/AgentGuidelines/Guidelines/GitHub/PullRequests.md b/AgentGuidelines/Guidelines/GitHub/PullRequests.md index 3a2c349..80c12b1 100644 --- a/AgentGuidelines/Guidelines/GitHub/PullRequests.md +++ b/AgentGuidelines/Guidelines/GitHub/PullRequests.md @@ -5,6 +5,9 @@ Use this guide whenever creating, reviewing, updating, or merging a GitHub pull ## Before opening - Review the complete diff and exclude unrelated changes. +- Keep each pull request to a coherent review unit with a bounded set of invariants. Split changes that combine independent architecture, persistence, security, transport, and CI concerns when they can be reviewed and delivered separately; do not split merely to minimize line count. +- State the supported use cases, explicit acceptance criteria, and relevant threat model for behavior whose review priority depends on those boundaries. +- For security guarantees based on enumerating formats or signatures, define the finite coverage contract and residual risk, or use a systemic boundary that enforces the guarantee without exhaustive enumeration. - Follow the repository's pull-request template and local contribution instructions. - Run the relevant local validation and document anything that could not be run. - Open the pull request without auto-merge and keep it unmerged while automated or agent review is pending. Use draft state only when configured reviewers also run on drafts. @@ -14,43 +17,92 @@ Use this guide whenever creating, reviewing, updating, or merging a GitHub pull When reviewing a consumer pull request, do not review or comment on files under `AgentGuidelines/**` after exact tagged-tree provenance has been verified. The subtree is a tracked, synchronized copy marked `linguist-generated`; substantive guideline changes are reviewed in the central `thatfactory/agent-guidelines` pull request. Verify `AgentGuidelines/VERSION`, compare the subtree tree with the matching central tag (for example with `git subtree split --prefix=AgentGuidelines HEAD` and a tree comparison after fetching that tag), and verify the required `.gitattributes` rule. If provenance does not match exactly, review the subtree contents and stop the merge. Report substantive guideline feedback against the central pull request instead. +## Review objective + +Automated review identifies release-blocking regressions; it does not attempt to eliminate every possible improvement. + +Classify findings by impact and reachable scope: + +- **P0 — critical:** an actively exploitable critical security issue, catastrophic durable data loss, or critical production outage. +- **P1 — blocking:** a supported use case, explicit acceptance criterion, or documented threat-model boundary has a concrete reachable failure path that causes a security-boundary bypass, durable data loss or corruption, a crash or deadlock, loss of availability, or a serious compatibility regression. +- **P2 — non-blocking:** robustness, defense-in-depth, bounded edge cases, malformed state that trusted code cannot produce, unsupported scenarios, theoretical completeness, or useful hardening. +- **P3 — non-blocking:** style, naming, preferred refactoring, documentation polish, or optional test improvements. + +Only unresolved P0 and P1 findings block merge. A finding may be technically correct without being release-blocking. + ## Review gate Opening a pull request starts review; it does not authorize merging it. 1. Wait for the configured Codex review to finish. No review yet means pending, not approved. -2. Inspect all review summaries, inline threads, checks, and requested changes. -3. Assess each comment on its technical merits. -4. Implement valid feedback and rerun the affected validation. -5. If feedback should not be implemented, reply in the original thread with a concise technical reason. -6. Reply to implemented feedback with what changed and where. -7. Resolve a thread only after its concern has been addressed or explicitly declined. -8. After addressing review comments, update the pull-request description so it matches the current implementation, validation, and any remaining limitations. -9. Recheck the pull request immediately before merge for late comments and check-state changes. +2. Record the reviewed head SHA and inspect all review summaries, inline threads, checks, and requested changes. +3. Assess each comment for technical correctness, severity, supported reachability, and root cause. +4. Give every thread one explicit disposition: `BLOCKER-P0`, `BLOCKER-P1`, `DEFER-P2`, `DEFER-P3`, `DECLINE`, or `DUPLICATE`. +5. Batch accepted P0/P1 corrections into one remediation pass and add regression coverage where reasonably possible. Lower-severity improvements may be included when they are small and clearly in scope, but they do not keep the review loop open. +6. Reply in the original thread with the disposition and either what changed or the concise technical reason for deferring, declining, or grouping it. +7. Resolve a thread only after its disposition is recorded. Reference a follow-up issue for deferred work when its value justifies one. +8. Rerun affected validation, then update the pull-request description so it matches the current implementation, validation, deferred work, and remaining limitations. +9. Recheck the pull request immediately before merge for late P0/P1 findings and check-state changes. When replying with a commit reference, write the commit hash as raw text without backticks (for example, the hash 185c04f should remain 185c04f). GitHub then auto-links the hash to the commit. A thumbs-up or clean Codex review satisfies the agent-review step, but it does not replace any human approval required by the repository. Do not enable auto-merge before all review gates are satisfied. +### Review state and round budget + +Track enough state to prevent duplicate requests and unbounded review loops: + +```text +initial_review_sha +last_reviewed_sha +review_requested_sha +review_round +pending_review +unresolved_p0 +unresolved_p1 +deferred_findings +``` + +The automatic review is the one initial full review. Do not request another review after each fix. A repository owner may explicitly authorize at most one delta-scoped verification review after the known P0/P1 findings have been batch-remediated. + +Before sending that request, verify that no Codex review is pending, no existing request targets the current head SHA, the current head differs from `last_reviewed_sha`, and the verification-round budget is unused. Persist `review_requested_sha`, increment `review_round`, and mark `pending_review` before waiting for a result so a retry cannot submit a duplicate request. + +When authorized, scope the verification request explicitly: + +```text +@codex review only unresolved P0/P1 findings and changes since . +Do not search unchanged code for new P2/P3 issues. +``` + +Do not request a third review or restart a full review without separate, explicit repository-owner authorization and a named unresolved P0/P1 concern. A new finding in verification must be a P0/P1 defect introduced by the remediation or genuinely hidden by the previous blocker. + +Stop the review loop when no unresolved P0/P1 finding remains, every thread has an explicit disposition, required checks pass, and required human authorization is present. Zero comments, zero possible improvements, and zero technical debt are not completion criteria. + ### Codex review monitoring Use GitHub review data, reactions, and checks together. An eyes reaction means Codex is processing the pull request; it is not an approval. A thumbs-up means the review completed without suggestions. A submitted review means its inline threads must be assessed individually. ```text -PR opened +PR opened at stable head | v -Codex adds eyes reaction +One automatic full review | - +--> thumbs-up ----------------> Clean review + +--> thumbs-up ----------------> No P0/P1 blockers | - `--> Review comments ----------> Assess each comment + `--> Review comments ----------> Classify and group | - fix or decline with reason + batch P0/P1 fixes | - reply in original thread - | - resolve thread + owner-authorized delta review? + | | + no yes + | | + stop one verification pass + | + no unresolved P0/P1 + | + stop ``` When using the GitHub CLI, monitor all three surfaces: @@ -97,20 +149,22 @@ gh api graphql --paginate \ -F thread= ``` -Continue polling while actively working on the pull request. Inspect every returned page for reactions, review threads, and thread comments. Do not treat missing comments, a pending reaction, truncated results, or elapsed time as review completion. +Continue polling only while an allowed review round is pending. Inspect every returned page for reactions, review threads, and thread comments. Do not treat missing comments, a pending reaction, truncated results, or elapsed time as review completion, and do not submit a duplicate request merely because polling has not completed. ## Merge requirements Do not merge while any of the following is true: - Codex review is still pending; -- an actionable review comment is unanswered; -- a review conversation is unresolved; +- an unresolved P0/P1 finding remains; +- a review thread lacks an explicit disposition or remains unresolved; - a required check is pending or failing; - the branch is out of date when the repository requires an up-to-date branch; - required human approval or explicit owner authorization is missing. -If a review arrives after a premature merge, treat that as a process failure: assess the feedback, reply to every thread, and ship valid corrections through a follow-up pull request. +## Late findings + +If a review arrives after merge, assess and disposition its findings. A valid late P0/P1 finding requires prompt remediation through a corrective pull request and indicates that a review gate was missed. A late P2/P3 observation becomes backlog work when useful and is not by itself a process failure. ## Repository protection diff --git a/AgentGuidelines/Guidelines/Packages.md b/AgentGuidelines/Guidelines/Packages.md index da332c3..a9b0576 100644 --- a/AgentGuidelines/Guidelines/Packages.md +++ b/AgentGuidelines/Guidelines/Packages.md @@ -17,11 +17,12 @@ Use only badges that describe the repository, in this order: 3. Supported platforms. 4. Relevant package manager, runtime, or ecosystem badges, such as SPM or NPM. 5. Relevant agent or tooling badges, such as Xcode MCP, Codex, or Claude. -6. Updated date. -7. Revision or latest release. -8. License. -9. CI. -10. Release, publishing, or documentation status when applicable. +6. DocC, documentation. +7. License. +8. Updated date. +9. Revision or latest release. +10. CI badges. +11. Release/publishing status when applicable. The common package baseline is Swift, Xcode, Platforms, License, and CI. Add optional badges only when they convey useful repository-specific information. Keep the order stable even when some positions are omitted. diff --git a/AgentGuidelines/Guidelines/Swift/SwiftFormat.md b/AgentGuidelines/Guidelines/Swift/SwiftFormat.md index b5060fb..eeb1f8d 100644 --- a/AgentGuidelines/Guidelines/Swift/SwiftFormat.md +++ b/AgentGuidelines/Guidelines/Swift/SwiftFormat.md @@ -19,6 +19,58 @@ - Source mutation requires either declared source inputs and outputs or disabling Xcode's **User Script Sandboxing** for the affected configurations. Record and review that choice locally; never disable sandboxing without the formatting phase requiring it. - Validate the integration in Xcode with an open, deliberately misformatted file. Confirm formatting happens before compilation and that editor saving, cursor state, and undo behavior remain acceptable. +## Swift package integration + +- Do not make `swift build` or `swift test` rewrite package sources. Formatting is an explicit local preparation step; builds and tests remain reproducible and non-mutating. +- Before building, testing, or handing off a package change, format and lint every checked-in Swift source root plus the manifest. A package with the standard layout runs: + + ```sh + AgentGuidelines/Scripts/swift_format.sh format-and-lint \ + Package.swift \ + Sources \ + Tests + + swift test + ``` + +- Omit a path only when it does not exist, and add nonstandard checked-in Swift source roots such as `Plugins` or `Examples`. Do not scan `.build`, generated artifacts, vendored dependencies, or another package's sources. +- Keep formatting and testing as consecutive, independently visible commands. A repository-owned convenience script may compose them, but formatting must finish before `swift test` begins and a formatting failure must stop the workflow. +- SwiftPM command plugins may provide an additional manual entry point, but they do not replace the shared configuration, wrapper, or CI check. Do not add a formatter package dependency solely to duplicate the toolchain-provided formatter without a documented repository need. + +[SwiftPM build-tool plugins](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0303-swiftpm-extensible-build-tools.md) have read-only access to package source directories. This makes non-mutating lint possible in a custom build integration, but source-rewriting formatting does not belong inside the build. Prefer the explicit workflow above unless a package documents why every build must also pay the cost of a dedicated lint plugin. + +## CI integration + +- Run `lint-strict` in a dedicated, non-mutating job for pull requests and merges to the protected branch. Never run `format` or `format-and-lint` in CI. +- Use the same explicit source scope as the local workflow. Package CI includes `Package.swift`, `Sources`, `Tests`, and any additional checked-in Swift roots that exist. Xcode-project CI covers the union of source folders compiled by the project's independently buildable targets. +- Select the consumer's documented self-hosted macOS runner labels and supported Xcode toolchain. Keep repository-specific runner labels and Xcode selection outside this shared example. + +A typical Swift package job is: + +```yaml +swift-format: + name: Swift Format + runs-on: [self-hosted, macOS, ARM64] + + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Select and log Xcode + run: | + xcodebuild -version + xcode-select -p + + - name: Run strict swift-format lint + run: | + AgentGuidelines/Scripts/swift_format.sh lint-strict \ + Package.swift \ + Sources \ + Tests +``` + +Adapt the runner labels and path list to the consumer. Keep the command shape unchanged so local execution, the consumer validator, and CI use the same shared wrapper and strict policy. + ## Shared customizations The checked-in configuration starts from the exhaustive Xcode toolchain dump. These deliberate overrides are the shared policy and must be reapplied when the toolchain changes. diff --git a/AgentGuidelines/README.md b/AgentGuidelines/README.md index e29c651..b6ab81a 100644 --- a/AgentGuidelines/README.md +++ b/AgentGuidelines/README.md @@ -87,7 +87,7 @@ From the consumer repository root, install a tagged release: git subtree add \ --prefix=AgentGuidelines \ https://github.com/thatfactory/agent-guidelines.git \ - 0.0.16 \ + 0.0.18 \ --squash ``` @@ -105,13 +105,13 @@ Keep the subtree tracked, but add this to the consumer's tracked `.gitattributes AgentGuidelines/** linguist-generated ``` -Copy and adapt [the consumer template](Templates/AGENTS.md). Keep the consumer file small: describe the product or package, map its concrete physical folders, point to the applicable shared guides, and state only genuine exceptions. +Copy and adapt [the consumer template](Templates/AGENTS.md). Keep the consumer file small: describe the product or package, map its concrete physical folders, point to the applicable shared guides, and state only genuine exceptions. Keep the version-marked code-review contract directly in the repository-root `AGENTS.md`; Markdown links to shared guides are navigation, not automatic instruction includes. ### Configure global Codex instructions Copy the contents of [`Templates/GlobalCodexInstructions.md`](Templates/GlobalCodexInstructions.md) into the user's global Codex instructions. -These instructions only bootstrap discovery of repository-local `AGENTS.md` files and shared guides. Repository engineering policy remains versioned in this repository rather than duplicated in each user's global configuration. +These instructions bootstrap discovery of repository-local `AGENTS.md` files and shared guides and provide generic high-signal code-review defaults. Repository engineering policy and specialized threat models remain versioned in this repository or the consumer rather than duplicated in each user's global configuration. Review this template when upgrading `agent-guidelines`, because the recommended global bootstrap instructions may change between releases. Installing or updating the Git subtree does not update a user's global Codex configuration. @@ -125,6 +125,14 @@ ln -s ../../AgentGuidelines/.agents/skills/agent-guidelines-audit \ .agents/skills/agent-guidelines-audit ``` +Validate the checked-in consumer integration directly or through the completion-audit skill: + +```sh +python3 AgentGuidelines/Scripts/validate_consumer_setup.py +``` + +The validator checks the version-marked root Code Review contract, Codex subtree-review scope, `.gitattributes`, local guide links, and the audit-skill symlink. When the root `AGENTS.md` links the shared Swift-format guide, it also requires both configuration symlinks and a non-mutating `lint-strict` CI invocation. Pass `--require-swift-format` only when auditing formatter adoption before adding that guide link. + ## Update a consumer Review the target release's changelog, then pull it deliberately: @@ -133,11 +141,11 @@ Review the target release's changelog, then pull it deliberately: git subtree pull \ --prefix=AgentGuidelines \ https://github.com/thatfactory/agent-guidelines.git \ - 0.0.16 \ + 0.0.18 \ --squash ``` -Confirm `AgentGuidelines/VERSION`, ensure the `.gitattributes` rule above is present, review the subtree diff, validate local `AGENTS.md` pointers, and run the consumer's relevant tests. Keep the subtree update in its own commit, and identify the old and new versions plus the central release or pull request in the consumer pull-request description. Updates are intentionally not automatic: one guideline release cannot silently change every project. +Confirm `AgentGuidelines/VERSION`, review the subtree diff, synchronize the marked code-review contract when its version changes, run `python3 AgentGuidelines/Scripts/validate_consumer_setup.py`, and run the consumer's relevant tests. Keep the subtree update in its own commit, and identify the old and new versions plus the central release or pull request in the consumer pull-request description. Updates are intentionally not automatic: one guideline release cannot silently change every project. ## Maintain the source of truth diff --git a/AgentGuidelines/Scripts/validate_consumer_setup.py b/AgentGuidelines/Scripts/validate_consumer_setup.py new file mode 100755 index 0000000..687e1a1 --- /dev/null +++ b/AgentGuidelines/Scripts/validate_consumer_setup.py @@ -0,0 +1,304 @@ +#!/usr/bin/env python3 +"""Validate a consumer repository's checked-in agent-guidelines integration.""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + + +GUIDELINES_ROOT = Path(__file__).resolve().parents[1] +CONTRACT_BEGIN = "" +CONTRACT_END = "" +MARKDOWN_LINK = re.compile(r"\[[^\]]+\]\(([^)]+)\)") +SWIFT_FORMAT_GUIDE = "AgentGuidelines/Guidelines/Swift/SwiftFormat.md" +STRICT_FORMAT_COMMAND = re.compile( + r"(?m)^[ \t]*(?:-\s+)?(?:run:\s*)?(?:\./)?" + r"AgentGuidelines/Scripts/swift_format\.sh\s+lint-strict(?=\s|\\|$)", +) +MUTATING_FORMAT_COMMAND = re.compile( + r"(?m)^[ \t]*(?:-\s+)?(?:run:\s*)?(?:\./)?" + r"AgentGuidelines/Scripts/swift_format\.sh\s+format(?:-and-lint)?(?=\s|\\|$)", +) +GENERATED_ATTRIBUTE = re.compile( + r"^\s*AgentGuidelines/\*\*\s+linguist-generated\s*$", + re.MULTILINE, +) + + +def read_text(path: Path, errors: list[str], label: str) -> str | None: + try: + return path.read_text(encoding="utf-8") + except OSError as error: + errors.append(f"{label}: cannot read {path}: {error}") + return None + + +def extract_contract(contents: str, errors: list[str], label: str) -> str | None: + if contents.count(CONTRACT_BEGIN) != 1 or contents.count(CONTRACT_END) != 1: + errors.append( + f"{label}: expected exactly one {CONTRACT_BEGIN!r} and {CONTRACT_END!r}" + ) + return None + + start = contents.index(CONTRACT_BEGIN) + end = contents.index(CONTRACT_END, start) + len(CONTRACT_END) + return contents[start:end].strip() + + +def validate_symlink(path: Path, expected: Path, errors: list[str], label: str) -> None: + if not path.is_symlink(): + errors.append(f"{label}: {path} must be a symlink to {expected}") + return + + if not path.exists(): + errors.append(f"{label}: {path} is a broken symlink") + return + + if path.resolve() != expected.resolve(): + errors.append(f"{label}: {path} resolves to {path.resolve()}, expected {expected.resolve()}") + + +def validate_agent_links( + agents_path: Path, + contents: str, + errors: list[str], +) -> None: + for raw_target in MARKDOWN_LINK.findall(contents): + target = raw_target.strip().strip("<>").split("#", maxsplit=1)[0] + if not target or target.startswith(("#", "http://", "https://", "mailto:")): + continue + + resolved = (agents_path.parent / target).resolve() + if not resolved.exists(): + errors.append(f"AGENTS.md: missing local link target {raw_target!r}") + + +def adopts_swift_format(contents: str) -> bool: + for raw_target in MARKDOWN_LINK.findall(contents): + target = raw_target.strip().strip("<>").split("#", maxsplit=1)[0] + if target.endswith(SWIFT_FORMAT_GUIDE): + return True + return False + + +def shell_invocations(contents: str, command: re.Pattern[str]) -> list[str]: + lines = contents.splitlines() + invocations: list[str] = [] + index = 0 + while index < len(lines): + line = lines[index] + if line.lstrip().startswith("#") or not command.search(line): + index += 1 + continue + + invocation = [line] + while invocation[-1].rstrip().endswith("\\") and index + 1 < len(lines): + index += 1 + invocation.append(lines[index]) + invocations.append("\n".join(invocation)) + index += 1 + return invocations + + +def validate_swift_format_ci(consumer_root: Path, errors: list[str]) -> None: + workflows_root = consumer_root / ".github" / "workflows" + workflows = sorted(workflows_root.glob("*.yml")) + sorted( + workflows_root.glob("*.yaml") + ) + if not workflows: + errors.append( + "consumer Swift format CI: no GitHub Actions workflows found under " + ".github/workflows" + ) + return + + strict_workflows: list[tuple[Path, str, list[str]]] = [] + for workflow in workflows: + contents = read_text( + workflow, + errors, + f"consumer Swift format CI workflow {workflow.relative_to(consumer_root)}", + ) + if contents is None: + continue + if shell_invocations(contents, MUTATING_FORMAT_COMMAND): + errors.append( + "consumer Swift format CI: " + f"{workflow.relative_to(consumer_root)} must not mutate sources with " + "format or format-and-lint" + ) + invocations = shell_invocations(contents, STRICT_FORMAT_COMMAND) + if invocations: + strict_workflows.append((workflow, contents, invocations)) + + if not strict_workflows: + errors.append( + "consumer Swift format CI: missing " + "'AgentGuidelines/Scripts/swift_format.sh lint-strict' invocation" + ) + return + + if not any( + re.search(r"(?m)^\s*pull_request\s*:", contents) + for _, contents, _ in strict_workflows + ): + errors.append("consumer Swift format CI: lint-strict does not run for pull requests") + + if not any( + re.search(r"(?m)^\s*push\s*:", contents) + and re.search(r"(?m)^\s*-?\s*main\s*$|branches\s*:\s*\[[^]]*\bmain\b", contents) + for _, contents, _ in strict_workflows + ): + errors.append("consumer Swift format CI: lint-strict does not run for pushes to main") + + if (consumer_root / "Package.swift").is_file(): + required_paths = ["Package.swift"] + required_paths.extend( + name for name in ("Sources", "Tests") if (consumer_root / name).is_dir() + ) + combined = "\n".join( + invocation + for _, _, invocations in strict_workflows + for invocation in invocations + ) + for path in required_paths: + if not re.search(rf"(? None: + consumer_root = consumer_root.resolve() + guidelines_root = guidelines_root.resolve() + subtree = consumer_root / "AgentGuidelines" + + if not subtree.exists(): + errors.append(f"consumer root: missing {subtree}") + elif subtree.resolve() != guidelines_root: + errors.append( + f"consumer root: {subtree} resolves to {subtree.resolve()}, " + f"expected the active guidelines at {guidelines_root}" + ) + + version_path = guidelines_root / "VERSION" + version = read_text(version_path, errors, "AgentGuidelines/VERSION") + if version is not None and not version.strip(): + errors.append("AgentGuidelines/VERSION: version is empty") + + template_path = guidelines_root / "Templates" / "AGENTS.md" + agents_path = consumer_root / "AGENTS.md" + template = read_text(template_path, errors, "AgentGuidelines template") + agents = read_text(agents_path, errors, "consumer AGENTS.md") + + swift_format_adopted = require_swift_format + if template is not None and agents is not None: + expected_contract = extract_contract(template, errors, "AgentGuidelines template") + actual_contract = extract_contract(agents, errors, "consumer AGENTS.md") + if ( + expected_contract is not None + and actual_contract is not None + and actual_contract != expected_contract + ): + errors.append( + "consumer AGENTS.md: code-review contract does not match " + "AgentGuidelines/Templates/AGENTS.md" + ) + + required_scope_values = ( + "## Codex review scope", + "AgentGuidelines/**", + "exact tagged-tree provenance", + ".gitattributes", + ) + for value in required_scope_values: + if value not in agents: + errors.append(f"consumer AGENTS.md: missing Codex review scope value {value!r}") + + validate_agent_links(agents_path, agents, errors) + swift_format_adopted = swift_format_adopted or adopts_swift_format(agents) + + attributes_path = consumer_root / ".gitattributes" + attributes = read_text(attributes_path, errors, "consumer .gitattributes") + if attributes is not None and not GENERATED_ATTRIBUTE.search(attributes): + errors.append( + "consumer .gitattributes: missing exact " + "'AgentGuidelines/** linguist-generated' rule" + ) + + expected_skill = guidelines_root / ".agents" / "skills" / "agent-guidelines-audit" + consumer_skill = consumer_root / ".agents" / "skills" / "agent-guidelines-audit" + validate_symlink(consumer_skill, expected_skill, errors, "consumer audit skill") + + formatter_links = { + ".swift-format": guidelines_root / "Configurations" / "Swift" / ".swift-format", + ".editorconfig": guidelines_root / "Configurations" / "Swift" / ".editorconfig", + } + for name, expected in formatter_links.items(): + path = consumer_root / name + if swift_format_adopted or path.is_symlink(): + validate_symlink(path, expected, errors, f"consumer {name}") + + if swift_format_adopted: + validate_swift_format_ci(consumer_root, errors) + + +def parse_arguments() -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--consumer-root", + type=Path, + help="Consumer repository root. Defaults to the parent of an AgentGuidelines subtree.", + ) + parser.add_argument( + "--require-swift-format", + action="store_true", + help=( + "Require shared Swift-format configuration links and strict CI even when " + "AGENTS.md does not link the Swift-format guide." + ), + ) + return parser.parse_args() + + +def main() -> int: + arguments = parse_arguments() + consumer_root = arguments.consumer_root + if consumer_root is None: + if GUIDELINES_ROOT.name != "AgentGuidelines": + print( + "Consumer setup validation failed:\n" + "- --consumer-root is required when this checkout is not installed " + "as an AgentGuidelines subtree" + ) + return 1 + consumer_root = GUIDELINES_ROOT.parent + + errors: list[str] = [] + validate_consumer_setup( + errors, + consumer_root, + require_swift_format=arguments.require_swift_format, + ) + if errors: + print("Consumer setup validation failed:") + for error in errors: + print(f"- {error}") + return 1 + + version = (GUIDELINES_ROOT / "VERSION").read_text(encoding="utf-8").strip() + print(f"Validated consumer setup for agent-guidelines {version}.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/AgentGuidelines/Scripts/validate_guidelines.py b/AgentGuidelines/Scripts/validate_guidelines.py index 23c935d..95fa1e3 100644 --- a/AgentGuidelines/Scripts/validate_guidelines.py +++ b/AgentGuidelines/Scripts/validate_guidelines.py @@ -17,6 +17,8 @@ SWIFT_FORMAT_CONFIGURATION = ROOT / "Configurations" / "Swift" / ".swift-format" EDITOR_CONFIGURATION = ROOT / "Configurations" / "Swift" / ".editorconfig" SWIFT_FORMAT_SCRIPT = ROOT / "Scripts" / "swift_format.sh" +SWIFT_FORMAT_GUIDELINE = ROOT / "Guidelines" / "Swift" / "SwiftFormat.md" +CONSUMER_SETUP_SCRIPT = ROOT / "Scripts" / "validate_consumer_setup.py" AUDIT_SKILL = ROOT / ".agents" / "skills" / "agent-guidelines-audit" / "SKILL.md" DEVELOPMENT_GUIDELINE = ROOT / "Guidelines" / "Development.md" AGENTS_TEMPLATE = ROOT / "Templates" / "AGENTS.md" @@ -148,6 +150,8 @@ def validate_readme_contract(errors: list[str]) -> None: "AgentGuidelines/Configurations/Swift/.swift-format": "swift-format symlink command", "AgentGuidelines/Configurations/Swift/.editorconfig": "EditorConfig symlink command", ".agents/skills/agent-guidelines-audit": "completion-audit skill setup", + "validate_consumer_setup.py": "consumer setup validation command", + "--require-swift-format": "explicit Swift-format adoption validation", } for value, description in required.items(): if value not in readme: @@ -252,6 +256,33 @@ def validate_swift_format_script(errors: list[str]) -> None: errors.append(f"{SWIFT_FORMAT_SCRIPT.relative_to(ROOT)}: script is not executable") +def validate_swift_format_guideline(errors: list[str]) -> None: + contents = SWIFT_FORMAT_GUIDELINE.read_text(encoding="utf-8") + required = { + "## Swift package integration": "Swift package workflow", + "format-and-lint \\": "local package formatting command", + "Package.swift": "package manifest formatting scope", + "## CI integration": "CI workflow", + "lint-strict \\": "strict CI command", + "Never run `format` or `format-and-lint` in CI": "non-mutating CI rule", + } + for value, description in required.items(): + if value not in contents: + errors.append( + f"{SWIFT_FORMAT_GUIDELINE.relative_to(ROOT)}: " + f"missing {description}: {value!r}" + ) + + +def validate_consumer_setup_script(errors: list[str]) -> None: + if not CONSUMER_SETUP_SCRIPT.is_file(): + errors.append(f"{CONSUMER_SETUP_SCRIPT.relative_to(ROOT)}: missing script") + elif not os.access(CONSUMER_SETUP_SCRIPT, os.X_OK): + errors.append( + f"{CONSUMER_SETUP_SCRIPT.relative_to(ROOT)}: script is not executable" + ) + + def validate_audit_skill(errors: list[str]) -> None: if not AUDIT_SKILL.is_file(): errors.append(f"{AUDIT_SKILL.relative_to(ROOT)}: missing audit skill") @@ -262,6 +293,10 @@ def validate_audit_skill(errors: list[str]) -> None: "name: agent-guidelines-audit": "skill name", "before claiming completion": "completion trigger", "git diff --check": "diff validation", + "validate_consumer_setup.py": "consumer integration validation", + "format-and-lint": "local Swift-format audit", + "lint-strict": "strict Swift-format CI audit", + "no unresolved P0/P1 blocker remains": "Codex review stopping rule", } for value, description in required_skill_values.items(): if value not in skill: @@ -295,6 +330,8 @@ def main() -> int: validate_swift_format_configuration(errors) validate_editor_configuration(errors) validate_swift_format_script(errors) + validate_swift_format_guideline(errors) + validate_consumer_setup_script(errors) validate_audit_skill(errors) if errors: diff --git a/AgentGuidelines/Templates/AGENTS.md b/AgentGuidelines/Templates/AGENTS.md index 845abf7..2728920 100644 --- a/AgentGuidelines/Templates/AGENTS.md +++ b/AgentGuidelines/Templates/AGENTS.md @@ -27,15 +27,31 @@ Read only the guides relevant to the task: For an application that uses Redux, also read [Redux architecture](AgentGuidelines/Guidelines/Architecture/Redux.md). -Add the following section to the consumer repository's root `AGENTS.md` so it is loaded for root-level Codex and pull-request work: +Keep the following marked contract in the consumer repository's root `AGENTS.md` so it is loaded directly for root-level Codex and pull-request work. Copy it unchanged and update it when the marker version changes in this template; a Markdown link to the detailed workflow is not an instruction include. ```md + +## Code Review Rules + +Review for release-blocking defects introduced or materially exposed by the pull request. A clean review means no unresolved P0/P1 findings; it does not mean exhaustive or perfect software. + +A blocking finding must identify a concrete, reachable path in a supported use case or the documented threat model that can cause a credible security-boundary bypass, durable data loss or corruption, a crash or deadlock, loss of availability, violation of an explicit acceptance criterion, or a serious compatibility regression. + +For every blocking finding, state the severity, preconditions, execution path, impact, evidence, and actionable remediation. Group manifestations that share the same root cause into one finding. + +Treat P2/P3 observations as non-blocking, including defense-in-depth, theoretical completeness, unsupported use cases, malformed state that trusted code cannot produce, behavior by components outside the threat model, style preferences, and speculative refactoring. Record a useful lower-severity observation once as deferred, declined, duplicate, or follow-up work; do not keep the review loop open for it. + +In an initial review, report substantiated blockers together. A follow-up review is limited to unresolved P0/P1 findings, changes since the last reviewed commit, and code directly affected by those changes. Do not restart an unrestricted review of unchanged code. A new follow-up finding must be a P0/P1 defect introduced by the remediation or genuinely hidden by the previous blocker. + +Automatic Codex review is the initial review. Do not request a manual Codex review unless the repository owner explicitly asks. Never request another review after each remediation commit. Within the normal review budget, at most one owner-authorized, delta-scoped verification review may be requested under [the pull-request review workflow](AgentGuidelines/Guidelines/GitHub/PullRequests.md). + + ## Codex review scope For consumer pull requests, do not substantively review `AgentGuidelines/**` after exact tagged-tree provenance has been verified. Verify its `VERSION`, compare its tree with the matching central tag, and verify the required `.gitattributes` rule. If provenance does not match exactly, review the subtree contents and stop the merge. Report substantive guideline feedback against the central `agent-guidelines` pull request. ``` -This tracked, synchronized subtree is reviewed centrally in `thatfactory/agent-guidelines`; the root-level instruction ensures the review scope is loaded even when Codex starts from the repository root. +The marked block is intentional controlled duplication of the shared review policy. The tracked, synchronized subtree is reviewed centrally in `thatfactory/agent-guidelines`; the root-level instructions ensure the review contract and subtree scope are loaded even when Codex starts from the repository root. ## Physical folder map diff --git a/AgentGuidelines/Templates/GlobalCodexInstructions.md b/AgentGuidelines/Templates/GlobalCodexInstructions.md index 4949810..abe4578 100644 --- a/AgentGuidelines/Templates/GlobalCodexInstructions.md +++ b/AgentGuidelines/Templates/GlobalCodexInstructions.md @@ -6,4 +6,25 @@ When a repository includes shared agent guidelines, read only the guides referen Repository and folder-level instructions may specialize the shared baseline within their scope. Do not replace deliberate repository conventions with generic global preferences. -Do not duplicate repository guidance in global instructions. Global instructions should bootstrap discovery of the repository's own sources of truth. +Do not duplicate repository-specific guidance in global instructions. Global instructions should bootstrap discovery of the repository's own sources of truth. + +## Code review behavior + +When acting as a code reviewer, optimize for high-signal release risk and convergence rather than exhaustive perfection. + +Create an inline finding only when all of the following are true: + +1. The issue is introduced or materially exposed by the proposed change. +2. There is a concrete, reachable failure path in a supported use case or the documented threat model. +3. The impact is P0 or P1: a credible security-boundary bypass, durable data loss or corruption, a crash or deadlock, loss of availability, violation of an explicit acceptance criterion, or a serious compatibility regression. +4. The evidence and remediation are specific enough to be actionable. + +State the finding's severity, preconditions, execution path, impact, and evidence. Group findings that share the same root cause. Do not create separate serial comments for additional manifestations of an already reported root cause. + +Treat P2 and P3 observations as non-blocking. This includes defense-in-depth, theoretical completeness, unsupported use cases, malformed state that trusted code cannot produce, adversarial behavior by components outside the threat model, style preferences, speculative refactoring, and exhaustive enumeration of equivalent input formats. Summarize valuable lower-severity observations once or recommend a follow-up issue. + +In the initial review, report substantiated blockers together rather than drip-feeding them across repeated reviews. + +In a follow-up review, verify previously reported P0/P1 findings and review only changes since the previously reviewed commit plus code directly affected by those changes. Do not restart an unrestricted search of unchanged code. A newly introduced follow-up finding must be a P0/P1 issue introduced by the remediation or genuinely hidden by the previous defect. + +A clean review means that there are no unresolved P0/P1 blockers. It does not mean perfect software, zero possible improvements, or zero technical debt. diff --git a/AgentGuidelines/Tests/test_validate_consumer_setup.py b/AgentGuidelines/Tests/test_validate_consumer_setup.py new file mode 100644 index 0000000..4db91af --- /dev/null +++ b/AgentGuidelines/Tests/test_validate_consumer_setup.py @@ -0,0 +1,253 @@ +"""Tests for consumer agent-guidelines integration validation.""" + +from __future__ import annotations + +import importlib.util +import tempfile +import unittest +from pathlib import Path + + +VALIDATOR_PATH = ( + Path(__file__).resolve().parents[1] / "Scripts" / "validate_consumer_setup.py" +) +SPEC = importlib.util.spec_from_file_location("validate_consumer_setup", VALIDATOR_PATH) +assert SPEC is not None +assert SPEC.loader is not None +VALIDATOR = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(VALIDATOR) + + +class ConsumerSetupTests(unittest.TestCase): + """Verifies checked-in consumer guidance and symlink wiring.""" + + def setUp(self) -> None: + self.temporary_directory = tempfile.TemporaryDirectory(dir=VALIDATOR.GUIDELINES_ROOT) + self.consumer_root = Path(self.temporary_directory.name) + (self.consumer_root / "AgentGuidelines").symlink_to( + VALIDATOR.GUIDELINES_ROOT, + target_is_directory=True, + ) + + template = ( + VALIDATOR.GUIDELINES_ROOT / "Templates" / "AGENTS.md" + ).read_text(encoding="utf-8") + contract_errors: list[str] = [] + contract = VALIDATOR.extract_contract(template, contract_errors, "template") + self.assertEqual(contract_errors, []) + self.assertIsNotNone(contract) + + agents = f"""# Project Instructions + +{contract} + +## Codex review scope + +For consumer pull requests, verify exact tagged-tree provenance before excluding +`AgentGuidelines/**`, and confirm the required `.gitattributes` rule. +""" + (self.consumer_root / "AGENTS.md").write_text(agents, encoding="utf-8") + (self.consumer_root / ".gitattributes").write_text( + "AgentGuidelines/** linguist-generated\n", + encoding="utf-8", + ) + + skill_parent = self.consumer_root / ".agents" / "skills" + skill_parent.mkdir(parents=True) + (skill_parent / "agent-guidelines-audit").symlink_to( + VALIDATOR.GUIDELINES_ROOT + / ".agents" + / "skills" + / "agent-guidelines-audit", + target_is_directory=True, + ) + + def tearDown(self) -> None: + self.temporary_directory.cleanup() + + def validate(self, require_swift_format: bool = False) -> list[str]: + errors: list[str] = [] + VALIDATOR.validate_consumer_setup( + errors, + self.consumer_root, + require_swift_format=require_swift_format, + ) + return errors + + def adopt_swift_format(self) -> None: + agents_path = self.consumer_root / "AGENTS.md" + agents = agents_path.read_text(encoding="utf-8") + agents_path.write_text( + f"{agents}\n[Swift format]({VALIDATOR.SWIFT_FORMAT_GUIDE})\n", + encoding="utf-8", + ) + + def add_swift_format_links(self) -> None: + (self.consumer_root / ".swift-format").symlink_to( + VALIDATOR.GUIDELINES_ROOT / "Configurations" / "Swift" / ".swift-format" + ) + (self.consumer_root / ".editorconfig").symlink_to( + VALIDATOR.GUIDELINES_ROOT / "Configurations" / "Swift" / ".editorconfig" + ) + + def add_package(self) -> None: + (self.consumer_root / "Package.swift").write_text( + "// swift-tools-version: 6.0\n", + encoding="utf-8", + ) + (self.consumer_root / "Sources").mkdir() + (self.consumer_root / "Tests").mkdir() + + def add_strict_format_ci(self, paths: tuple[str, ...] = ()) -> None: + workflows = self.consumer_root / ".github" / "workflows" + workflows.mkdir(parents=True) + command = " AgentGuidelines/Scripts/swift_format.sh lint-strict" + if paths: + path_lines = [] + for index, path in enumerate(paths): + continuation = " \\" if index < len(paths) - 1 else "" + path_lines.append(f" {path}{continuation}") + command += " \\\n" + "\n".join(path_lines) + (workflows / "ci-pr.yml").write_text( + f"""name: CI (PR) +on: + pull_request: + push: + branches: [main] +jobs: + swift-format: + name: Swift Format + steps: + - name: Run strict swift-format lint + run: | +{command} +""", + encoding="utf-8", + ) + + def test_valid_consumer_setup(self) -> None: + """Accepts a synchronized root contract and repository skill symlink.""" + self.assertEqual(self.validate(), []) + + def test_rejects_stale_code_review_contract(self) -> None: + """Rejects a consumer root contract that drifted from the template.""" + agents_path = self.consumer_root / "AGENTS.md" + agents = agents_path.read_text(encoding="utf-8") + agents_path.write_text(agents.replace("P0/P1", "P0", 1), encoding="utf-8") + + self.assertTrue( + any("code-review contract does not match" in error for error in self.validate()) + ) + + def test_rejects_copied_audit_skill(self) -> None: + """Rejects a stale-copy risk in place of the repository symlink.""" + skill = self.consumer_root / ".agents" / "skills" / "agent-guidelines-audit" + skill.unlink() + skill.mkdir() + (skill / "SKILL.md").write_text("stale copy\n", encoding="utf-8") + + self.assertTrue(any("must be a symlink" in error for error in self.validate())) + + def test_requires_generated_attribute(self) -> None: + """Rejects a consumer without the collapsed subtree diff rule.""" + (self.consumer_root / ".gitattributes").write_text("*.md text\n", encoding="utf-8") + + self.assertTrue(any("linguist-generated" in error for error in self.validate())) + + def test_rejects_missing_local_agent_link(self) -> None: + """Rejects a local guideline pointer that does not resolve.""" + agents_path = self.consumer_root / "AGENTS.md" + agents = agents_path.read_text(encoding="utf-8") + agents_path.write_text( + f"{agents}\n[Missing guide](AgentGuidelines/Guidelines/Missing.md)\n", + encoding="utf-8", + ) + + self.assertTrue( + any("missing local link target" in error for error in self.validate()) + ) + + def test_explicitly_requires_formatter_adoption(self) -> None: + """Allows an audit to require formatting before AGENTS.md links the guide.""" + self.assertEqual(self.validate(), []) + + errors = self.validate(require_swift_format=True) + + self.assertTrue(any("consumer .swift-format" in error for error in errors)) + self.assertTrue(any("consumer .editorconfig" in error for error in errors)) + self.assertTrue(any("Swift format CI" in error for error in errors)) + + def test_adopted_formatter_requires_links_and_ci(self) -> None: + """Detects incomplete adoption from the consumer AGENTS.md link.""" + self.adopt_swift_format() + + errors = self.validate() + + self.assertTrue(any("consumer .swift-format" in error for error in errors)) + self.assertTrue(any("consumer .editorconfig" in error for error in errors)) + self.assertTrue(any("Swift format CI" in error for error in errors)) + + def test_accepts_adopted_package_formatter(self) -> None: + """Accepts package configuration links and complete strict CI coverage.""" + self.adopt_swift_format() + self.add_swift_format_links() + self.add_package() + self.add_strict_format_ci(("Package.swift", "Sources", "Tests")) + + self.assertEqual(self.validate(), []) + + def test_rejects_mutating_ci_formatter(self) -> None: + """Rejects source rewriting inside CI even when strict lint is also present.""" + self.adopt_swift_format() + self.add_swift_format_links() + self.add_strict_format_ci() + workflow = self.consumer_root / ".github" / "workflows" / "ci-pr.yml" + workflow.write_text( + workflow.read_text(encoding="utf-8") + + " AgentGuidelines/Scripts/swift_format.sh format Sources\n", + encoding="utf-8", + ) + + self.assertTrue(any("must not mutate sources" in error for error in self.validate())) + + def test_rejects_ci_without_required_triggers(self) -> None: + """Requires strict lint for pull requests and main-branch merges.""" + self.adopt_swift_format() + self.add_swift_format_links() + self.add_strict_format_ci() + workflow = self.consumer_root / ".github" / "workflows" / "ci-pr.yml" + contents = workflow.read_text(encoding="utf-8") + workflow.write_text( + contents.replace(" pull_request:\n", "").replace( + " push:\n branches: [main]\n", + "", + ), + encoding="utf-8", + ) + + errors = self.validate() + + self.assertTrue(any("does not run for pull requests" in error for error in errors)) + self.assertTrue(any("does not run for pushes to main" in error for error in errors)) + + def test_rejects_incomplete_package_ci_scope(self) -> None: + """Requires all standard package Swift roots in strict CI.""" + self.adopt_swift_format() + self.add_swift_format_links() + self.add_package() + self.add_strict_format_ci(("Sources",)) + workflow = self.consumer_root / ".github" / "workflows" / "ci-pr.yml" + workflow.write_text( + workflow.read_text(encoding="utf-8") + + " - run: echo Package.swift Tests\n", + encoding="utf-8", + ) + + errors = self.validate() + + self.assertTrue(any("'Package.swift'" in error for error in errors)) + self.assertTrue(any("'Tests'" in error for error in errors)) + + +if __name__ == "__main__": + unittest.main() diff --git a/AgentGuidelines/Tests/test_validate_guidelines.py b/AgentGuidelines/Tests/test_validate_guidelines.py index d250633..1216b37 100644 --- a/AgentGuidelines/Tests/test_validate_guidelines.py +++ b/AgentGuidelines/Tests/test_validate_guidelines.py @@ -111,6 +111,14 @@ def test_editor_configuration(self) -> None: self.assertEqual(errors, []) + def test_swift_format_guideline(self) -> None: + """Accepts the documented package and non-mutating CI workflows.""" + errors: list[str] = [] + + VALIDATOR.validate_swift_format_guideline(errors) + + self.assertEqual(errors, []) + class AgentGuidelinesAuditSkillTests(unittest.TestCase): """Verifies the mandatory completion-audit skill contract.""" @@ -123,6 +131,14 @@ def test_audit_skill_contract(self) -> None: self.assertEqual(errors, []) + def test_consumer_setup_script(self) -> None: + """Accepts the executable deterministic consumer validator.""" + errors: list[str] = [] + + VALIDATOR.validate_consumer_setup_script(errors) + + self.assertEqual(errors, []) + if __name__ == "__main__": unittest.main() diff --git a/AgentGuidelines/VERSION b/AgentGuidelines/VERSION index e3b86dd..32786aa 100644 --- a/AgentGuidelines/VERSION +++ b/AgentGuidelines/VERSION @@ -1 +1 @@ -0.0.16 +0.0.18 diff --git a/Package.swift b/Package.swift index 305890f..394a8d0 100644 --- a/Package.swift +++ b/Package.swift @@ -15,7 +15,7 @@ let package = Package( .library( name: "CloudSaveKit", targets: ["CloudSaveKit"] - ), + ) ], dependencies: [ .package(url: "https://github.com/thatfactory/applogger", from: "1.1.0"), @@ -28,7 +28,7 @@ let package = Package( .product( name: "AppLogger", package: "applogger" - ), + ) ] ), .testTarget(