# Code Guidelines

Rugent — an AI agent in Rust. The local gate is `rug fmt --check`; the full suite is CI's job on `master`. **SHOULD** rules are strongly recommended.

This file holds only what the repository cannot tell you by itself. Structure comes from `ls`, commands from `rug help` and [docs/COMANDLETS.md](docs/COMANDLETS.md), label values from `rug bug labels`, subsystems from `docs/[NAME].md`.

## Orientation

The workspace is `crates/rugent-*`; each crate's `lib.rs` says what it owns. Two things are not visible from the tree:

- The **master** reads its resources from the filesystem under `RUGENT_ROOT` (the repo tree in dev, `/app` in the container) and drives the host Docker daemon to spawn per-user sandbox siblings. The **slave** binary (`rugent`) stays self-contained — it runs on fresh hosts with no resource root
- The one tool registry is `rugent-tools/src/executor/registry.rs` — `TOOLS` is the list, and **nothing else enumerates tools by name**

## Project environment

- **MUST** Use the global `rug` command for every dev task — it loads `.env`, which the scripts under `bin/` do not do for themselves (`rug bug labels` works, `./bin/bug labels` dies on `GITLAB_SITE not set`)
- **MUST** Run the tests your change is about — the ones written for it by the TDD protocol plus whatever they touch (`cargo test -p [crate] [name]`). The FULL suite belongs to CI, not to the developer machine: `rug test` exists for when you want it (it recreates the test DB), it is not a per-change obligation
- **MUST** Run `rug fmt` before committing — it formats **and** lints Rust (rustfmt + clippy), `web/` and `bin/`. `rug fmt --check` is the CI gate; `cargo fmt` alone covers only Rust formatting
- **MUST** Redirect long build/test runs to a log file — `rug test > /tmp/rug-test.log 2>&1 &`, then watch the file. Piping through `tail`/`head` buffers until EOF, so a hung run looks identical to a slow one and the filter can mask the exit status. On a stall, `pgrep` the nextest child binaries to find the test that has been running for minutes
- **MUST NOT** Never wait on a `pgrep -f` whose pattern appears in the watcher's own command line — the shell running the loop carries the pattern in its argv, so the watcher matches itself (and any sibling watchers) and spins forever while the watched run looks eternally busy. The mine is in the PATTERN, not the loop: rewriting the loop keeps the same match. Break the self-match with a bracket (`pgrep -f 'bin/[p]ull'` searches for `bin/pull` while the argv holds `bin/[p]ull`) or match the exact process name (`pgrep -x <binary>`)
- **MUST NOT** Do not add emoji unless asked to

## Hard rules (Rust)

- **MUST NOT** Never slice strings by byte index (`&s[..n]`, `&s[n..]`) — Rust strings are UTF-8 and a byte index may split a char and panic. Use `rugent_common::truncate_str(s, max_bytes)`
- **MUST NOT** Never swallow an error. A failing `Result`/`None` from any fallible op (network, DB, file I/O, attachment up/download, RPC, sending a user message, billing) is **propagated** (`?`), **logged with context** (`warn!`/`error!` with the relevant ids), or — on a user-facing path — **surfaced to the user**. clippy catches none of this. Banned when they drop a real failure: `if let Ok(..)`, `let _ = fallible();`, `.ok();`, empty `Err(_) => {}` arms, `.unwrap_or(..)`/`.unwrap_or_default()` masking a failure. (A dropped photo-download error once left the agent running on an empty turn in prod.)
- **SHOULD** Idiomatic exceptions stay fine — `let _ = tx.send(..)` on a maybe-dead channel, best-effort telemetry/typing indicators, genuine config defaults — but comment _why_ it is safe
- **MUST** `mod.rs` / `lib.rs` hold only module declarations and re-exports — no structs, impls or logic. Move implementation into a named submodule (`core.rs`, `types.rs`)

## Designing a fix

The failure modes of an agent product are not ordinary bugs, and the wrong-shaped fix costs more than the bug did. Every rule below is negative knowledge — the thing it forbids is **absent** from the tree, because it was removed at a cost. Reading the code cannot rediscover any of it.

