diff --git a/README.md b/README.md index 3694e1e..41f71c4 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/cli-tooling.md b/cli-tooling.md index 9f971ec..b7b533a 100644 --- a/cli-tooling.md +++ b/cli-tooling.md @@ -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//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:" "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: diff --git a/code-languages/php.md b/code-languages/php.md new file mode 100644 index 0000000..5d6bcec --- /dev/null +++ b/code-languages/php.md @@ -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 -- 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 +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`, 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`, `@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`. +- **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 `` or `