Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
  BluscreamandClaude Opus 5
|
6207de1a97 | ||
|
Bluscream
|
b8505fc623 | ||
|
BluscreamandClaude Opus 5
|
5aef9707d2 |
No files matched your search
@@ -19,10 +19,13 @@ 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 |
|
||||
| **Lua** | [`lua.md`](skills-public/code-languages/lua.md) | pin the dialect first, `luac -p` + luacheck as the only gate, locals over globals, `pcall` at the boundary, `#t`/pattern traps, embedded & HavokScript hosts |
|
||||
| **GSC** | [`gsc.md`](skills-public/code-languages/gsc.md) | no compiler or test harness, `isdefined` on everything external, `endon` before the first wait, per-frame budget, precache at load, level-state side effects |
|
||||
|
||||
Each language file extends `coding-dev.md` rather than repeating it. A project's own
|
||||
`AGENTS.md` / `CLAUDE.md` / `.rules` always overrides both.
|
||||
|
||||
+141
-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
|
||||
@@ -49,6 +51,24 @@ Before writing a script to do something, check whether:
|
||||
- the tool you are already using has a subcommand for it,
|
||||
- the project's own `scripts/` directory already contains it.
|
||||
|
||||
### Desktop Steam Client Lifecycle (`steamcli`)
|
||||
|
||||
> [!TIP]
|
||||
> **Never spawn Steam via raw shell backgrounding (`nohup steam &` or `steam -silent &`) or kill it with `pkill`.**
|
||||
> On desktop Linux/Bazzite, raw spawning leaves orphaned bash wrappers, misses custom client flags, or fails to initialize properly.
|
||||
> Instead, manage the desktop Steam client through **`steamcli client`**:
|
||||
> ```bash
|
||||
> # Graceful exit and wait for it to finish flushing configuration:
|
||||
> steamcli client shutdown
|
||||
>
|
||||
> # Launch the desktop client cleanly with configured defaults:
|
||||
> steamcli client launch
|
||||
>
|
||||
> # Launch with custom Steam arguments:
|
||||
> steamcli client launch -- -silent
|
||||
> ```
|
||||
|
||||
|
||||
---
|
||||
|
||||
## 2. Non-Interactive Invocation
|
||||
@@ -83,6 +103,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 +196,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,237 @@
|
||||
---
|
||||
name: gsc
|
||||
description: >-
|
||||
Use this skill when writing, reviewing, or debugging GSC (Game Script / Call of Duty script)
|
||||
or CSC — including entity-vs-level scope, threads and notify/wait semantics, endon/waittill
|
||||
discipline, undefined-value guards, the no-OOP/no-exceptions reality, asset precaching,
|
||||
per-frame cost, and the fact that GSC has no compiler, linter, or test harness worth the name.
|
||||
---
|
||||
|
||||
# GSC — Rules & Preferences
|
||||
|
||||
Extends [`coding-dev.md`](skills-public/coding-dev.md). Everything there applies; this adds GSC specifics.
|
||||
|
||||
GSC is the Call of Duty engine's own scripting language (`.gsc` server-side, `.csc`
|
||||
client-side). It is **not Lua, not JavaScript, and not C** despite resembling all three. It has
|
||||
no type system, no exceptions, no classes, no closures in most dialects, and — in a modded
|
||||
install — **no diagnostics beyond the in-game console**. That last fact drives most of what
|
||||
follows: the language will not tell you that you are wrong, so the code has to be written so
|
||||
that it cannot be.
|
||||
|
||||
> [!IMPORTANT]
|
||||
> **Establish the dialect before writing anything.** IW3/IW4 (MW/MW2), IW5 (MW3), T5/T6 (Black
|
||||
> Ops 1/2), IW6, s1/s2, H1/H2 (MWR / MW2CR), and the various mod loaders (h2-mod, iw4x, plutonium,
|
||||
> t6 GSC Studio) each differ in available builtins, in how scripts get loaded, and in whether
|
||||
> `foreach`, `switch` fallthrough, or function pointers exist. A builtin from the wrong title is
|
||||
> a silent `undefined` or a hard crash.
|
||||
|
||||
---
|
||||
|
||||
## 1. There Is No Gate — Build One
|
||||
|
||||
GSC has no `gsc -Wall`. What exists:
|
||||
|
||||
| Check | How |
|
||||
| :--- | :--- |
|
||||
| **Syntax** | The engine's own compile at load. Read the console; a parse error names the file and line and then stops the whole script loading. |
|
||||
| **Runtime errors** | In-game console / `developer 1`. Many mod loaders print a script stack trace. |
|
||||
| **Static lint** | Third-party only, and dialect-specific. Use one if it exists for the target title; otherwise there is none. |
|
||||
| **Tests** | None off-host. Behaviour is verified by playing the map. |
|
||||
|
||||
Therefore:
|
||||
|
||||
- **`developer 1` (or the loader's equivalent) is mandatory during development.** Without it the
|
||||
engine suppresses script warnings and an undefined-value error looks like "the feature did
|
||||
nothing".
|
||||
- **Load the script and read the console on every change**, before judging behaviour. "Nothing
|
||||
happened" is far more often a parse error at load than a logic bug.
|
||||
- **Instrument with `iprintln` / `iprintlnbold` / `println`** on the success *and* the failure
|
||||
path. A function that silently returns on a guard is indistinguishable from one that never ran.
|
||||
- **Say plainly in your report that GSC behaviour is unverified** unless you actually loaded the
|
||||
map and watched it. There is no test suite to hide behind.
|
||||
|
||||
---
|
||||
|
||||
## 2. Scope — `self`, `level`, `game`, and Locals
|
||||
|
||||
| Scope | Lifetime | Use for |
|
||||
| :--- | :--- | :--- |
|
||||
| local variable | the function call | everything it can hold |
|
||||
| `self` | the entity | per-entity state (a player's, an actor's) |
|
||||
| `level` | the current map/level | per-round state, map config, shared handles |
|
||||
| `game` | across map restarts | persistent state only |
|
||||
|
||||
- **Prefix your own fields.** `level.mymod_spawn_queue`, not `level.queue`. `level` and `self`
|
||||
are one flat namespace shared with the stock game scripts and every other mod; a collision
|
||||
overwrites engine state and the symptom appears somewhere unrelated.
|
||||
- **Never write to a stock field you did not create** unless that is the documented integration
|
||||
point. Read it, copy it, leave it.
|
||||
- **A function called *on* an entity gets it as `self`**: `player giveweapon("m4")`. Calling the
|
||||
same function without a prefix leaves `self` undefined and the first `self.x` access errors.
|
||||
- `self` is not inherited into a `thread`ed call's children automatically in the way callers
|
||||
expect — `ent thread f()` gives `f` `self == ent`, but a plain `f()` inside it does not.
|
||||
|
||||
---
|
||||
|
||||
## 3. Undefined Is The Default — Guard Everything External
|
||||
|
||||
GSC has no types and no nullability. **Reading a field that was never set yields `undefined`,
|
||||
and using it in arithmetic or a method call is a runtime error that aborts the thread** — often
|
||||
silently from the player's point of view.
|
||||
|
||||
```gsc
|
||||
// WRONG: any of these can be undefined
|
||||
ent = level.target;
|
||||
ent setorigin(level.player.origin + (0, 0, 64));
|
||||
|
||||
// RIGHT
|
||||
if (!isdefined(level.target) || !isdefined(level.player))
|
||||
return;
|
||||
```
|
||||
|
||||
- **`isdefined()` on every entity, every `level.*`/`self.*` read you did not write in the same
|
||||
function, and every return value from a lookup.**
|
||||
- **Entities die.** A handle held across a `wait` may be a dead entity by the time the thread
|
||||
resumes. Re-check `isdefined(ent)` *after* every `wait`, not only before.
|
||||
- `isalive(ent)` is the check for an actor you intend to act on; `isdefined` only proves the
|
||||
handle exists.
|
||||
- This is the one place where the general "don't over-guard" rule inverts: in GSC a value from
|
||||
outside the current function genuinely can be undefined, so the check is not dead code.
|
||||
- Arrays: `array.size` is safe on an empty array but errors on an undefined one. Guard the array,
|
||||
then the index.
|
||||
|
||||
---
|
||||
|
||||
## 4. Threads, `notify`, `waittill`, `endon`
|
||||
|
||||
```gsc
|
||||
monitor_thing()
|
||||
{
|
||||
level endon("game_ended");
|
||||
self endon("death");
|
||||
|
||||
for (;;)
|
||||
{
|
||||
self waittill("trigger_activated");
|
||||
do_work();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- **`thread` starts a coroutine and returns immediately.** It does not return the function's
|
||||
value — a `thread`ed function's `return` value is unreachable. Pass a handle or notify.
|
||||
- **Every long-lived thread needs an `endon`.** Without one, a loop keeps running after the
|
||||
entity dies or the level ends, and then errors on a dead `self` — or leaks, one copy per spawn.
|
||||
`level endon("game_ended")` plus the relevant entity `endon` is the baseline.
|
||||
- **`endon` must be registered before the first `wait`**, at the top of the function. Registering
|
||||
it later leaves a window.
|
||||
- **`waittill` is the event primitive — prefer it to polling.** `self waittill("death")` is
|
||||
correct; `while (isalive(self)) wait 0.05;` is the same thing, worse, and costs every frame
|
||||
forever. This is the general events-over-polling rule, and GSC gives you the event.
|
||||
- **A `notify` with no waiter is lost.** There is no queue. If a thread might not be waiting yet,
|
||||
set a flag as well as notifying, and have the waiter check the flag first.
|
||||
- `waittill("x", a, b)` receives the notify's extra parameters. The names are positional.
|
||||
- **`waittillframeend`** is the tool for "after everything else this frame" — use it instead of
|
||||
`wait 0.05` when the intent is ordering, not delay.
|
||||
- `wait 0` is not zero; it is one frame. `wait 0.05` is one server frame at 20Hz — write the
|
||||
intent, not a magic number, and name the constant.
|
||||
|
||||
---
|
||||
|
||||
## 5. Per-Frame Cost Is Your Budget
|
||||
|
||||
The script VM shares the server frame with everything else, and there is a **hard instruction
|
||||
budget per frame**. Exceeding it produces `script runtime error: exceeded maximum number of
|
||||
script variables` or a visible server hitch.
|
||||
|
||||
- **One polling loop per concern, never one per entity.** A `for(;;) { wait 0.05 }` loop on each
|
||||
of 60 actors is 60 wakeups a frame. Keep one manager loop over an array.
|
||||
- **Where an event exists, the polling loop should not.** See §4.
|
||||
- **Never loop without a `wait` inside.** A `for(;;)` with no `wait` hangs the server frame and
|
||||
takes the match down — this is the single most common fatal GSC mistake.
|
||||
- Keep the body of a per-frame loop trivial. Move anything expensive behind a condition or a
|
||||
longer interval, and say in a comment why that interval.
|
||||
- Spawning is expensive and entity counts are capped. Reuse and pool; delete what you spawned.
|
||||
|
||||
---
|
||||
|
||||
## 6. Assets Must Be Precached
|
||||
|
||||
```gsc
|
||||
init()
|
||||
{
|
||||
precachemodel("com_laptop_open");
|
||||
precacheshader("hud_icon_frag");
|
||||
precachestring(&"MYMOD_HINT");
|
||||
}
|
||||
```
|
||||
|
||||
- **Precaching only works during the level's load phase.** A `precachemodel` after the map is
|
||||
live either errors or silently fails, and the model then renders as a missing-asset
|
||||
placeholder or nothing at all.
|
||||
- Put every precache in the init path, unconditionally — not behind the feature flag that decides
|
||||
whether the asset gets used.
|
||||
- **A model or weapon name not present in the loaded level's zone will not spawn**, regardless of
|
||||
precaching. Asset availability is per-map; a name verified on one map proves nothing about
|
||||
another. State which map you verified on.
|
||||
- Localized strings use the `&"REFERENCE"` form and must exist in the string table; a missing one
|
||||
displays the raw reference.
|
||||
|
||||
---
|
||||
|
||||
## 7. Language Facts That Surprise People
|
||||
|
||||
- **No exceptions, no `try`.** An error aborts the current thread and nothing else. There is no
|
||||
recovery and no handler — which is why §3's guards are the error handling.
|
||||
- **No classes, no inheritance, no `new`.** "Objects" are entities with fields, or plain arrays
|
||||
used as structs. `spawnstruct()` is the idiom for a bag of fields with no world presence.
|
||||
- **Function pointers**: `::funcname` (and `[[ptr]]()` to call) exist in most dialects but not
|
||||
all. Verify before relying on them.
|
||||
- **No closures** in most dialects — an inner function cannot capture a local. Pass state
|
||||
explicitly or hang it on a struct.
|
||||
- `foreach (x in arr)` exists in IW4+ and T6 but not in the earliest dialects; `for (i = 0;
|
||||
i < arr.size; i++)` always works.
|
||||
- **Arrays are associative.** `array[]` with integer keys behaves like a list, but deleting an
|
||||
element leaves a hole and `.size` counts keys. Rebuild rather than delete-in-place.
|
||||
- String comparison is `==`; there is no `switch` on anything but constants, and fallthrough
|
||||
rules differ by dialect.
|
||||
- Vectors are `(x, y, z)` literals with `+`, `-`, and scalar `*`. Component access is `v[0]`,
|
||||
`v[1]`, `v[2]`. `anglestoforward` / `vectornormalize` / `distance` are builtins — do not
|
||||
hand-roll vector maths.
|
||||
|
||||
---
|
||||
|
||||
## 8. Style & Structure
|
||||
|
||||
- **Allman braces, 4 spaces** — the stock scripts' convention; match it, because GSC is almost
|
||||
always read next to engine scripts.
|
||||
- `main()` is the level entry point; `init()` is the convention for a module's setup called from
|
||||
it. Which one the loader calls is dialect- and loader-specific — verify, do not assume.
|
||||
- `#include maps\_utility;` (dialect-dependent path syntax) at the top; prefer the stock utility
|
||||
functions over reimplementing them. `maps\_utility` and `common_scripts\utility` already have
|
||||
flag, wait, and array helpers.
|
||||
- **`/* */` comments do not nest.** Commenting out a block that contains one breaks the file.
|
||||
- File and function size limits from the general rules apply, and matter more here than usual:
|
||||
there is no debugger worth the name, so a 300-line function is unanalysable.
|
||||
- **Delete dead code.** With no linter to flag it, commented-out GSC accumulates forever.
|
||||
|
||||
---
|
||||
|
||||
## 9. Side Effects on Level State
|
||||
|
||||
GSC runs inside someone else's running game. A change to shared state is not scoped to your
|
||||
feature and is not undone when your thread ends.
|
||||
|
||||
- **Mutating a stock spawner, trigger, or flag changes the mission.** Setting
|
||||
`spawner.count = 999` to get an extra AI makes that spawner infinite for the rest of the level.
|
||||
Save the old value and restore it, or clone rather than mutate.
|
||||
- **Anything you spawn, you delete.** Track handles in your own `level.mymod_*` array and provide
|
||||
a cleanup path; an orphaned entity survives until map change and counts against the cap.
|
||||
- **`setdvar` is global and persistent.** A dvar used as a command channel must be cleared
|
||||
immediately on read, and the clear must happen even on the failure path — otherwise the next
|
||||
poll re-fires the same command.
|
||||
- A dvar as an IPC channel is a **poll**, which §5 says to avoid; it is acceptable only because
|
||||
GSC cannot receive a host event, and the interval and the clear-on-read should both be
|
||||
explicit and commented.
|
||||
- Never leave the player in a modified state (god mode, `notarget`, a changed movement speed)
|
||||
when the feature that set it is turned off. Toggles need an off path that actually restores.
|
||||
@@ -0,0 +1,232 @@
|
||||
---
|
||||
name: lua
|
||||
description: >-
|
||||
Use this skill when writing, reviewing, or debugging Lua — including Lua 5.1/5.2/5.3+ and
|
||||
LuaJIT dialect differences, HavokScript (HKS) as used by Call of Duty's LUI, luacheck
|
||||
configuration, locals over globals, pcall error handling, the integer-vs-float and
|
||||
table-length pitfalls, module layout, and embedded/sandboxed-host constraints.
|
||||
---
|
||||
|
||||
# Lua — Rules & Preferences
|
||||
|
||||
Extends [`coding-dev.md`](skills-public/coding-dev.md). Everything there applies; this adds Lua specifics.
|
||||
|
||||
Lua is almost always **embedded**: the host decides the dialect, the available standard
|
||||
library, the module loader, and whether an uncaught error takes the process down. **Establish
|
||||
which host and which dialect you are writing for before writing a line** — the same file is
|
||||
correct in one and a crash in another.
|
||||
|
||||
---
|
||||
|
||||
## 1. Pin the Dialect First
|
||||
|
||||
| Dialect | Notable traits |
|
||||
| :--- | :--- |
|
||||
| **5.1** | `loadstring`, `unpack`, `setfenv`/`getfenv`, `module()`, no integer type, no goto |
|
||||
| **LuaJIT** | 5.1 plus `bit`, `ffi`, `jit`; a few 5.2 extensions |
|
||||
| **5.2** | `load` replaces `loadstring`, `_ENV` replaces `setfenv`, `goto`, `table.unpack` |
|
||||
| **5.3** | distinct integer subtype, `//`, bitwise operators, `math.type` |
|
||||
| **5.4** | `<const>` / `<close>` attributes, generational GC |
|
||||
| **HavokScript (HKS)** | a 5.1 fork; see §8 |
|
||||
|
||||
- **Determine it from the host, not from habit.** `print(_VERSION)` in the target runtime, or
|
||||
read the host's embedding code. A `goto` in a 5.1 host is a parse error, and a `loadstring`
|
||||
in 5.4 is a nil call.
|
||||
- **Write for the host's dialect; do not write portable-across-all-five code** unless the
|
||||
module genuinely ships to several. Compatibility shims
|
||||
(`local unpack = table.unpack or unpack`) are worth it only where that is true.
|
||||
- Lua 5.1 has **no integer type** — every number is a double. `1/2` is `0.5`, and
|
||||
`tostring(3.0)` is `"3"` in 5.1 but `"3.0"` in 5.3. Never round-trip a number through a
|
||||
string and expect the same text across dialects.
|
||||
|
||||
---
|
||||
|
||||
## 2. The Gate
|
||||
|
||||
```bash
|
||||
luacheck . # lint; must be clean
|
||||
stylua --check . # or lua-format; formatting
|
||||
luac -p $(git ls-files '*.lua') # parse check against the TARGET dialect's luac
|
||||
busted # tests, where the code is testable off-host
|
||||
```
|
||||
|
||||
- **`luac -p` is the cheapest real check available** and the one most often skipped. It catches
|
||||
syntax errors, and when run with the host's own Lua version it catches dialect mismatches
|
||||
before the host does. For an embedded host whose scripts cannot be unit tested, this plus
|
||||
`luacheck` *is* the gate — wire it into the build script and never commit a file that fails it.
|
||||
- `luacheck` needs its globals declared or it reports the host's API as undefined. Declare them
|
||||
rather than disabling the check:
|
||||
```lua
|
||||
-- .luacheckrc
|
||||
std = "lua51"
|
||||
read_globals = { "Engine", "LUI", "GenericButtonSettings" }
|
||||
globals = { "MyModuleState" } -- only what this code deliberately writes
|
||||
max_line_length = 120
|
||||
```
|
||||
- `read_globals` for the host's API, `globals` only for names you intentionally define. If the
|
||||
list of writable globals is growing, that is the finding — see §3.
|
||||
|
||||
---
|
||||
|
||||
## 3. Locals, Not Globals
|
||||
|
||||
**Every variable is `local` unless it must be visible to the host.** An unqualified assignment
|
||||
creates a global, silently, with no warning from the language.
|
||||
|
||||
```lua
|
||||
-- WRONG: typo creates a new global; the real one keeps its old value
|
||||
local function update() cofnig.volume = 1 end
|
||||
|
||||
-- RIGHT
|
||||
local config = require("config")
|
||||
local function update() config.volume = 1 end
|
||||
```
|
||||
|
||||
- Locals are also **faster** — they are register slots, while a global is a hash lookup in
|
||||
`_ENV` on every access. Hoist hot library functions into locals at the top of a module:
|
||||
`local floor, insert = math.floor, table.insert`.
|
||||
- **Namespace what must be global.** One table (`MyMod.state`), never a scatter of
|
||||
`myModFoo` / `myModBar`. In a shared host every global is in one namespace with every other
|
||||
script's.
|
||||
- `luacheck` flags accidental globals. Keep it clean and the typo class disappears.
|
||||
- The 200-local-per-function limit is real; hitting it means the function needed splitting long ago.
|
||||
|
||||
---
|
||||
|
||||
## 4. Errors — `pcall` at the Boundary, Not Everywhere
|
||||
|
||||
- **`error()` for real failures; return `nil, message` for expected ones.** A file that may not
|
||||
exist returns `nil, err`; a caller passing the wrong type gets an `error`.
|
||||
- **`pcall` / `xpcall` at the boundary only** — a callback the host invokes, a plugin entry
|
||||
point, a request handler. Wrapping every internal call turns a stack trace into a string and
|
||||
hides where the fault was.
|
||||
- **`xpcall(f, debug.traceback)` when you need the trace.** Plain `pcall` discards it, and the
|
||||
message alone rarely identifies the line in a long callback chain.
|
||||
- **Never swallow it.** `pcall(f)` with the result ignored is an empty catch block:
|
||||
```lua
|
||||
-- WRONG
|
||||
pcall(risky)
|
||||
|
||||
-- RIGHT
|
||||
local ok, err = pcall(risky)
|
||||
if not ok then log.warn("risky failed: " .. tostring(err)) end
|
||||
```
|
||||
- `error("msg", 2)` blames the caller instead of the raising line — the right level for an
|
||||
argument-validation error.
|
||||
- `error()` can be given a **table**, which is how you carry structured error data; the
|
||||
`tostring` of it is then meaningless, so handlers must check `type(err)`.
|
||||
|
||||
---
|
||||
|
||||
## 5. Tables — The Traps
|
||||
|
||||
- **`#t` is only defined for a sequence** (keys `1..n` with no holes). With a hole it may return
|
||||
either boundary, and the choice can change between runs. Track the count yourself when a
|
||||
table can be sparse, and never use `#t` on a map.
|
||||
- **`table.insert` / `table.remove` shift every later element** — O(n). Appending with
|
||||
`t[#t+1] = v` is cheaper than `table.insert(t, v)`; building a new table beats removing from
|
||||
the middle in a loop.
|
||||
- **Never mutate a table while iterating it with `pairs`.** Assigning to an existing key is
|
||||
defined; adding a new one is not. Collect keys first, then act.
|
||||
- **`pairs` order is unspecified** and varies run to run. Anything user-visible or compared
|
||||
between runs must sort its keys first.
|
||||
- `ipairs` stops at the first `nil`. Use it for sequences and `pairs` for maps — the choice
|
||||
documents which the table is.
|
||||
- `nil` cannot be stored in a table; `t[k] = nil` is a delete, and a sequence containing `nil`
|
||||
is not a sequence.
|
||||
- Beware `0` and `""` being **truthy**. Only `nil` and `false` are false, so
|
||||
`if count then` is true at zero — test `if count ~= 0 then`.
|
||||
- `1` and `1.0` are the **same key** in 5.3+ (the float is normalized), but a non-integral
|
||||
float is its own key.
|
||||
|
||||
---
|
||||
|
||||
## 6. Strings
|
||||
|
||||
- **Never build a string in a loop with `..`.** Each concatenation allocates; collect into a
|
||||
table and `table.concat` once.
|
||||
- **Lua patterns are not regex.** `%` is the escape (not `\`), there is no alternation and no
|
||||
`+`/`?` on groups, character classes are `%a %d %s %w %p %c %x` and their uppercase
|
||||
complements. Reach for `lpeg` when the grammar is real.
|
||||
- `string.format("%q", s)` produces a Lua-readable quoted literal — correct for serialization,
|
||||
wrong for display.
|
||||
- `%s` in `string.format` calls `tostring`, so a nil argument renders `"nil"` rather than
|
||||
erroring — the opposite of `..`, which errors on nil. That asymmetry is a frequent crash
|
||||
source in host callbacks: **prefer `string.format` over `..` for any message built from
|
||||
values that could be nil**, or guard with `tostring()`.
|
||||
- Indices are 1-based and inclusive on both ends: `s:sub(1, 3)` is the first three bytes.
|
||||
- Strings are **byte** strings. `#s` and `s:sub` count bytes, not characters — use `utf8` (5.3+)
|
||||
or a library for anything non-ASCII.
|
||||
|
||||
---
|
||||
|
||||
## 7. Modules & Structure
|
||||
|
||||
```lua
|
||||
local M = {}
|
||||
|
||||
local function helper(x) -- private: a local, not M.helper
|
||||
return x * 2
|
||||
end
|
||||
|
||||
function M.public(x)
|
||||
return helper(x)
|
||||
end
|
||||
|
||||
return M
|
||||
```
|
||||
|
||||
- **Return a local table. Never set globals, never use `module()`** (deprecated in 5.1,
|
||||
removed in 5.2).
|
||||
- `require` caches by module name — a module body runs once. Do not rely on re-`require` to
|
||||
reset state; expose an explicit `M.reset()` if a reset is needed.
|
||||
- `:` for methods that take `self`, `.` for everything else. Be consistent — a `.` call on a
|
||||
`:` method passes the first argument as `self` and fails confusingly.
|
||||
- File and function size limits from the general rules apply.
|
||||
|
||||
---
|
||||
|
||||
## 8. Embedded & Sandboxed Hosts
|
||||
|
||||
When the host owns the interpreter (a game engine, nginx, Redis, Neovim, a plugin API), its
|
||||
constraints outrank every preference above.
|
||||
|
||||
- **Verify an API exists before calling it.** The host's surface differs between versions and
|
||||
builds, and a missing function is a `nil` call that takes down whatever invoked you:
|
||||
```lua
|
||||
if type(Engine.SetDvarString) == "function" then
|
||||
Engine.SetDvarString(name, value)
|
||||
end
|
||||
```
|
||||
Prefer `type(x) == "function"` over a bare `if x then` — it also rejects a truthy non-callable.
|
||||
- **An uncaught error in a host callback may crash the process, not just the script.** In a C
|
||||
host the error unwinds through C frames that were not written to expect it. Treat every
|
||||
callback body as a trust boundary and `pcall` it.
|
||||
- **Do not monkey-patch host globals** (`print`, `require`, a host API function) outside a
|
||||
tightly scoped save/restore — and even then, remember the restore does not happen if the body
|
||||
errors, so do it in the `pcall` result path. Reentrancy makes the saved original a wrapper.
|
||||
- **Load order is the host's, not yours.** Do not assume a sibling script has run. Read shared
|
||||
state inside callbacks rather than at load time, and register your own state defensively.
|
||||
- **Check what the standard library actually is.** Many hosts remove `io`, `os`, and `debug`, or
|
||||
replace them with their own (`io.readfile` rather than `io.open`). Feature-test; do not assume.
|
||||
- Logging is whatever the host provides. Use it rather than `print`, and never leave a
|
||||
high-frequency print in a per-frame callback.
|
||||
|
||||
### HavokScript (HKS) — Call of Duty LUI
|
||||
|
||||
Call of Duty's UI (LUI) runs **HavokScript**, a proprietary Lua 5.1 fork:
|
||||
|
||||
- It is **5.1-based**: `loadstring`, `unpack`, `setfenv` — no `goto`, no integer division, no
|
||||
bitwise operators, no `table.unpack`.
|
||||
- There is **no `require` for siblings inside a UI script folder** in h2-mod-style loaders; a
|
||||
module directory's `__init__.lua` is loaded on its own. Keep each module self-contained in one
|
||||
file rather than splitting it and discovering the split does not load.
|
||||
- There is **no off-host test harness**. `luac -p` with a 5.1 `luac`, `luacheck --std lua51`,
|
||||
and reading the game's console output are the entire verification story — so lint and parse
|
||||
checks are not optional here, they are the only gate that exists.
|
||||
- **A dvar read is not type-safe.** `Engine.GetDvarString` on an unregistered dvar returns `nil`,
|
||||
and `nil .. "text"` is an error that propagates out of the LUI event dispatcher as an access
|
||||
violation. Register the dvar first, feature-test the accessor, and build messages with
|
||||
`string.format`.
|
||||
- **Hot-reload with `dofile` leaves the previous registration in place.** Guard re-entry with a
|
||||
module flag, and expect duplicate menu buttons when you do not.
|
||||
@@ -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.
|
||||
+4
-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,9 +34,12 @@ 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) |
|
||||
| Lua | [`code-languages/lua.md`](skills-public/code-languages/lua.md) |
|
||||
| GSC (Call of Duty script) | [`code-languages/gsc.md`](skills-public/code-languages/gsc.md) |
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in new issue
Block a user