Add the no-hardcoded-tunables convention and its review check

A number or string that two reasonable deployments could want set
differently — a timeout, grace period, output cap, result-count limit,
model name, base URL — belongs on the plugin's schemastery Config with
the shipped value as its default, not in a bare literal or module
constant. A DEFAULT_* constant or a test-only injection seam is not
configurability: the test is whether a cordis.yml deployment can change
the value without a code edit. Protocol/wire constants, semantic
constants, values pinned by an external spec, and security invariants
stay hardcoded.

The convention lands in AGENTS.md § Conventions (the authoritative
source the review skill cites); dsh-code-review gains the matching
reviewer-only check, since no mechanical gate can detect a hardcoded
tunable.

AGENTS.md

@@ -261,6 +261,7 @@ Dev/test/demo run **unbuilt** via tsx + the source `paths` map in the root `tsco
 - **Plugins, not loop changes**: new behavior goes into a plugin on the documented extension seams (see the plugin sanity checklist in docs/architecture.md). Changing `agent-loop` requires updating that doc.
 - **Capability seams are three packages**: when adding a swappable capability (an execution backend, a provider integration, …), split it into *interface* (abstract service + vocabulary types, e.g. `bash/`), *implementation* (a concrete subclass, e.g. `bash-local/`), and *consumer* (what the model/plugins see, e.g. `tool-bash/`). Implementations and consumers then evolve independently — a sandboxed executor replaces `bash-local` without touching tool schemas. The LLM seam follows the same shape (`llm/` is interface + consumer surface; adapters are implementations). See docs/architecture.md § "Capability seams" for when NOT to split.
 - **Explicit > implicit at package seams**: interface/vocabulary types spell out every field a consumer must supply — no optional field that the implementation silently fills with a hidden `?? default`. Put defaulting in the owning implementation as an explicit step (a `resolve(request): Spec` method that turns the optional-field request into the required-field spec), not smuggled inside `run()`/`start()`. Example: `dsh-bash` splits `BashExecRequest` (optional `workdir`/`timeoutMs`, model-facing) from `BashExecSpec` (required, what `run`/`start` act on); the tool layer calls `ctx.bash.resolve()` between them. The reader of a `BashExecSpec` never has to wonder where the working directory came from.
+- **No hardcoded tunables in plugins — a deployment knob belongs on `Config`**: a number or string that two reasonable deployments could want set differently — a timeout, grace period, output/truncation cap, result-count limit, retry count, buffer size, model name, API base URL, user agent, filesystem path — is plugin configuration, not a bare literal or module constant. Expose it as a field on the plugin's schemastery `Config` with the shipped value as its `.default(…)`, document it in the package README, and validate the range where garbage would misbehave silently (see `assertPositiveFinite` in `dsh-bash-local`/`dsh-web-fetch-local`). A named `DEFAULT_*` constant does not make a value configurable, and neither does a test-only injection seam (`internals`) — the test is whether a `cordis.yml` deployment can change the value without a code edit. The rule is scoped to genuine tunables: protocol/wire constants (format versions, method names, event tags), semantic constants (exit codes, signal names, HTTP statuses), values pinned by an external spec, and security invariants (the credential-scrub env pattern) stay hardcoded — making those configurable invites misconfiguration, not flexibility. When unsure, ask who would ever set it: a deployer tuning the product (config) or only a maintainer changing the design (constant). The exemplar is `dsh-web-fetch-local`, whose every cap is a defaulted `Config` field.
 - **Opaque cross-boundary ids are branded, never bare `string`**: an identity that crosses a package seam and that a consumer must store-and-return but never parse (a backend-defined version token, a target key, a task/session/call id) is a `Branded<B>` from `@deepseek-ai/dsh-brand` with a same-named cast factory in the owning package — a zero-cost compile-time guard so semantically-distinct strings stop being interchangeable. Not every string needs it: author-readable names (`ToolName`) and closed code unions (`ErrorCode`) don't. See [Branded IDs everywhere they belong](docs/rfc/implemented/architecture/2026-06-20-branded-ids.md).
 - **An empty `catch` must name what it swallows and why nothing else can hit it**: a bare `catch {}` hides bugs. When you deliberately ignore a throw, the comment must (a) name the single expected failure, (b) say why ignoring it is correct — usually because the useful state was already captured *before* the `try` — and (c) make clear nothing else of consequence can reach the catch (ideally the `try` wraps a single statement). Example: the error-body `response.json()` parse in `dsh-llm-deepseek`'s adapter sets `code` + HTTP `status` from the status line before the `try`, so a malformed provider body can only cost a richer message, never the real error.
 - **Symmetry is usually more correct**: when two related values play parallel roles (a test fixture and its expected output, a request shape and its response shape, a buggy input and the test that checks the fix), give them parallel form — both named consts, or both inline, not one each way. Asymmetry is a smell that usually points at a missed extraction.