feat(php): add a PHP skill, grounded in the synchra-php SDK and the bleichi site
PHP was the one language with real code in two shapes here and no skill: a Composer library held at PHPStan level max, and a plain PHP-FPM application behind nginx with no framework. The rules come from reading both rather than from general PHP advice, so each one names the failure it prevents. The two halves differ enough to be separate sections. A library requires PSR interfaces and ships no implementation; an application's rules are about escaping, keeping the document root unwritable, atomic writes and sessions. Everything in §1-§8 applies to both. Three findings are worth the space they take because they were each paid for once already: `json_encode` with JSON_UNESCAPED_SLASHES into an inline <script> is an XSS hole, a web user that cannot write its own cache fails silently on every request, and inside the dev container the host's own `php` bridge shadows the real interpreter so every vendor/bin tool dies naming distrobox. docs(python): cover MCP servers, Red cogs and HA components, and correct three rules the owner's own code contradicts The skill described a toolchain and was silent on the four things most of this Python actually is. Three of its rules are also wrong in context, and the code is right: getLogger(__name__) puts a cog outside the red.* tree where Red's log controls apply; TaskGroup's cancel-on-first-error is the wrong default for fan-out over N providers, where the real pattern is a semaphore plus gather(return_exceptions=True); and the gate runs locally and through pre-commit, not through CI, which never runs under this account. Also: ruff ALL-minus-documented-ignores is the preferred shape rather than the explicit select list, lockfiles split by artifact type, and bare `python` and bare `pip` are different interpreters on this host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1b688d128c
commit
5aef9707d2
5 files changed
+694
-24
No files matched your search
@@ -19,7 +19,8 @@ A collection of public, reusable agent skills, cheatsheets, and operational work
|
||||
| **Rust** | [`rust.md`](skills-public/code-languages/rust.md) | `--workspace` clippy gate, `[workspace.lints]` single source of truth, no `unwrap`/`unsafe`, core-crate split |
|
||||
| **TypeScript / JavaScript** | [`js-typescript.md`](skills-public/code-languages/js-typescript.md) | TS > JSDoc > plain JS, banned constructs table, React & hooks rules, resource lifecycle, sandbox boundaries |
|
||||
| **Bash / Shell** | [`bash.md`](skills-public/code-languages/bash.md) | `set -euo pipefail`, shellcheck, no wait loops, destructive-command guards, build-gate structure |
|
||||
| **Python** | [`python.md`](skills-public/code-languages/python.md) | `mypy --strict`, broad ruff rule set, asyncio task discipline, rate-limit queueing |
|
||||
| **Python** | [`python.md`](skills-public/code-languages/python.md) | `mypy --strict`, broad ruff rule set, asyncio task discipline, rate-limit queueing, MCP/cog/HA framework rules |
|
||||
| **PHP** | [`php.md`](skills-public/code-languages/php.md) | `strict_types` everywhere, PHPStan `level: max`, PER-CS 2.0 risky rules, `final readonly` value objects, PSR interfaces over implementations, escaping & unwritable document root |
|
||||
| **Go** | [`go.md`](skills-public/code-languages/go.md) | strict `golangci-lint` set, concrete types over `interface{}`, context everywhere, CGO-free static builds |
|
||||
| **C# / .NET** | [`csharp-dotnet.md`](skills-public/code-languages/csharp-dotnet.md) | `TreatWarningsAsErrors`, nullable enabled, no `.Result`, trimming/single-file publish caveats, SDK shims |
|
||||
| **C / C++** | [`c-cpp.md`](skills-public/code-languages/c-cpp.md) | full warning set + `-Werror`, sanitizers, RAII/Rule of Zero, target-based CMake, FFI ownership |
|
||||
|
||||
+123
-3
@@ -4,8 +4,10 @@ description: >-
|
||||
Use this skill whenever invoking a command-line tool, choosing between a CLI and an ad-hoc
|
||||
script, writing a standalone utility script, or orienting on unfamiliar work before starting.
|
||||
Covers discovering a CLI's real interface instead of guessing flags, non-interactive and
|
||||
machine-readable invocation, script placement and symlinking, and care around destructive
|
||||
or outward-facing commands.
|
||||
machine-readable invocation, checking an API's payload rather than its exit status, keeping
|
||||
credentials out of argv, eventually-consistent systems that briefly report the old state,
|
||||
script placement and symlinking, and care around destructive or outward-facing commands —
|
||||
including additive changes that silently take over a name already serving traffic.
|
||||
---
|
||||
|
||||
# CLI & Tooling — Discovery, Invocation, and Script Placement
|
||||
@@ -83,6 +85,16 @@ tool list -o csv # structured columns for a spreadsheet or script
|
||||
|
||||
- **Never scrape a table meant for humans.** Column widths, truncation, colour codes, and
|
||||
alignment all change without notice; a structured format is a contract.
|
||||
- **Check the payload, not just the exit status.** Plenty of HTTP APIs answer `200 OK` with
|
||||
`{"success": false, "errors": [...]}` in the body, and `curl` exits `0` because the
|
||||
*request* worked. A script that trusts the exit code reports a clean success and writes
|
||||
nothing. Assert on the field the API uses for this, and print `errors[]` when it trips:
|
||||
```bash
|
||||
if [[ "$(jq -r '.success' <<<"$response")" != true ]]; then
|
||||
jq -r '.errors[]? | " - \(.message)"' <<<"$response" >&2
|
||||
exit 1
|
||||
fi
|
||||
```
|
||||
- Strip colour explicitly (`--color never`, `NO_COLOR=1`) when capturing output, or escape
|
||||
sequences end up in your parsed values.
|
||||
- Keep in mind that a "compact" or summary format is the right default for feeding an agent,
|
||||
@@ -166,9 +178,117 @@ system, not scattered at the repository root.
|
||||
- Anything that sends, publishes, deploys, or purchases is outward-facing: confirm first, and
|
||||
treat approval as applying to that one action rather than the category.
|
||||
|
||||
### Adding something can take something over
|
||||
|
||||
> [!CAUTION]
|
||||
> **A change that deletes nothing can still break a live service.** The dangerous operations
|
||||
> are not only the ones named `delete`. Adding a routing rule, a proxy entry, a redirect, a
|
||||
> DNS override or a higher-priority config can silently intercept a name that is already
|
||||
> serving traffic — while the thing it shadows still looks perfectly healthy in every
|
||||
> listing, because it was never touched.
|
||||
>
|
||||
> That is the worst shape a failure can take: the old config is still there, still correct,
|
||||
> still pointing where it should, and the service is dead anyway. Nothing in the inventory
|
||||
> says what happened.
|
||||
|
||||
**Before claiming a name, ask what currently answers to it** — and refuse by default when
|
||||
something does:
|
||||
|
||||
```bash
|
||||
# Only an unrouteable placeholder, or an entry this tool made, is safe to assume is free.
|
||||
if [[ "$(current_target "$host")" != "$PLACEHOLDER" && "$(owner "$host")" != "$MARKER" ]]; then
|
||||
[[ "$FORCE" == true ]] || die "$host already serves $(current_target "$host") — pass --force"
|
||||
fi
|
||||
```
|
||||
|
||||
Two things make that check possible:
|
||||
|
||||
- **Mark what you create.** A comment field, a tag, a description prefix — anything that
|
||||
distinguishes *your* entries from ones a human made. It turns "is this mine to replace?"
|
||||
into a lookup rather than a guess, and it is what lets the matching `--delete` refuse to
|
||||
remove something it did not create.
|
||||
- **Make `--force` the only way past**, and name in the refusal what is there now. A tool that
|
||||
silently overwrites is one nobody can safely run against production.
|
||||
|
||||
> [!TIP]
|
||||
> Test such a tool against a name you **expect** it to reject, not only against a free one.
|
||||
> The rejection path is where the damage would have been, and it is the half a happy-path test
|
||||
> never exercises.
|
||||
|
||||
---
|
||||
|
||||
## 7. Environment Constraints
|
||||
## 7. Credentials Belong on stdin, Not in `argv`
|
||||
|
||||
> [!WARNING]
|
||||
> **`argv` is world-readable.** Any user on the box can run `ps` and read the full command
|
||||
> line of every process. A token passed as `-H "Authorization: Bearer $TOKEN"`, `--password`,
|
||||
> or `--token` is therefore exposed for as long as the call runs — and lands in shell history
|
||||
> and process accounting too.
|
||||
|
||||
Most CLIs that accept a secret as a flag also accept it somewhere safer. Use that instead:
|
||||
|
||||
```bash
|
||||
# WRONG — the token is visible in `ps` for the life of the request
|
||||
curl -H "Authorization: Bearer $TOKEN" "$url"
|
||||
|
||||
# RIGHT — curl reads its options from stdin; argv holds nothing sensitive
|
||||
printf 'header = "Authorization: Bearer %s"\n' "$TOKEN" | curl --config - "$url"
|
||||
```
|
||||
|
||||
The equivalent elsewhere: `--password-stdin` (docker login, many package tools), `-u` with a
|
||||
`.netrc`, `--token-file`, or simply an environment variable the tool reads by name. Reach for
|
||||
the env var when nothing better exists — it is readable through `/proc/<pid>/environ` but only
|
||||
by the owner and root, which is a meaningfully smaller exposure than `argv`.
|
||||
|
||||
See [`code-languages/bash.md` §8](skills-public/code-languages/bash.md) for the surrounding rules on
|
||||
where secrets are stored and why they are never echoed.
|
||||
|
||||
---
|
||||
|
||||
## 8. Eventually-Consistent Systems Lie Briefly
|
||||
|
||||
Edge networks, DNS, CDNs, object stores, container registries and clustered schedulers all
|
||||
acknowledge a write before it is visible everywhere. **A read taken immediately after a
|
||||
successful write is not evidence of anything.**
|
||||
|
||||
The failure mode is believing the tool is broken and "fixing" it:
|
||||
|
||||
- A changed rule still returns the **old** value for a while — which reads as "my write was
|
||||
ignored".
|
||||
- A brand-new name returns an **error** for a while — a 5xx, an NXDOMAIN, a 404 — which reads
|
||||
much worse than it is.
|
||||
|
||||
So: take the write's own success response as the result, and treat the first few reads as
|
||||
unreliable. When a verification read matters, **say how long propagation takes** in the tool's
|
||||
own help or header comment, so the next person does not rediscover it at 2am. State it as a
|
||||
number — "~30s for a rule, ~40s for a new name" — because "eventually" tells nobody when to
|
||||
start worrying.
|
||||
|
||||
Do not wrap this in a polling loop (see
|
||||
[`code-languages/bash.md` §3](skills-public/code-languages/bash.md)); take one sample per
|
||||
invocation and let the caller run it again.
|
||||
|
||||
> [!WARNING]
|
||||
> **Checking too early can make the delay much worse.** A failed lookup is itself cached. Query
|
||||
> a DNS name before it exists and the resolver stores the `NXDOMAIN` for the SOA's negative TTL
|
||||
> — commonly 300-1800s — so the name stays invisible to *you* long after it is live for
|
||||
> everyone else. Polling during propagation is what creates this, which is a second reason not
|
||||
> to loop.
|
||||
>
|
||||
> When a check does come back negative, confirm where the negative lives before concluding
|
||||
> anything: query the authoritative source directly and compare. The answer is often "the
|
||||
> record is fine, my resolver is lying", and flushing the local cache will not help if the
|
||||
> stale entry is on an upstream resolver you do not control.
|
||||
>
|
||||
> ```bash
|
||||
> dig +short @1.1.1.1 "$name" # authoritative-ish: is it actually published?
|
||||
> dig +short "$name" # what my resolver believes
|
||||
> curl --resolve "$name:443:<ip>" "https://$name/" # bypass DNS entirely to test the service
|
||||
> ```
|
||||
|
||||
---
|
||||
|
||||
## 9. Environment Constraints
|
||||
|
||||
- **Entering a container or changing shells resets the working directory.** Chain `cd` inside
|
||||
the same command string rather than relying on it persisting:
|
||||
|
||||
@@ -0,0 +1,402 @@
|
||||
---
|
||||
name: php
|
||||
description: >-
|
||||
Use this skill when writing, linting, analysing, testing, or releasing PHP — libraries,
|
||||
Composer packages, generated SDKs, and plain PHP-FPM web applications behind nginx. Covers
|
||||
strict_types, PHPStan at max, PER-CS 2.0 with risky rules, final readonly value objects,
|
||||
backed enums, PSR interfaces over implementations, output escaping, and PHPUnit strictness.
|
||||
---
|
||||
|
||||
# PHP — Rules & Preferences
|
||||
|
||||
Extends [`coding-dev.md`](skills-public/coding-dev.md). Everything there applies; this adds PHP specifics.
|
||||
|
||||
PHP here means one of two things, and they differ: a **library** published through Composer,
|
||||
which depends on PSR interfaces and ships no implementation, and a **plain PHP-FPM application**
|
||||
behind nginx with no framework, where the filesystem is the router. Both carry §1–§8; §9 is the
|
||||
library half and §10 the application half.
|
||||
|
||||
---
|
||||
|
||||
## 1. The Gate
|
||||
|
||||
```bash
|
||||
composer lint # php-cs-fixer fix --dry-run --diff
|
||||
composer stan # phpstan analyse
|
||||
composer test # phpunit
|
||||
composer check # all three, in that order
|
||||
```
|
||||
|
||||
Define those four as `scripts` in `composer.json` so the gate has one name. `composer check`
|
||||
must be clean before anything is called done — zero fixer diffs, zero PHPStan errors, zero
|
||||
test failures, and no warnings, notices or deprecations (see §7).
|
||||
|
||||
- Use `@php tools/x.php` rather than `php tools/x.php` in a composer script: it runs the
|
||||
interpreter Composer itself is running under, which matters on a machine with several.
|
||||
- `composer test` must never touch the network. Point `defaultTestSuite` at the unit suite so
|
||||
a bare run is safe to fire blind.
|
||||
- Name tool configs `.dist` — `phpstan.neon.dist`, `phpunit.xml.dist`, `.php-cs-fixer.dist.php` —
|
||||
so a developer can drop a local override beside them without dirtying the repo. Add the
|
||||
caches (`.php-cs-fixer.cache`, `.phpunit.cache`) to `.gitignore`.
|
||||
- Lockfile by artifact type: an **application commits `composer.lock`**, a **library ignores
|
||||
it**. Getting this backwards shows up in every diff.
|
||||
|
||||
---
|
||||
|
||||
## 2. There Is No PHP On The Host
|
||||
|
||||
On this workstation PHP is not installed on the host at all. `~/.local/bin/php` is a three-line
|
||||
bridge that execs into the development container, and **`composer` has no bridge** — see
|
||||
[`distrobox-buildbox-dev.md`](skills-private/distrobox-buildbox-dev.md) for the container, and
|
||||
[`coding-dev.md` §13](skills-public/coding-dev.md) for why toolchains never land on an immutable
|
||||
host root.
|
||||
|
||||
```bash
|
||||
# php works from the host. composer does not — go through the container, and chain the cd,
|
||||
# because entering one resets the working directory.
|
||||
distrobox enter <container> -- bash -c "cd /abs/path/to/project && composer check"
|
||||
```
|
||||
|
||||
> [!CAUTION]
|
||||
> **Inside the container, prepend `PATH=/usr/bin:/bin`.** The host `~/.local/bin` is on PATH
|
||||
> ahead of `/usr/bin` there, so the host's own `php` bridge shadows the container's real
|
||||
> interpreter and dies with `/usr/bin/distrobox: No such file or directory`. Every
|
||||
> `vendor/bin/*` tool is a PHP script, so `vendor/bin/phpstan` fails this way and the error
|
||||
> names distrobox rather than anything to do with PHP. `php vendor/bin/phpstan` with the fixed
|
||||
> PATH works.
|
||||
|
||||
Edit PHP sources with `Read`/`Edit`/`Write`, not with `sed` or a `php -r` patch script. The
|
||||
nested quoting inside `distrobox enter … bash -c "…"` mangles `$`, backslashes and quotes in
|
||||
exactly the places PHP uses them, and a half-applied patch script reports success.
|
||||
|
||||
---
|
||||
|
||||
## 3. Strictness — `strict_types` In Every File, PHPStan At Max
|
||||
|
||||
```php
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
```
|
||||
|
||||
**Every file, with no exceptions**, enforced mechanically by the fixer's `declare_strict_types`
|
||||
rule rather than by review. Without it a `"7"` from an API quietly becomes `int 7` and a
|
||||
hydrator whose whole purpose is to fail loudly on drift stops failing at all.
|
||||
|
||||
```neon
|
||||
# phpstan.neon.dist
|
||||
parameters:
|
||||
level: max
|
||||
paths:
|
||||
- src
|
||||
- tests
|
||||
- tools
|
||||
- examples
|
||||
treatPhpDocTypesAsCertain: false
|
||||
```
|
||||
|
||||
- **Analyse tests, tools and examples too**, not just `src`. Those are the three places authors
|
||||
assume are exempt, and they are where a convention actually breaks first.
|
||||
- `treatPhpDocTypesAsCertain: false` stops PHPStan trusting a PHPDoc that is narrower than the
|
||||
signature, so genuine runtime `is_string`/`is_array` guards are not reported as always-true.
|
||||
- **No baseline and no `ignoreErrors`.** Level max is reachable with standard tags and real
|
||||
guards; a baseline is a list of things nobody will ever go back to.
|
||||
- A suppression, when genuinely needed, is the narrowest `@phpstan-ignore-next-line` with the
|
||||
reason on it. Reaching for `@phpstan-*` or `@psalm-*` to describe a *type* is a smell — the
|
||||
standard tags in §5 carry everything level max needs.
|
||||
- **Never relax a strictness flag without an adjacent comment saying why.** This is the house
|
||||
habit across every config file in the repo, and it is what makes a relaxation reviewable
|
||||
instead of load-bearing folklore.
|
||||
|
||||
---
|
||||
|
||||
## 4. Formatting — PER-CS 2.0, Risky Rules On
|
||||
|
||||
```php
|
||||
// .php-cs-fixer.dist.php
|
||||
->setRiskyAllowed(true)
|
||||
->setRules([
|
||||
'@PER-CS2.0' => true,
|
||||
'@PHP82Migration' => true,
|
||||
'declare_strict_types' => true,
|
||||
'strict_param' => true,
|
||||
'strict_comparison' => true,
|
||||
'global_namespace_import' => ['import_classes' => false, 'import_functions' => false],
|
||||
'no_unused_imports' => true,
|
||||
'ordered_imports' => ['sort_algorithm' => 'alpha'],
|
||||
'php_unit_test_case_static_method_calls' => ['call_type' => 'self'],
|
||||
])
|
||||
```
|
||||
|
||||
php-cs-fixer and PHPStan, not Pint and not Psalm. The fixer config does more normative work
|
||||
than the prose does: these rules are where five of the conventions below stop being review
|
||||
items.
|
||||
|
||||
- `strict_comparison` + `strict_param` mean every comparison is `===`/`!==` and every
|
||||
`in_array()` carries its third argument. Loose `in_array` matches numeric strings.
|
||||
- `global_namespace_import => false` is why every global function, constant and class is written
|
||||
with a leading backslash: `\sprintf`, `\JSON_THROW_ON_ERROR`, `\DateTimeImmutable`,
|
||||
`\RuntimeException`. Inside a namespace an unqualified call makes PHP look up the namespaced
|
||||
name first and fall back to global — a per-call cost, and a hazard the day something defines
|
||||
`App\json_decode`.
|
||||
- Guard a fixer API that newer releases added, rather than calling it unconditionally and
|
||||
breaking the lint step on the oldest version `composer.json` still allows:
|
||||
`if (\method_exists($config, 'setUnsupportedPhpVersionAllowed')) { … }`.
|
||||
|
||||
---
|
||||
|
||||
## 5. Banned Constructs
|
||||
|
||||
| Never | Use instead |
|
||||
| :--- | :--- |
|
||||
| `==` / `!=` | `===` / `!==` (the fixer enforces it) |
|
||||
| `in_array($x, $a)` | `\in_array($x, $a, true)` |
|
||||
| `stdClass` from `json_decode` | `\json_decode($b, true, 512, \JSON_THROW_ON_ERROR)` into `array<string, mixed>`, then a typed model |
|
||||
| `json_last_error()` checks | `\JSON_THROW_ON_ERROR` on every encode and decode |
|
||||
| `\DateTime` | `\DateTimeImmutable` — a mutable date in a `readonly` class is a hole in the guarantee |
|
||||
| `setX()` on a value object | `withX()` returning a new instance |
|
||||
| `http_build_query()` | a builder that drops nulls, spells booleans `true`/`false`, and repeats a key instead of emitting `k[0]=` |
|
||||
| `extract()`, `eval()`, `$$var` | named variables |
|
||||
| `@` error suppression | check the return value |
|
||||
| `array()` long syntax | `[]` |
|
||||
| A pure `enum` for a wire value | a backed `enum X: string` (§6) |
|
||||
| `echo $x` in HTML | `\htmlspecialchars($x, \ENT_QUOTES, 'UTF-8')` (§10) |
|
||||
| Unqualified `strlen`/`count`/`sprintf` inside a namespace | `\strlen`, `\count`, `\sprintf` |
|
||||
|
||||
- **PHPDoc only says what the signature cannot**: array shapes, generics, `class-string`,
|
||||
`list<T>`, `@template`, `@implements`, `@return never`. A `@param string $x` restating a
|
||||
declared scalar is a second source of truth that will drift from the first.
|
||||
- **`@return never` in a docblock over a native `: mixed`** is the deliberate exception: it
|
||||
gives PHPStan the unreachability while still letting the helper sit in a ternary tail
|
||||
position (`return \is_string($v) ? $v : $this->fail(…);`), which a native `: never` forbids.
|
||||
|
||||
---
|
||||
|
||||
## 6. Types — `final readonly` By Default
|
||||
|
||||
- **`final readonly class` is the default shape** for anything carrying data. Plain
|
||||
`final class` only for things that memoise, hold mutable state, or are pure behaviour.
|
||||
`readonly` on the class is what makes "models are immutable, build a new one" enforced by the
|
||||
engine instead of promised in a docblock.
|
||||
- **Constructor property promotion for every property**, with `private readonly` written out
|
||||
explicitly even inside a `final` class. The one property declared the long way should be the
|
||||
one the constructor has to run through a normaliser — promotion cannot transform a value, so
|
||||
that asymmetry is a readable signal rather than an inconsistency.
|
||||
- **`new` in a parameter default**, not nullable-plus-`??`: `private readonly LoggerInterface
|
||||
$logger = new NullLogger()`. The signature then states the real type and PHPStan never sees a
|
||||
nullable to guard. Keep `?X $x = null` only where a *static factory* forwards the argument and
|
||||
"the caller said nothing" has to stay distinguishable.
|
||||
- **Named arguments** wherever a call has more than two or three arguments or skips one. A
|
||||
wide constructor built positionally silently mis-wires the day two same-typed fields swap
|
||||
order.
|
||||
- **First-class callable syntax** (`Model::fromArray(...)`) over a `'Model::fromArray'` string —
|
||||
it is checked at compile time. Mark a closure that needs no binding `static`, so it cannot
|
||||
capture `$this` into a long-lived callable and keep the owner alive.
|
||||
- Model properties may keep the **API's own `snake_case` names** rather than being renamed to
|
||||
camelCase. A rename table has to be maintained in both directions, silently drops whatever
|
||||
the mapping forgot, and makes a devtools capture stop matching the code.
|
||||
|
||||
---
|
||||
|
||||
## 7. Enums & Wire Values
|
||||
|
||||
- **Backed enums only** (`enum Provider: string`). A backed case is what lets one branch
|
||||
collapse any enum to its wire form and a generic reader be typed
|
||||
`class-string<T of \BackedEnum>`.
|
||||
- **An unrecognised wire value throws**, naming the model, the field, the value, the enum class
|
||||
and the remedy. `::from()` raises a bare `\ValueError` with no field name; an `Unknown`
|
||||
fallback case hands the caller a value the API's own type does not have and defers the failure
|
||||
to someone else's `match`.
|
||||
- Where a field genuinely carries any JSON value, type it **`mixed` and explain it in prose**. A
|
||||
PHPDoc union nothing actually validates is a lie PHPStan will then trust.
|
||||
|
||||
---
|
||||
|
||||
## 8. Errors & Exceptions
|
||||
|
||||
- **The root of the hierarchy is an interface**, `interface AppException extends \Throwable {}`,
|
||||
not a base class. Callers can then `catch (AppException $e)` without catching unrelated
|
||||
`\RuntimeException`s, while each exception still extends the most apt SPL class — which an
|
||||
abstract base makes impossible the moment two of them want different parents.
|
||||
- One **static factory mapping status to class** (`match (true)` over 400/401/403/404/409/422/
|
||||
429/`>= 500`). Declare the base's constructor `final public function __construct` so `new
|
||||
$class(…)` in that factory stays safe.
|
||||
- Carry the **parsed error envelope, the raw body, and the request method and URI** on the
|
||||
exception, and truncate the raw body — a 2 MB proxy error page does not belong in a log line.
|
||||
Document that callers branch on an error *code*, never on the message, which is prose.
|
||||
- **A failure to parse the error body must never mask the real status.** A 502 from a load
|
||||
balancer returns HTML; if the envelope parser throws, the caller sees a JSON parse error
|
||||
instead of "the service is down". Return null from the parser on every one of those paths.
|
||||
- **Retry only idempotent methods** — `GET`, `HEAD`, `OPTIONS`, `PUT`, `DELETE`. POST and PATCH
|
||||
need an explicit opt-in, because a timeout on a request that actually succeeded is how a
|
||||
giveaway gets drawn twice. Honour `Retry-After` over the computed backoff, and inject the
|
||||
sleeper so the retry test does not take 30 seconds of wall clock.
|
||||
|
||||
---
|
||||
|
||||
## 9. Libraries — PSR Interfaces, Not Implementations
|
||||
|
||||
Follow the global ordering (reuse → fork → write, [`coding-dev.md` §3](skills-public/coding-dev.md)).
|
||||
For a published package specifically:
|
||||
|
||||
- **Require PSR interfaces and ship no implementation.** `psr/http-client` (18),
|
||||
`psr/http-factory` (17), `psr/http-message` (7), `psr/log` (3), plus `php-http/discovery`.
|
||||
Guzzle and a PSR-7 implementation go in `require-dev` with a `suggest` entry saying any
|
||||
implementation works. Pinning a client forces a version on every consuming application.
|
||||
- Widen the constraint on an interface package that has had a major (`"psr/http-message":
|
||||
"^1.1 || ^2.0"`) so a consumer's choice does not conflict with yours.
|
||||
- **Wrap the PSR client behind your own one-method transport interface.** That is what lets the
|
||||
whole unit suite run against a fake and assert on exact bytes, instead of building PSR-7
|
||||
mocks in every test.
|
||||
- Convert a discovery miss into your own configuration exception whose message says what to
|
||||
`composer require`.
|
||||
- **Secrets enter through an interface**, never a string parameter: a token provider whose
|
||||
implementation reads the environment, with a null return treated as legitimate because some
|
||||
endpoints are public. A token passed as a constructor literal lands in every stack trace and
|
||||
`var_dump`. See [`coding-dev.md` §10](skills-public/coding-dev.md) for the general rule.
|
||||
- **Partial updates**: distinguish "field absent" from "field is null" per field. Serialise a
|
||||
fetched model, PATCH it back, and a blanket `array_filter(…, fn($v) => $v !== null)` wipes
|
||||
every server-side value the caller never mentioned — while the opposite default breaks the
|
||||
fields for which null *is* the value.
|
||||
- Accept an `int` where the schema says `number`. JSON has one number type, so a whole number
|
||||
arrives as an int and strict `is_float()` rejects every such value. This wants a named
|
||||
regression test.
|
||||
|
||||
**Generated code** lives in directories that are wiped and rewritten whole, never overwritten
|
||||
file by file — otherwise a deleted endpoint lingers as a class that still compiles and 404s.
|
||||
Every generated file opens with a banner carrying the exact regeneration command, generated and
|
||||
hand-written namespaces never share a directory, and **nothing is added to a generated
|
||||
namespace** — a method on a generated enum is deleted by the next run, so it belongs on a
|
||||
hand-written sibling instead.
|
||||
|
||||
---
|
||||
|
||||
## 10. Web Applications — Escaping, Writes, Sessions
|
||||
|
||||
A plain PHP-FPM app with no framework still gets **one `require __DIR__ . '/../vendor/autoload.php'`
|
||||
per entry point** and PSR-4 for everything else. Hand-rolled `require` graphs between
|
||||
application files are what produce double-inclusion and load-order bugs.
|
||||
|
||||
- **Entry points are three lines over a class.** Two copies of the same handler are how one of
|
||||
them loses its CSRF check.
|
||||
- **Compute first, echo second.** With no template engine the discipline is the separation: all
|
||||
logic above the `?>`, then markup with `<?= ?>` echoes of values already shaped. Logic mixed
|
||||
into markup is where an unescaped echo hides.
|
||||
- **Escape at every echo**, with the explicit form `\htmlspecialchars($t, \ENT_QUOTES, 'UTF-8')`.
|
||||
There are no trusted values.
|
||||
|
||||
> [!CAUTION]
|
||||
> **`json_encode` into an inline `<script>` is an XSS hole**, and `JSON_UNESCAPED_SLASHES`
|
||||
> opens it by removing the `<\/script>` escaping that would otherwise have saved you. Any
|
||||
> `</script>` or `<!--` inside the data closes the element and the rest is parsed as HTML. Use
|
||||
> `JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT`, or emit a
|
||||
> `<script type="application/json">` block and `JSON.parse` it.
|
||||
|
||||
- **The document root stays unwritable.** A web server that can create files in its own
|
||||
document root is one bug away from writing a `.php` there. Anything PHP writes goes under
|
||||
`sys_get_temp_dir()`; a file it must update in place gets permission on *the file*, not on
|
||||
the directory. If an upload directory must live under the root anyway, sniff content
|
||||
(`getimagesize()` plus an image-type→extension allowlist, never the uploaded name) **and**
|
||||
deny `.php` under it in the server config.
|
||||
- **Check the return value of every write.** Both halves of the uid trap recorded in this
|
||||
machine's memory were silent: a cache the web user could not create meant every page view
|
||||
re-scraped three third-party APIs, and an `.env` the web user could not read looked exactly
|
||||
like "no token configured".
|
||||
- **Atomic writes, two routes.** Write a temp file beside the target and `rename()` it, copying
|
||||
the victim's mode *and group* first — a rename-created file lands in PHP's group and locks
|
||||
everyone else out of editing it. Fall back to `r+` plus `flock(LOCK_EX)` plus `ftruncate`,
|
||||
never `w`, which truncates before the lock and happily creates a stray file at a typo'd path.
|
||||
Treat a short write as a failure.
|
||||
- **Optimistic concurrency by content hash**, not mtime — the mtime of a file on a network
|
||||
mount is not something to bet a save on. Validate the whole document before writing; a
|
||||
partial save is never attempted.
|
||||
- **Validate a filename against a listing**, not by sanitising it: `in_array($name,
|
||||
array_column($this->backups(), 'name'), true)`. Cleaning the string is how `../../.env` gets
|
||||
through, and for a path under a base directory only comparing `realpath()` results catches a
|
||||
symlink.
|
||||
- **Sessions in one place.** One guarded `session_start()`, an explicit `save_path` you know is
|
||||
writable (a session that silently fails to persist looks exactly like a login that silently
|
||||
fails), `use_strict_mode=1`, `use_only_cookies=1`, and an `httponly` + `samesite=Lax` +
|
||||
`secure` cookie. Decide `secure` by **defaulting to true and excepting loopback**: behind a
|
||||
terminating proxy `$_SERVER['HTTPS']` is empty on a real HTTPS visit, and reading
|
||||
`X-Forwarded-Proto` lets a forged header strip the flag. Expire by inactivity, which the client
|
||||
does not control, and `session_regenerate_id(true)` the moment a visitor gains privilege.
|
||||
- **Re-check authorisation per request**, never from an `is_admin` flag stamped into the session
|
||||
at login — otherwise revoking access means waiting for a session to expire. Request the minimum
|
||||
OAuth scope and discard the access token after use; storing it turns a session leak into an
|
||||
account compromise.
|
||||
- **Discard unsafe input rather than cleaning it**: a redirect target must start with `/`, must
|
||||
not start with `//`, and must have no control characters or backslashes; strip CR/LF from any
|
||||
header value built from a query string.
|
||||
- **An authenticated JSON API answers 401 with JSON**, not a 302 to an HTML login page, which is
|
||||
useless to `fetch()` and surfaces as a parse error. Every admin call is a POST carrying a CSRF
|
||||
token compared with `hash_equals` — **including the reads**, since a GET is reachable from
|
||||
another site's page — dispatched through `match` with a `default` that 400s.
|
||||
- **One response shape**, `{ok: bool, …}` with a snake_case `reason` code, and set the content
|
||||
type, `X-Content-Type-Options: nosniff` and the cache policy *before* anything can fail so the
|
||||
error path carries them too. Choose per endpoint whether upstream trouble is a 200 with
|
||||
`ok: false` (a public endpoint the page polls) or a real status code (a command channel).
|
||||
- **A multipart request has already been consumed into `$_POST`/`$_FILES`**, so check for the
|
||||
upload action *before* reading `php://input` — otherwise the generic JSON path reports "not
|
||||
valid JSON" for a body PHP ate.
|
||||
- **Every outbound call gets a timeout and a connect timeout**, `http_errors => false` so a 4xx
|
||||
body can be read for its reason, and redirects pinned to `https`. No timeout means a hung third
|
||||
party holding an FPM worker.
|
||||
- **A missing credential is a supported state**: separate "configured" from "not configured",
|
||||
answer with a machine-readable reason, and render a page saying it is not set up rather than a
|
||||
500 nobody can diagnose.
|
||||
|
||||
---
|
||||
|
||||
## 11. Testing
|
||||
|
||||
- **PHPUnit with attributes**, never docblock annotations: `#[CoversClass]`, `#[DataProvider]`,
|
||||
`#[Group]`. Annotations are deprecated in PHPUnit 11 and the config below turns that into a
|
||||
failure rather than a warning.
|
||||
- Turn on every escalation in `phpunit.xml.dist`: `failOnWarning`, `failOnNotice`,
|
||||
`failOnDeprecation`, `failOnRisky`, `failOnEmptyTestSuite`, `beStrictAboutOutputDuringTests`.
|
||||
`failOnSkipped="false"` is the one justified exception, because a suite that skips itself
|
||||
without credentials is the intended outcome on a machine without them — say so in an XML
|
||||
comment next to it.
|
||||
- Test classes are `final`; assertions are `self::assertSame(…)` (the fixer enforces it), while
|
||||
expectation setup, which is genuinely instance state, stays on `$this`.
|
||||
- Test method names are full sentences: `testAWrongTypeIsReportedRatherThanCoerced()`.
|
||||
- **Mock at the transport boundary with a fake that fails loudly.** Running out of queued
|
||||
responses throws, naming the method, the URI and how many were already served — a fake
|
||||
returning null turns "one more request than expected" into a type error three frames away.
|
||||
Record every send so a test can assert on exact bytes.
|
||||
- **Fixtures are sanitised real captures**, not hand-written JSON. A hand-written fixture
|
||||
encodes what the author *thinks* the API returns, so a drift test passes against an imagined
|
||||
payload. Keep shape, keys and enum values exactly; replace every identifier, handle, display
|
||||
name, avatar URL and message body with a deterministic placeholder so the diff stays stable.
|
||||
The raw capture stays out of the repository.
|
||||
- **A suite needing credentials is a separate testsuite**, tagged, not the default, and skips
|
||||
per test by re-reading the environment in a helper — so the skip message is attached to the
|
||||
test that needed it. `\getenv()` returns `false` when unset, so guard with `is_string` before
|
||||
`trim`. Everything in it is a read, because the only credentials anyone will run it with
|
||||
belong to something live. **Regression test every fixed bug.**
|
||||
- **Put the file and function size limits in a test**, since neither PHPStan nor the fixer has a
|
||||
length rule — see [`coding-dev.md` §4](skills-public/coding-dev.md). Measure function length
|
||||
by walking `\PhpToken::tokenize()`, not by counting braces in text: a `{` inside a string or a
|
||||
comment looks exactly like a block opener. Skip generated files, whose length is a property of
|
||||
the API surface. Carry over-limit files in an `ACCEPTED` map whose value is the written reason,
|
||||
and fail when a listed file drops back under so the entry gets removed.
|
||||
|
||||
---
|
||||
|
||||
## 12. CI
|
||||
|
||||
A PHP workflow runs `shivammathur/setup-php@v2` with `tools: composer:v2`, then
|
||||
`composer install --prefer-dist --no-interaction --no-progress`, then the gate in order: fixer
|
||||
dry-run, PHPStan, unit suite. Set `permissions: contents: read` explicitly. Put a
|
||||
credential-needing suite behind a `workflow_dispatch` input and pass the secret as `env:`,
|
||||
never as a command-line argument ([`cli-tooling.md` §7](skills-public/cli-tooling.md)).
|
||||
|
||||
> [!IMPORTANT]
|
||||
> **Under this account, only `workflow_dispatch` may be live.** Every automatic trigger —
|
||||
> `push`, `pull_request`, `schedule` — stays in the file but commented out, under a comment
|
||||
> saying why, so a fork restores CI by deleting `#` characters. See the GitHub Actions section
|
||||
> of the agent profile; a workflow written with `on: push` looks correct and never runs.
|
||||
|
||||
A job that reports upstream drift should `::warning::` rather than fail: the API moving is
|
||||
information, not a defect in this repository.
|
||||
+165
-19
@@ -2,8 +2,9 @@
|
||||
name: python
|
||||
description: >-
|
||||
Use this skill when writing, linting, type-checking, testing, or packaging Python —
|
||||
including async code, CLI tools, bot cogs, and library packages. Covers strict typing with
|
||||
mypy, ruff configuration, virtualenv discipline, async patterns, and packaging.
|
||||
including async code, CLI tools, MCP servers, Red-DiscordBot cogs, Home Assistant custom
|
||||
components, and library packages. Covers strict typing with mypy, ruff configuration,
|
||||
virtualenv discipline, async patterns, framework conventions, and packaging.
|
||||
---
|
||||
|
||||
# Python — Rules & Preferences
|
||||
@@ -11,6 +12,9 @@ description: >-
|
||||
Extends [`coding-dev.md`](skills-public/coding-dev.md). Everything there applies; this adds Python specifics.
|
||||
For the surrounding build and deploy scripts, see [`bash.md`](skills-public/code-languages/bash.md).
|
||||
|
||||
§1–§9 are the general rules. §10–§12 are the three frameworks most of this Python is written
|
||||
against, each of which overrides a general rule somewhere — and says so.
|
||||
|
||||
---
|
||||
|
||||
## 1. The Gate
|
||||
@@ -23,7 +27,16 @@ pytest
|
||||
```
|
||||
|
||||
All four clean before anything is called done. Configure them in `pyproject.toml` so the
|
||||
editor, the CLI, and CI all read the same settings.
|
||||
editor, the CLI, and the hooks all read the same settings.
|
||||
|
||||
**The gate runs locally and through `pre-commit`, not through CI** — workflows do not run under
|
||||
this account (see the GitHub Actions section of the agent profile), so a green repository with no
|
||||
local run means nothing was checked. A `.pre-commit-config.yaml` with `ruff-pre-commit`
|
||||
(check + format) and `mirrors-mypy` scoped `files: src` is where enforcement actually lives.
|
||||
|
||||
Scale the gate to what the code is: **a new owner-authored project gets all four**. A vendored
|
||||
fork gets lint only — match the surrounding style, keep the patch minimal and reviewable, and
|
||||
do not open a mass reformat of someone else's package (`coding-dev.md` §8 scope discipline).
|
||||
|
||||
---
|
||||
|
||||
@@ -47,6 +60,11 @@ disallow_untyped_defs = true
|
||||
- `@dataclass(frozen=True, slots=True)` for value objects.
|
||||
- Third-party library missing stubs: install `types-*` if it exists, otherwise a narrowly
|
||||
scoped `# type: ignore[import-untyped]` with a comment — never a blanket module-level ignore.
|
||||
For a **framework with no stubs at all**, the per-import ignore does not scale; use a named
|
||||
`[[tool.mypy.overrides]]` block listing those modules with `ignore_missing_imports = true`
|
||||
and a comment saying why, so the exemption is one reviewable list rather than scattered.
|
||||
- Worth adding beyond `strict`: `no_implicit_optional`, `warn_redundant_casts`,
|
||||
`warn_unused_ignores`, `warn_unused_configs`, `show_error_codes`, `allow_redefinition = false`.
|
||||
|
||||
### Value types over primitives
|
||||
|
||||
@@ -70,15 +88,32 @@ choices are:
|
||||
## 3. Linting
|
||||
|
||||
`ruff` is the linter and formatter (it replaces flake8, isort, black, pyupgrade, and most
|
||||
plugins). Enable a broad rule set rather than the default minimum:
|
||||
plugins). The preferred shape is **everything on, minus a documented ignore list**:
|
||||
|
||||
```toml
|
||||
[tool.ruff.lint]
|
||||
select = ["ALL"]
|
||||
ignore = [
|
||||
"COM812", # conflicts with the formatter
|
||||
"D203", "D213", # mutually exclusive with the docstring style chosen below
|
||||
"E501", # the formatter owns line length
|
||||
]
|
||||
```
|
||||
|
||||
The explicit set below is the floor, for a codebase where `ALL` would bury real findings:
|
||||
|
||||
```toml
|
||||
select = ["E", "F", "W", "I", "N", "UP", "B", "A", "C4", "SIM", "RUF", "ASYNC", "S", "PTH"]
|
||||
```
|
||||
|
||||
`S` (bandit security rules) and `ASYNC` are worth the noise. As always: fix, don't suppress —
|
||||
and when you must suppress, use a specific code (`# noqa: S603`) with a reason.
|
||||
- `S` (bandit security rules) and `ASYNC` are worth the noise. `PTH` keeps filesystem access on
|
||||
`pathlib`. Add `PLR0915` and `C901` as the function-size proxies (§8).
|
||||
- **Comment the ignore list too.** Every entry gets the reason beside it, exactly as a `# noqa`
|
||||
does — an uncommented ignore list is a set of rules nobody can tell were disabled on purpose.
|
||||
- As always: fix, don't suppress — and when you must suppress, use a specific code
|
||||
(`# noqa: S603`) with a reason.
|
||||
- Do not add `black` alongside ruff. Where both appear it is an inherited upstream toolchain,
|
||||
not a choice; ruff's formatter is the one to keep in new work.
|
||||
|
||||
---
|
||||
|
||||
@@ -88,8 +123,26 @@ and when you must suppress, use a specific code (`# noqa: S603`) with a reason.
|
||||
Never install into the system interpreter, and never `pip install --user` as a workaround
|
||||
for an immutable OS — use a container or a venv.
|
||||
- `uv` is the preferred installer/resolver where free choice exists; it is dramatically faster
|
||||
and lockfile-native.
|
||||
- Commit the lockfile. Pin the Python version in `pyproject.toml` (`requires-python`).
|
||||
and lockfile-native. **Check it exists before relying on it** (`command -v uv`) — a committed
|
||||
`uv.lock` is not evidence that the tool is installed on this machine.
|
||||
|
||||
> [!CAUTION]
|
||||
> **Bare `python` and bare `pip` can be different interpreters.** On this host `python` is the
|
||||
> system 3.14 while `pip`/`pip3` on PATH are bridge wrappers into the development container's
|
||||
> 3.13 — so `pip install x` followed by `python x.py` produces a `ModuleNotFoundError` that
|
||||
> looks like a bad install. Always go through a project venv, or `python -m pip` with the
|
||||
> interpreter named explicitly. See [`distrobox-buildbox-dev.md`](skills-private/distrobox-buildbox-dev.md).
|
||||
|
||||
- **Lockfile by artifact type**: an application commits its lock, a library does not. A
|
||||
throwaway script directory legitimately stays on a `requirements.txt` with floor pins and no
|
||||
lock at all.
|
||||
- Dev dependencies go in PEP 735 `[dependency-groups] dev = [...]`. `[tool.uv]
|
||||
dev-dependencies` is the older uv-specific form — fine to leave in a fork, not what new work
|
||||
uses.
|
||||
- Pin the Python version in `pyproject.toml` (`requires-python`). Where there is no `[project]`
|
||||
table at all — a plugin or custom component rather than a package — the pin lives in ruff's
|
||||
`target-version` and mypy's `python_version` instead, and **those two will drift apart unless
|
||||
something checks them**; set them in the same edit.
|
||||
- `pathlib.Path` over `os.path` everywhere (ruff's `PTH` rules enforce this).
|
||||
|
||||
---
|
||||
@@ -103,8 +156,13 @@ and when you must suppress, use a specific code (`# noqa: S603`) with a reason.
|
||||
- Custom exception types deriving from a single package-level base, so callers can catch the
|
||||
whole family.
|
||||
- **`logging` module, never `print`,** in anything that isn't a CLI writing to stdout as its
|
||||
actual output. One `logger = logging.getLogger(__name__)` per module, at module scope.
|
||||
actual output. One `logger = logging.getLogger(__name__)` per module, at module scope — unless
|
||||
the framework owns the logger tree, in which case its prefix wins (§11).
|
||||
- **Never log secrets** — tokens, auth headers, message content, personal data.
|
||||
- A blanket `contextlib.suppress(Exception)` is permitted in exactly one situation: a
|
||||
development-ergonomics escape hatch whose every suppression carries a written paragraph saying
|
||||
what staleness or platform quirk it absorbs. The rule is not "never suppress", it is
|
||||
**suppression is allowed when the comment carries the argument**.
|
||||
|
||||
---
|
||||
|
||||
@@ -114,11 +172,19 @@ and when you must suppress, use a specific code (`# noqa: S603`) with a reason.
|
||||
synchronous `requests`, no blocking file I/O in a coroutine. Use `asyncio.to_thread` for
|
||||
genuinely blocking calls.
|
||||
- `httpx` (or the framework's own client) over `requests` in async code.
|
||||
- **`asyncio.TaskGroup`** (3.11+) over bare `create_task` — it propagates exceptions and
|
||||
cancels siblings instead of losing them to an unawaited task.
|
||||
- **`asyncio.TaskGroup`** (3.11+) over bare `create_task` **where one failure should cancel the
|
||||
rest** — it propagates exceptions instead of losing them to an unawaited task.
|
||||
- **For fan-out over N external calls, TaskGroup is the wrong default**: cancel-on-first-error
|
||||
means one dead provider takes down the other nine. Use a bounded helper instead — an
|
||||
`asyncio.Semaphore(limit)` around each worker plus `asyncio.gather(..., return_exceptions=True)`
|
||||
— and keep it in one shared function rather than re-deriving it per caller.
|
||||
- **Every `create_task` result must be held in a reference**, or the garbage collector can
|
||||
cancel it mid-flight.
|
||||
- Cancellation is cooperative: clean up in `finally`, and never swallow `CancelledError`.
|
||||
cancel it mid-flight. Clear it in `finally`.
|
||||
- Cancellation is cooperative: clean up in `finally`, and never swallow `CancelledError` —
|
||||
but **distinguish a cancellation you asked for from a framework shutdown**. Re-raise unless a
|
||||
stop was requested; converting a real shutdown into an application error strands it.
|
||||
- Where a blocking library runs **its own** background thread, don't wrap it in `to_thread` —
|
||||
bridge back onto the loop with `loop.call_soon_threadsafe(future.set_result, …)`.
|
||||
- **Events over polling.** Subscribe to the library's event/callback API; do not loop on a
|
||||
timer. Where polling is genuinely unavoidable (no event source exists), use a **staggered
|
||||
backoff** rather than a fixed interval, and say why polling was necessary.
|
||||
@@ -141,7 +207,13 @@ When talking to a rate-limited API (a chat platform, a store, a tracker):
|
||||
|
||||
## 8. Structure
|
||||
|
||||
- Package layout: `src/<package>/`, tests in `tests/`.
|
||||
- Package layout: `src/<package>/`, tests in `tests/`. A single-binary stdlib-only CLI may stay
|
||||
flat; a plugin or component follows its framework's layout instead.
|
||||
- A tool that has to run on an immutable host is worth keeping **dependency-free** where the
|
||||
stdlib covers it (`argparse`, `subprocess`, `shlex`, `json`, `pathlib`) — nothing to provision
|
||||
is the strongest form of "it works on that machine".
|
||||
- Entry points are `[project.scripts]`. `typer` for a CLI with subcommands, stdlib `argparse`
|
||||
for everything else; do not add a CLI framework to a two-flag script.
|
||||
- **Shared behavior goes in a real shared module**, imported by each consumer — not
|
||||
copy-pasted between sibling packages. When three components grow the same queue/retry/
|
||||
formatting logic, that is the signal to extract it.
|
||||
@@ -153,17 +225,91 @@ When talking to a rate-limited API (a chat platform, a store, a tracker):
|
||||
function-length rule — `PLR0915` (`too-many-statements`, default 50) and `C901`
|
||||
(cyclomatic complexity) are the closest proxies and are worth enabling, but neither
|
||||
measures lines. Put **both** the function-length and file-length limits in a pytest
|
||||
case (see [`coding-dev.md` §4](skills-public/coding-dev.md)).
|
||||
case (see [`coding-dev.md` §4](skills-public/coding-dev.md)): file hard 1000 / soft 600 (soft
|
||||
prints a note rather than failing), function hard 100 measured by walking the `ast` rather than
|
||||
counting lines of text. **Guard the guard** with a test asserting the glob matched anything at
|
||||
all — a pattern that matches nothing makes every limit vacuous and still passes.
|
||||
|
||||
---
|
||||
|
||||
## 9. Testing
|
||||
|
||||
- `pytest`, with `pytest-asyncio` for coroutines.
|
||||
- `pytest`, with `pytest-asyncio`. Set `asyncio_mode = "auto"` in
|
||||
`[tool.pytest.ini_options]` — it removes the per-test marker entirely — plus
|
||||
`asyncio_default_fixture_loop_scope = "function"`.
|
||||
- Fixtures over setup/teardown methods; `tmp_path` for anything touching the filesystem.
|
||||
- **Regression test every fixed bug.**
|
||||
- Mock at the network boundary (`respx`, `responses`), not by monkeypatching your own
|
||||
internals — a test that patches the code under test proves nothing.
|
||||
- **Regression test every fixed bug**, and give them a home: a `test_regressions.py` is where
|
||||
they stop being scattered and start being a list.
|
||||
- **Mock at a boundary, not inside your own code.** A test that patches the code under test
|
||||
proves nothing. The boundary can be a network mock (`respx`, `responses`), a **real local fake
|
||||
service** the test drives end to end, or **recorded payloads fed to the provider seam** — the
|
||||
last two are usually better evidence than a mocked HTTP layer, because they exercise the
|
||||
parsing too.
|
||||
- **Never exercise destructive operations against real accounts or real data.** Temporary
|
||||
fixtures only.
|
||||
- `pytest -p no:randomly` only when order genuinely matters, and note why.
|
||||
|
||||
---
|
||||
|
||||
## 10. MCP Servers
|
||||
|
||||
> [!CAUTION]
|
||||
> **On the stdio transport, stdout is the protocol.** A stray `print`, a `logging`
|
||||
> `StreamHandler` pointed at stdout, or a library's progress bar corrupts the frame and the
|
||||
> client sees a parse error rather than your message. Log to **stderr** only, and keep §5's
|
||||
> no-`print` rule absolute here.
|
||||
|
||||
- Make the transport an explicit option (`--transport stdio|http`) rather than hardcoding one.
|
||||
- **Tool schemas are derived from the signature and the docstring**, so §2's "every function
|
||||
annotated" rule has a second payoff: an unannotated parameter produces a useless client-facing
|
||||
schema. Never hand-write JSON schema.
|
||||
- Set the behavioural annotations on every tool — `readOnlyHint`, `openWorldHint`, a human
|
||||
`title`.
|
||||
- **Errors are raised, not returned.** Raise (FastMCP serialises it) or return
|
||||
`isError=True` with a `TextContent` on the low-level SDK. A tool that returns the string
|
||||
`"ERROR: …"` as its normal value is indistinguishable from data to the client and defeats
|
||||
its retry logic.
|
||||
- `pydantic-settings` for layered config is good, but note what using a settings value as a
|
||||
**tool parameter default** means: it is frozen at import time *and* published in the advertised
|
||||
schema. Fine for a hostname, a secret leak for anything else.
|
||||
|
||||
---
|
||||
|
||||
## 11. Red-DiscordBot Cogs
|
||||
|
||||
- `self.config = Config.get_conf(self, identifier=<int>, force_registration=True)`, then
|
||||
`register_global`/`register_guild` for every key. **Never a side-car JSON file** beside the cog.
|
||||
- Read-modify-write goes through the context manager — `async with
|
||||
self.config.guild(ctx.guild).channels() as channels:` — because a plain get/set around a
|
||||
mutable list is a lost-update bug.
|
||||
- Lifecycle: `async def setup(bot)` in `__init__.py`; `async def cog_load(self)` for I/O-bearing
|
||||
startup; `@commands.Cog.listener()` for gateway events.
|
||||
- **`cog_unload` is synchronous.** It cannot await, so it cancels held tasks and fires teardown
|
||||
off into `self.bot.loop.create_task(self.api.close())`. Start long-lived tasks the same way,
|
||||
on the bot's loop.
|
||||
- `async def red_delete_data_for_user(self, *, _requester: str, _user_id: int) -> None` is a
|
||||
mandatory contract, not optional boilerplate.
|
||||
- **The logger name overrides §5**: `logging.getLogger("red.<owner>.<cog>")`, not `__name__`.
|
||||
Under `__name__` the cog sits outside the `red.*` tree where Red's own log-level controls
|
||||
apply, so the generic rule produces a logger nobody can turn up.
|
||||
- Red loads each cog directory independently, so a cross-cog shared module is awkward by design.
|
||||
The answer is a per-cog `utils/` package, not a copy-pasted `lib.py` — §8's shared-module rule
|
||||
still applies *within* a cog.
|
||||
|
||||
---
|
||||
|
||||
## 12. Home Assistant Custom Components
|
||||
|
||||
- `from __future__ import annotations` at the top of every module.
|
||||
- The `async_setup_entry` / `async_unload_entry` / `_async_update_listener` triple, with state on
|
||||
**`entry.runtime_data`**, not `hass.data`.
|
||||
- **Never create your own `aiohttp.ClientSession`** — take `async_get_clientsession(hass)`, or
|
||||
you leak a session per reload and HA logs it.
|
||||
- A `DataUpdateCoordinator[T]` subclass whose `_async_update_data` raises `UpdateFailed`. **No
|
||||
provider exception may escape it.**
|
||||
- Declare dependencies in `manifest.json` `requirements`, not in `pyproject.toml` — the component
|
||||
is not a package.
|
||||
- Fire events in size-bounded chunks with a run id in the envelope; a single huge event is not
|
||||
a thing the bus handles gracefully.
|
||||
- PEP 695 syntax is current here: `type MyConfigEntry = ConfigEntry[MyCoordinator]`, and
|
||||
`def f[T](…)` for generics.
|
||||
+2
-1
@@ -5,7 +5,7 @@ description: >-
|
||||
in any language. Defines standing preferences on strictness, dependency choice, file and function
|
||||
size limits, test discipline, build output handling, error/warning policy, and commit hygiene.
|
||||
Links to per-language rules under code-languages/ for Rust, TypeScript/JavaScript, Bash, Python,
|
||||
Go, C#/.NET, and C/C++.
|
||||
PHP, Go, C#/.NET, and C/C++.
|
||||
---
|
||||
|
||||
# Coding & Development — Standing Rules and Preferences
|
||||
@@ -34,6 +34,7 @@ Per-language specifics live in [`code-languages/`](skills-public/code-languages/
|
||||
| TypeScript / JavaScript | [`code-languages/js-typescript.md`](skills-public/code-languages/js-typescript.md) |
|
||||
| Bash / Shell | [`code-languages/bash.md`](skills-public/code-languages/bash.md) |
|
||||
| Python | [`code-languages/python.md`](skills-public/code-languages/python.md) |
|
||||
| PHP | [`code-languages/php.md`](skills-public/code-languages/php.md) |
|
||||
| Go | [`code-languages/go.md`](skills-public/code-languages/go.md) |
|
||||
| C# / .NET | [`code-languages/csharp-dotnet.md`](skills-public/code-languages/csharp-dotnet.md) |
|
||||
| C / C++ | [`code-languages/c-cpp.md`](skills-public/code-languages/c-cpp.md) |
|
||||
|
||||
Reference in new issue
Block a user