- **MUST NOT** Never add a gate — any layer that inspects the agent's output and refuses it: a publish check, a pre-flight predicate, a validator, an output judge. It is a product feature, shipped only when the operator asks for it by name. Five shipped in good faith, all removed: a visual judge (#474); a destructive-command policy walked past by `python3 -c` five seconds after it fired (#493); a paid validator that charged 955₽ in 48h for verdicts on its own infrastructure failures (#341); a browser check that refused 76% of publishes on a healthy browser and caught nothing across 302 of them (#506). The last standing was the "deterministic, cannot false-positive" syntax gate: 300 firings, **zero** true positives, and the agent rewrote working user code to get past it (#600). **A gate's own author cannot tell whether it false-positives: the cases it mangles are the ones nobody thinks to try.** A proposal carries the burden of a _measured_ prod false-positive rate, not an argument about its shape
- **MUST NOT** Never wrap a weak tool in a gate — fix the tool. When a call is useless on some class of host (a GUI launched into session 0, a screenshot with nobody logged in, HKCU resolving to the service profile), make the _tool_ do the real thing or fail with an accurate, actionable error. Banned as "fixes": keyword/pattern gates, pre-flight "don't let the model call X here" checks, safety layers over a still-broken mechanism
- **MUST** Measure before claiming a tool is useless. `screenshot` already reached the desktop and already errored honestly without a session, so the gate proposed for it was pure overhead; one `rug host-exec` settled it (#466)
- **MUST** Size the fix to the evidence, **both ways**. For each part of a proposal, say what share of the observed cases it closes — a part that closes none is YAGNI. But a fix is not "simple" when its simplicity comes from ignoring evidence you already hold (#352: one line closed 10 of 12 cases and two further points were dropped; hardcoding the same value outright would have closed the same 10 and broken 11 real sessions). State the residue you deliberately leave, and why, in the commit and on the ticket
- **MUST** One fix = one layer. A proposal touching the prompt _and_ the scaffold _and_ a gate _and_ the tool is four fixes with one owner: not revertable, not measurable, not attributable. Prefer, in order: **the tool or generator** (what produces the defect) → **the prompt** (what chooses it) → nothing else. Name the dropped layers; if the chosen one does not hold, the next audit says so with data and you move one layer, deliberately

### Tool-call errors

- **MUST NOT** Never guess or salvage a malformed LLM tool call — return an accurate, schema-grounded error and let the model self-correct. State only the _verified_ cause, name the tool's real schema (exact required param names, or the valid tool names), never assert an unconfirmed one. Banned: Rust-side "did-you-mean" key-rename matching, fabricated arg shapes (`{"raw": …}`), misleading framing — calling a plain missing field "truncated in transit" made a weak model give up on the tool
- **MUST** The cause depends on the provider's arg transport, so the error builder is polymorphic on `rugent_llm::ToolArgTransport`: OpenAI-compat streams args as a JSON _string_ (truncatable → corrupt-JSON hints are real); GigaChat delivers a structured _object_ (never truncated → a missing field means the model omitted it)
- **MUST** All tool-call error text goes through the builders in `rugent_tools::tool_errors` — do not hand-roll at call sites
- **SHOULD** Lossless coercion of a known encoding is fine (`serde_lenient` accepting `"50"` for a number, stripping harmony tool-name framing) — that is decoding a wire format, not guessing intent

## Strict TDD Protocol

**MANDATORY for every bug fix and feature.** The only exceptions: thin binary glue in `rugent-bin/src`, purely mechanical edits (renames, formatting, doc text), and static marketing/landing content (`templates/`, `web/public/*.html`, their copy in `rugent-server/src/pages.rs`).

1. **Write a failing test FIRST**, next to the code under test, named for the bug or feature (`fn truncate_str_keeps_utf8_boundary`). DB-touching tests get an isolated schema via `db::connect_ephemeral_schema(&url)`.
2. **Verify it fails** for the right reason (`cargo test -p [crate] [name]`). If it passes, the test is wrong.
3. **Implement the minimal fix.** Track reasoning in test-file comments (`HYPOTHESIS:`, `SOLUTION:`, `RESULT:`) and undo every wrong-hypothesis change — no dead exploratory code in the tree.
4. **Verify green** (the tests for this change + `rug fmt`) and leave `STATUS: FIXED — see #123` in the test.

- **MUST** Stop and ask if you truly cannot write the test
- **MUST NOT** Never implement a fix before the failing test
- **MUST NOT** Never weaken a test to make it pass — assert correct behavior, never degraded-as-correct
- **MUST NOT** Never silently skip. A behavior that genuinely cannot be unit-tested (real WS peer, external API, visual) is written `#[ignore]` with a `// MANUAL:` repro plan, and you say so
- **MUST NOT** Never write trivial render tests for static pages, and **delete any you find**. A template render followed by `assert!(html.contains("…literal…"))` only restates the template — that covers marketing copy, nav, links, headings _and_ the mere presence of static meta/SEO tags. Test page _logic_: routing/redirects, URL **construction** (host-derived canonical/absolute `og:image`), conditional/data-driven rendering, handlers and validation. Test the builder (`seo_context`, `pay_context`, `business_lead_complete`), not the prose
- **MUST** Model-facing prose (prompt templates, tool descriptions) is guarded by the golden snapshots under `fixtures/golden/` ONLY. Editing a template or a tool description means re-blessing in the SAME commit — the failing test prints the exact command — and the fixture diff is the review surface
- **MUST NOT** Never duplicate a golden with substring tests, and **delete any you find**. A `tpl.contains("…")` needle restates the prose, rots silently (a retired-validator section lived in the prompt while 16 such tests stayed green) and taxes every prompt edit. Non-trivial invariants stay as code tests: cross-validation against the tool registry and `install::SLAVE_ARTIFACTS`, the path-scoped-checkout lint, cached-prefix stability, language selection

## Bug tracking

Bugs live in **GitLab issues, never in a file**; credentials come from `.env`. The CLI is `rug bug` and it validates every label value itself — `rug bug labels` prints the dictionary, an invalid value prints the valid set. The audit-time protocol (classifying a finding, measuring a behavior class, updating the tracker per window) belongs to the `bugs` skill, which is loaded when an audit runs. What follows applies to any turn.

- **MUST** Use `rug bug` for every interaction with the tracker — never the GitLab API directly. If a capability is missing, extend `bin/bug` (flag/subcommand + a unit test for its pure logic in `tests/comandlets/test_bug.py`), then use it
- **MUST** Issue titles, bodies and comments are in Russian. Code, tests, branch names and commits stay English
- **MUST** Every unit of work — bug or feature — is an issue BEFORE the fix. State the symptom, the root cause if known (never assert an unconfirmed one), the prod evidence (session ids, ₽ burned) and what to do
- **MUST** Every issue carries exactly one label from each of the three mandatory scoped groups (`status::`, `sev::`, `component::` — a second of the same group silently replaces the first). Pick the `component::` where the fix lands; name other affected subsystems in the body (`**Затрагивает также:** …`)
- **MUST** Classify before opening anything by asking **«which deterministic layer lied or stayed silent?»**, never «is this the model or the product?». Blame is unanswerable in an agent product and it buys gates: #245 was filed as «модельная поломка → circuit breaker», and a re-read found the guard already there, blinded by compaction. A finding where every deterministic layer told the truth is a **behavior class** — it gets no ticket per incident, only a comment on the standing closed `status::model` issue (`rug bug list --all --label status::model`); a finding where the action does not exist at all is a capability gap, not a defect (`--type feature`). The `bugs` skill carries the full three-way rule
- **MUST NOT** There is no `status::partial` — it is banned. Every ticket is atomic and carries one truth: `live` (the only open status) or `fixed`, or `model` for a standing rate. «Half-done» is the tell of a compound ticket. A landed fix that later regresses is a **new** atomic bug, never a reopened `partial`
- **MUST** `parked` (a real defect deliberately deferred) is legitimate **only** with a measured frequency and a named un-park trigger in the body — without both it is a dump, and `noise` («not a defect») is never a substitute for either it or `dup`
- **MUST** `sev::` is the harm **class**, not the queue rank — do not inflate it because a user was angry (that is `frust::hi`). `prio::` is **derived**: `rug bug relabel --prio auto` computes it from the ticket's own labels
- **MUST** One issue = exactly one bug or one task. The tells of a compound ticket: a symptom needing two unrelated root causes, a numbered list under «Что делать», a title joined by «, а …» or «… + …». Split it — one issue per item, each with its own three labels and evidence, cross-linked (`**Отделено от #iid:**` / `**Разбит на:** #a, #b`). The concrete harm is that one part can ship while the other stays live and a single `status::` cannot say so. When you split, fix the parent's TITLE too — a parent still advertising the half that moved out reads as a duplicate in every future audit
- **MUST** Search before opening — mandatory, not a courtesy. Grep the open list for the failing surface, the tool name, the error string and the symptom words (`rug bug list --all | grep -iE '…'`), then _read the bodies_: two tickets can share a root cause under different wording (#435/#447), and two can look identical while being separate work (#317/#354)
- **MUST** The test is **the fix, not the phrasing**: if one change in one place closes both, it is one issue. A shared root cause with several symptoms is one issue; several root causes under one heading are several. Same root cause → merge into the ticket with the strongest evidence, carrying over every distinct fact, then close the other `--status dup`. Same symptom, different mechanism → separate issues, cross-linked (`**Родня:** #a` in both bodies)
- **MUST** A guarded soft-degradation is not a bug. When a verify-contract catches degraded model output, refuses it, does **not** charge the user and the agent recovers on the same turn, that is `noise` or evidence on the standing class. It becomes a bug only when harm reaches the user or the wallet: a **charged** junk result, a result **silently dropped with no gate**, an auto-sent blank frame, a false «готово»
- **MUST** A single-session papercut with no confirmed root is a comment, not a ticket. The tell is all three at once: **one** session of evidence, `sev::low`, and a root you can only call «вероятный». It goes onto the nearest live ticket or standing class and becomes an issue when a second occurrence gives it a denominator — say in the comment that you deferred, so the second occurrence knows it is the second. Two exceptions: a behavior-class finding never gets a per-incident ticket at all, and a one-off whose harm is money or data loss is a ticket at n = 1

## Continuous delivery (solo CD)

Every push to `master` **deploys to prod** (the full suite in `build-server` gates the master-only `deploy` job via `needs:`). That job IS the suite gate — a red pipeline stops the deploy, so the developer machine runs only what the change is about and pushes.

- **MUST** Every ticket lives on its own branch `[type]/[iid][-slug]` (`type` = `bug`, `feat`, `hotfix`, `refactor`, `ci`, `docs`), branched from a fresh `origin/master`. Create it with `rug bug start [iid]`; the slug is ASCII — never transliterate the Russian title
- **MUST** Land by local integration, one push: `git merge --no-ff` the finished branch into a fresh local `master` (one merge commit per ticket keeps `git revert -m 1 [merge]` as the one-push rollback), run `rug fmt --check` on the **integrated** result, then `git push origin master` once (CI runs the suite and gates the deploy on it). Several tickets may ride one push and each stays individually revertable
- **MUST** Reference the issue in every commit subject as a **bare** `#74` — never a closing keyword (`close`/`fix`/`resolve`). The project auto-closes referenced issues on `master`, and a master push deploys, so a keyword would close the issue before you have recorded the closing commit and guarding test. Name each issue when a commit spans several
- **MUST** Close the ticket right after the merge-deploy: `rug bug close [iid] --comment "…"` naming the commit and the guarding test. Acceptance is the ticket's own tests, CI-gated and already satisfied at merge — prod is **not** a gating status, and there is no «landed but unverified» holding state
- **MUST** Hotfix escape: prod on fire → branch `hotfix/[iid]`, merge and push immediately; the master pipeline still gates the deploy
- **MUST** Watch the master pipeline you just triggered: it is the suite gate now, and a red `build-server` means the fix did NOT deploy. Fix forward or revert the merge (`git revert -m 1`), never leave a red master unattended
- **MUST NOT** Do not push a ticket branch just to run a pipeline — one push per landing, on `master`
- **SHOULD** If a fix introduced a new prod observable (a should-never-fire `error!` guard line), add checking it to the next audit's checklist instead of keeping the issue open. Any regression surfaces in the next daily audit as a _new_ atomic bug
- **SHOULD** `rug bug mr` / MR auto-merge remain available for work that wants review or must land unattended; they are no longer the default path
- **SHOULD** The TDD-protocol exceptions may be committed straight to `master`

## Documentation

Subsystem deep-dives live in `docs/`, one UPPERCASE file per subsystem (`ls docs/`).

- **MUST** New docs live in `docs/` under a single-word UPPERCASE name
- **SHOULD** When the user types a bare UPPERCASE word (`BILLING`, `SLAVE`, …), read `docs/[WORD].md`