docs(core): add Basic Machines agent style guidance (#921)

Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

.agents/skills/basic-machines-review/SKILL.md

@@ -0,0 +1,68 @@
+---
+name: basic-machines-review
+description: Use when reviewing Basic Machines code for house style, architecture risk, pre-merge hardening, or whether a change fits basic-memory/basic-memory-cloud conventions.
+license: MIT
+---
+
+# Basic Machines Review
+
+Use this skill for repo-local review passes where ordinary code review needs Basic Machines
+house style and architecture judgment. Report findings only; do not edit code unless the user
+asks you to fix specific findings.
+
+## Scope
+
+Review the current diff or named files against:
+
+- The repo's `AGENTS.md` / `CLAUDE.md`
+- `docs/ENGINEERING_STYLE.md`
+- The touched code paths and tests
+
+Apply only the guidance for the active repo. In `basic-memory`, prioritize local-first
+file/database/MCP boundaries. In `basic-memory-cloud`, prioritize tenant/workspace isolation,
+cloud worker behavior, and web-v2 state/runtime boundaries.
+
+## Review Rubric
+
+Report only concrete, falsifiable risks:
+
+- **Cognitive load:** Is the change harder to understand than the problem requires?
+- **Change propagation:** Will one product change force edits across unrelated layers?
+- **Knowledge duplication:** Is the same rule encoded in multiple places that can drift?
+- **Accidental complexity:** Did the change add abstractions, fallbacks, or state without need?
+- **Dependency direction:** Are API/MCP/CLI, services, repositories, and UI stores respecting
+  their intended boundaries?
+- **Domain model distortion:** Do names and types still match the product concept, or did a
+  transport/storage detail leak into the domain?
+- **Test oracle quality:** Would the tests fail for the bug or regression the change claims to
+  protect against?
+
+## House Rules To Check Explicitly
+
+- No speculative `getattr(obj, "attr", default)` for unknown model shapes.
+- No broad exception swallowing, warning-only failure paths, or hidden fallback behavior.
+- No casts or `Any` that hide an unclear type relationship.
+- Dataclasses for internal value/result objects; Pydantic at validation/serialization
+  boundaries.
+- Narrow `Protocol`s when only a capability is needed.
+- Explicit async/resource ownership, cancellation, and cleanup.
+- Meaningful regression tests or verification for risky changes.
+- Comments explain why, not what.
+
+## Reporting Format
+
+Lead with findings ordered by severity. Each finding should include:
+
+| Severity | Use for |
+| -------- | ------- |
+| `high` | A likely correctness, security, data-loss, or tenant/workspace isolation failure |
+| `medium` | A concrete maintainability or boundary risk that can cause future defects |
+| `low` | A minor consistency issue, ambiguous guidance, or review-only cleanup |
+
+```text
+severity | file:line | risk category | claim
+Why: concrete behavior or code path that proves the risk.
+Fix: smallest practical change, or "none obvious" if the risk needs product input.
+```
+
+If there are no findings, say so and note any verification gaps that remain.

.claude/skills/basic-machines-review

@@ -0,0 +1 @@
+../../.agents/skills/basic-machines-review

AGENTS.md

@@ -108,13 +108,34 @@ Before opening or updating a PR, run the checks that mirror the common required
 - Follow the repository pattern for data access
 - Tools communicate to api routers via the httpx ASGI client (in process)
 
+### Programming Style
+
+See [docs/ENGINEERING_STYLE.md](docs/ENGINEERING_STYLE.md) for the fuller house style. The
+short version for agents:
+
+- Prefer type-safe, explicit designs over object-heavy indirection. Use Python 3.12 `type`
+  aliases, full annotations, and narrow `Protocol`s when a caller only needs a capability.
+- Use dataclasses for internal value objects and operation results; use Pydantic v2 at API,
+  CLI, MCP, and persistence boundaries where validation and serialization matter.
+- Keep async boundaries obvious. Resource-owning code should use context managers, propagate
+  cancellation, and avoid hidden background work unless the lifecycle is explicit.
+- Fail fast. Do not add silent fallback logic, broad exception swallowing, speculative
+  `getattr`, or casts that hide an unclear model shape.
+- Keep control flow simple and local. Push branching decisions up, keep leaf helpers focused,
+  and name values after the domain concept they carry.
+- Use evidence-first testing. Add or update meaningful regression tests for bugs and risky
+  behavior, prefer real code paths over mocks, and run the narrowest command that proves the
+  change before widening verification.
+- Comments should explain why a branch, invariant, or constraint exists. Avoid comments that
+  merely narrate obvious code.
+
 ### Code Change Guidelines
 
 - **Full file read before edits**: Before editing any file, read it in full first to ensure complete context; partial reads lead to corrupted edits
 - **Minimize diffs**: Prefer the smallest change that satisfies the request. Avoid unrelated refactors or style rewrites unless necessary for correctness
-- **No speculative getattr**: Never use `getattr(obj, "attr", default)` when unsure about attribute names. Check the class definition or source code first
-- **Fail fast**: Write code with fail-fast logic by default. Do not swallow exceptions with errors or warnings
-- **No fallback logic**: Do not add fallback logic unless explicitly told to and agreed with the user
+- **House style is canonical**: Follow the Programming Style section above for type-safe,
+  fail-fast code; do not hide unclear models with speculative attributes, broad exception
+  handling, casts, or unapproved fallback logic
 - **No guessing**: Do not say "The issue is..." before you actually know what the issue is. Investigate first.
 
 ### Literate Programming Style

docs/ENGINEERING_STYLE.md

@@ -0,0 +1,64 @@
+# Basic Memory Engineering Style
+
+Style is how we make code easier to verify. Prefer explicit, typed, local-first code that
+preserves the file system as the source of truth while keeping the database, API, and MCP
+surfaces in sync.
+
+## Design Center
+
+- Basic Memory is local-first. Markdown files are the durable source; SQLite/Postgres indexes
+  are derived state that should be rebuilt or reconciled from files when needed.
+- Keep the existing boundary order: CLI/MCP/API entrypoints compose dependencies, services own
+  business behavior, repositories own database access, and file services own filesystem writes.
+- MCP tools should remain atomic and composable. They should call API routers through typed MCP
+  clients, not reach around into services.
+- Prefer small, explicit abstractions that match a real domain boundary. Avoid object
+  hierarchies when a function, dataclass, type alias, or protocol describes the concept better.
+
+## Types And Data
+
+- Use full type annotations and Python 3.12 syntax. Introduce `type` aliases for repeated
+  structured shapes, callback signatures, or domain concepts that would otherwise become
+  anonymous `dict[str, Any]` values.
+- Use dataclasses for internal values, operation inputs, and service results. Prefer
+  `frozen=True` when the value should not change and `slots=True` when identity/dynamic
+  attributes are not needed.
+- Use Pydantic v2 at boundaries that validate, serialize, or deserialize data: API payloads,
+  CLI/MCP schemas, configuration, and persistence-adjacent schemas.
+- Use narrow `Protocol`s when a caller needs a capability rather than a concrete repository or
+  service. Keep protocols small enough that fake implementations in tests are obvious.
+- Avoid speculative `getattr`, broad casts, or `Any` as a way to paper over uncertainty. Read
+  the model or schema definition and make the type relationship explicit.
+
+## Control Flow And Resources
+
+- Fail fast when an invariant is broken. Do not swallow exceptions, add warning-only error
+  handling, or introduce fallback behavior unless the user explicitly agrees to that behavior.
+- Keep control flow simple and close to the domain decision. Push `if` statements up into the
+  function that owns orchestration; keep leaf helpers focused on computation or one side effect.
+- Make async/resource boundaries visible with context managers and explicit lifecycles. Do not
+  start background work without a clear owner, cancellation story, and verification path.
+- Keep file mutations centralized through the existing file utilities/services so checksum,
+  atomic write, and index synchronization behavior stays coherent.
+
+## Testing And Verification
+
+- Use evidence-first testing, not mechanical TDD. For bugs and risky behavior, add or update a
+  regression test that would catch the failure. For small documentation-only edits, use the
+  relevant doc/repo hygiene checks.
+- Prefer tests that exercise real code paths. Use mocks, doubles, or `monkeypatch` only when
+  the external boundary would be slow, nondeterministic, or impossible to trigger directly.
+- Keep coverage at 100% for new code. Use `# pragma: no cover` only for code that would require
+  disproportionate mocking and is covered through an integration or runtime path.
+- Start with targeted commands, then widen as risk grows: focused pytest, `just fast-check`,
+  `just doctor`, package checks for agent packaging changes, and full SQLite/Postgres gates
+  when behavior crosses shared boundaries.
+
+## Comments And Names
+
+- Name values after the domain concept they carry: project, entity, permalink, tenant, route,
+  checksum, observation, relation, batch, or index state.
+- Comments should say why a branch, invariant, retry, lifecycle, or compatibility constraint
+  exists. Section headers are useful when a function or file has clear phases.
+- Avoid comments that restate the code. If a comment cannot explain a decision, simplify the
+  code or improve the name instead.