Share general review and documentation rules

General error, cancellation, test, and documentation rules keep
contracts precise without importing another project's package layout.
Multiline boundaries, real-path tests for text-dependent errors,
required test dependencies, and support claims have explicit rules.

Signature verification can use a temporary allowed-signers file without
configuration changes. Discussion-derived changes include an archive
link. The existing code placement and signing exception stay intact.

Issue: 3cdc678

diff --git a/.agents/skills/commit/SKILL.md b/.agents/skills/commit/SKILL.md
index 11b5eb2f53d6850e26e3f97dc5eca7d3086eb410..35c8be887c6f525a1a6f7c85574dc21f6394ccc6 100644 (file)
--- a/.agents/skills/commit/SKILL.md
+++ b/.agents/skills/commit/SKILL.md
@@ -30,8 +30,10 @@ exception does not permit unsigned code or documentation commits.
    commit hook still runs. Never skip it with `--no-verify`.
 6. Cryptographically verify each new normal commit's signature against
    the configured key before reporting completion. A signature header
    commit hook still runs. Never skip it with `--no-verify`.
 6. Cryptographically verify each new normal commit's signature against
    the configured key before reporting completion. A signature header

-   alone is not verification. If verification fails, stop and report it.
-   Do not claim completion or publish the commit.
+   alone is not verification. If `gpg.ssh.allowedSignersFile` is not
+   set, use a temporary allowed-signers file with the configured key. Do
+   not change the Git configuration. If verification fails, stop and
+   report it. Do not claim completion or publish the commit.

 
 ## Size
 
 
 ## Size
 


@@ -75,6 +77,8 @@ exception does not permit unsigned code or documentation commits.
   `git interpret-trailers --trailer` before the message check. Never use
   a closing keyword such as "Fixes" or "Closes".
 - Link the bug report when a change fixes a reported bug.
   `git interpret-trailers --trailer` before the message check. Never use
   a closing keyword such as "Fixes" or "Closes".
 - Link the bug report when a change fixes a reported bug.

+- Include a stable archive URL when a change comes from a mailing-list
+  or forum discussion.

 - Do not add an AI tool as co-author. Do not add attribution trailers.
 
 ## Example
 - Do not add an AI tool as co-author. Do not add attribution trailers.
 
 ## Example

diff --git a/.agents/skills/docs/SKILL.md b/.agents/skills/docs/SKILL.md
index 2f7f3dc97dc83ddfd2f37a4214fe8fc9dc6dfbe8..b95c3ba7839b166d80bceca8eb9feac4e513805b 100644 (file)
--- a/.agents/skills/docs/SKILL.md
+++ b/.agents/skills/docs/SKILL.md
@@ -5,6 +5,9 @@ description: Docs and comment rules. Use before you write any prose.
 
 # Documentation and comments
 
 
 # Documentation and comments
 

+Change documentation and comments in the same change as the behavior,
+interface, or constraint that they describe.
+

 ## Documentation
 
 - `README.rst` is the entry point. It says what OneChat is, its status,
 ## Documentation
 
 - `README.rst` is the entry point. It says what OneChat is, its status,


@@ -17,9 +20,34 @@ description: Docs and comment rules. Use before you write any prose.
   in the Go doc comments of the code that owns them.
 - Do not add other documentation files or a `docs/` folder. Document
   commands and flags in the command help, and code contracts and design
   in the Go doc comments of the code that owns them.
 - Do not add other documentation files or a `docs/` folder. Document
   commands and flags in the command help, and code contracts and design

-  reasons in Go doc comments.
+  reasons in Go doc comments. Keep help and Go comments consistent with
+  the implementation.

 - Use Markdown only for files that a tool requires by name, such as
   `AGENTS.md` and `SKILL.md`.
 - Use Markdown only for files that a tool requires by name, such as
   `AGENTS.md` and `SKILL.md`.

+- Follow
+  [Google's timeless documentation guide](https://developers.google.com/style/timeless-documentation).
+  Describe what the checked-in code does. Do not use labels that depend
+  on time, such as "new", "now", "currently", and "previously".
+- Follow
+  [Google's tense guide](https://developers.google.com/style/tense). Use
+  the present tense for general behavior. Use the future tense only when
+  the timing of an action requires it, not for planned features.
+- Keep previous designs, development history, roadmaps, and future work
+  in issues.
+- State exact support and limits instead of labels such as "prototype".
+  Keep implemented, tested, and unverified behavior apart.
+- Keep safety requirements and known limits explicit. For example, say
+  when fake-service tests do not verify compatibility with the live
+  service.
+- Keep compatibility and old-schema details when the code or the
+  migration tests require them. They describe an active contract, not
+  history.
+- Do not remove a warning or claim support only to remove historical
+  wording. Check the implementation and the tests before you change a
+  claim.
+- Compare claims, examples, paths, and links with the implementation and
+  the tests. Make sure that examples use supported interfaces and
+  contain no secrets.

 
 ## Lists
 
 
 ## Lists
 


@@ -30,9 +58,13 @@ description: Docs and comment rules. Use before you write any prose.
 ## Comments
 
 - A comment explains the declaration that it belongs to: the contract,
 ## Comments
 
 - A comment explains the declaration that it belongs to: the contract,

-  invariants, and non-obvious reasons. Do not repeat what the code shows
-  or describe unrelated caller behavior.
-- Update or remove a comment when its code changes.
+  invariants, non-obvious reasons, and required compatibility behavior.
+  Do not repeat what the code shows or describe unrelated caller
+  behavior.
+- State input constraints, results, errors, and concurrency or resource
+  ownership rules when a caller needs them.
+- Keep a comment next to the code that it describes. Update or remove a
+  comment when its code changes.

 - Write prose comments as continuous paragraphs, with one space between
   words and sentences. Do not use semicolons in prose.
 - Reflow each changed comment paragraph with `bin/reflow`. Do not reflow
 - Write prose comments as continuous paragraphs, with one space between
   words and sentences. Do not use semicolons in prose.
 - Reflow each changed comment paragraph with `bin/reflow`. Do not reflow


@@ -45,3 +77,9 @@ This example reflows the comment on lines 12 to 15:
 ```sh
 bin/reflow main.go 12 15
 ```
 ```sh
 bin/reflow main.go 12 15
 ```

+
+## Go comments
+
+- Follow [Go Doc Comments](https://go.dev/doc/comment). Write complete
+  sentences. Start a declaration comment with the name of the symbol.
+- Document each exported declaration and its contract.

diff --git a/.agents/skills/go-code/SKILL.md b/.agents/skills/go-code/SKILL.md
index 374c2e932d810090dc29af95043f7b20f0490e12..59fd54453a676105491a0bfb95695a26af4afa76 100644 (file)
--- a/.agents/skills/go-code/SKILL.md
+++ b/.agents/skills/go-code/SKILL.md
@@ -33,6 +33,9 @@ description: Go style rules. Use before you write or review Go code.
   them. Order functions by first use within their declaration group, not
   across groups. Entry points do not go before types or constructors
   merely because they are entry points.
   them. Order functions by first use within their declaration group, not
   across groups. Entry points do not go before types or constructors
   merely because they are entry points.

+- Group related constants in one `const (...)` block and related
+  variables in one `var (...)` block. Put an empty line between entries
+  that have comments.

 
 ## Entry point and context
 
 
 ## Entry point and context
 


@@ -68,6 +71,11 @@ description: Go style rules. Use before you write or review Go code.
 - Use `errors.Is` and `errors.As` to examine errors. Do not compare an
   error directly with a sentinel, because errors can be wrapped.
   `errorlint` checks comparisons and type assertions on errors.
 - Use `errors.Is` and `errors.As` to examine errors. Do not compare an
   error directly with a sentinel, because errors can be wrapped.
   `errorlint` checks comparisons and type assertions on errors.

+- Do not match the text of an error from another package. When an
+  external fault leaves no other way, test it through the real code
+  path, so that a change of the text fails a test.
+- Handle a documented sentinel error explicitly when it does not show a
+  failure.

 - When a function has more than one error path, wrap each returned error
   with the context of its operation. A function with one error path can
   return the error directly. `wrapcheck` checks package and interface
 - When a function has more than one error path, wrap each returned error
   with the context of its operation. A function with one error path can
   return the error directly. `wrapcheck` checks package and interface


@@ -76,7 +84,7 @@ description: Go style rules. Use before you write or review Go code.
 - When deferred cleanup can fail, use a named `err` result and join the
   cleanup error with `errors.Join`.
 
 - When deferred cleanup can fail, use a named `err` result and join the
   cleanup error with `errors.Join`.
 

-## Logs
+## Logs and cancellation

 
 - Log with the `*slog.Logger` of the part (`daemonLogger`). The handler
   formats the line; do not format values into the message.
 
 - Log with the `*slog.Logger` of the part (`daemonLogger`). The handler
   formats the line; do not format values into the message.


@@ -89,6 +97,12 @@ description: Go style rules. Use before you write or review Go code.
 - Use info for normal events, warn for a failure that is retried or
   loses nothing, and error for a failure that loses data or stops a
   part.
 - Use info for normal events, warn for a failure that is retried or
   loses nothing, and error for a failure that loses data or stops a
   part.

+- Cancellation of a service or request context is an expected stop, not
+  a failure. Do not log it as an error, and do not hide all errors only
+  because the context is canceled. A joined error with an independent
+  failure stays a failure.
+- Use the worker or request context for this check. An operation that
+  reaches its own timeout while its worker is active stays a failure.

 - Never log secrets, tokens, message text, or phone numbers. Redact IDs
   as the existing code does.
 
 - Never log secrets, tokens, message text, or phone numbers. Redact IDs
   as the existing code does.
 


@@ -104,14 +118,21 @@ description: Go style rules. Use before you write or review Go code.
   table-driven test. A small type of two fields that acts as one value,
   such as a key or a coordinate pair, can use a positional literal on
   one line.
   table-driven test. A small type of two fields that acts as one value,
   such as a key or a coordinate pair, can use a positional literal on
   one line.

-- Put an empty line between logical blocks and after multiline
-  declarations, assignments, literals, and calls. The only exception
-  after a multiline construct is its immediate error check. Separate a
-  multiline literal from a following loop, even when the loop uses that
-  literal. Keep a single-line declaration with its immediate use, and an
-  operation with its immediate error check. This does not waive the
-  separator after a multiline construct. `wsl_v5` and `nlreturn` check
-  only part of this rule. Review the other boundaries by hand.
+- Put an empty line between logical blocks, such as two independent `if`
+  statements. Put an empty line before and after loops, switches,
+  selects, standalone blocks, and multiline declarations, assignments,
+  literals, and calls, when there is an adjacent statement. These
+  boundaries also apply when a loop immediately uses the declaration
+  before it.
+- The only exception after a multiline construct is its immediate error
+  check. Keep a single-line declaration with its immediate use, and an
+  operation with its immediate error check. A result assertion does not
+  waive the separator after a multiline construct. `wsl_v5` and
+  `nlreturn` check only part of these rules. Review the other boundaries
+  by hand.
+- Group imports in this order: standard library, external dependencies,
+  project imports. Review these groups by hand when the formatter does
+  not check them.

 - Use GNU-style command flags: two dashes for long flags, such as
   `--cwd`, and one dash only for one-letter flags, such as `-h`.
 - Prefer the Go standard library. Prefer Go over cgo.
 - Use GNU-style command flags: two dashes for long flags, such as
   `--cwd`, and one dash only for one-letter flags, such as `-h`.
 - Prefer the Go standard library. Prefer Go over cgo.


@@ -127,8 +148,9 @@ description: Go style rules. Use before you write or review Go code.
 
 ## Tests
 
 
 ## Tests
 

-- Add tests for meaningful behavior. Do not add tests that only repeat
-  implementation details.
+- Add tests for behavior changes. Do not add tests that only repeat
+  implementation details, and do not add a test file without behavior
+  worth a test.

 - Use `testify/require` for assertions. `testifylint` checks the
   assertion forms.
 - Name a table-driven case slice `testCases` and its range variable
 - Use `testify/require` for assertions. `testifylint` checks the
   assertion forms.
 - Name a table-driven case slice `testCases` and its range variable


@@ -151,4 +173,4 @@ description: Go style rules. Use before you write or review Go code.
   that more than one test file uses in a file without tests. Name that
   file for what it holds, and end the name with `_helper_test.go`.
 - Supply valid required dependencies in tests. Do not make a dependency
   that more than one test file uses in a file without tests. Name that
   file for what it holds, and end the name with `_helper_test.go`.
 - Supply valid required dependencies in tests. Do not make a dependency

-  optional only to make a test simpler.
+  optional or add a nil fallback only to make a test simpler.