fix(#856): gate scripts/ and contrib/ with ruff, scripts/ with pyright

[lmorchard]

Closes #856.

scripts/ (8 files) was checked by nothing: make lint / fmt covered src/
and tests/, and pyrightconfig.json included only src/decafclaw.
contrib/ (25 files) had the same ruff gap and was reached only by pytest via
testpaths.

The live bug the missing gate was hiding

scripts/build-eval-fixtures.py:

with tempfile.TemporaryDirectory() as tmp:
    config.data_home = tmp
    config.agent_id = "fixture"

Neither is a field on Config — they live on config.agent. Config isn't
frozen, so these silently created undeclared attributes that nothing read.
Two independent failures followed:

1. The redirect no-opped, so index_entry wrote to the real
   data/{agent_id}/workspace/embeddings.db — polluting the live embedding
   index.
2. The pickup path was independently wrong:
   Path(tmp)/"workspace"/"fixture" vs workspace_path's
   data_home/agent_id/workspace. Wrong component order, so the shutil.move
   would have failed even once the redirect worked.

So make build-eval-fixtures clobbered real agent data and then crashed.
This is exactly the anti-pattern CLAUDE.md names — "Don't setattr undeclared
attributes on Config/Context" — and pyright reports it for free.

Fixed with a fixture_config() helper using nested dataclasses.replace per
the dataclass-copy convention, and the pickup path is now derived from
build_config.workspace_path instead of hand-assembled.

check is now composed, not duplicated

check carried its own ruff check src/ tests/ line, so widening lint alone
would not have widened what CI gates. It is now:

check: install-js check-message-types lint typecheck check-js

Same duplication that let check-message-types and check-js run nowhere
before #854 — worth removing rather than adding a third copy.

Scope calls, both measured

contrib/ joins ruff but not pyright. ruff check contrib/ is clean
today, so it's free. pyright contrib/ reports 21 errors — its own
cleanup, which shouldn't ride along here.

The ruff format sweep is dropped, and #856's checklist item for it is
superseded by #857. The premise was that scripts/ was a formatting
outlier; it isn't. ruff format --check says 312 files across src/ and
tests/ would also be reformatted, so the repo has never been formatted at
all. I ran the 24-file sweep, verified it semantically inert, then reverted it:
formatting 2 of 4 directories is less consistent than formatting none, and
make check doesn't check formatting anyway, so it buys the gate nothing.
#857 covers the real problem, which is that make fmt produces a 312-file diff
for anyone who runs it.

Verification

• Tests written first and observed failing on the missing fixture_config.
  The third test pins the root cause — that data_home/agent_id are not
  Config fields, so attribute assignment is a silent no-op — so a future
  "simplification" can't reintroduce it. The module is loaded by path because
  its filename is hyphenated.
• Teeth-checked, all three paths: an F821 under scripts/ and under
  contrib/ each make make lint exit 2; a type error under scripts/
  makes pyright exit 1.
• make check exit 0 (ruff check src/ tests/ scripts/ contrib/ → all passed;
  pyright → 0 errors, 0 warnings; tsc --noEmit). make test → 3897 passed,
  2 skipped.
• The format sweep, before being reverted, was confirmed AST-equivalent for all
  24 files (0 genuine semantic differences) by comparing ast.dump with
  adjacent string Constants inside JoinedStr merged — ruff joins implicit
  f-string concatenation, which changes the raw token stream and AST without
  changing the runtime value. Recorded in make fmt is a 312-file landmine: the tree has never been ruff-formatted, and formatting is ungated #857 for whoever does the real sweep.

One thing worth a decision

pyright exits 0 when it reports only warnings, and
pyrightconfig.json sets reportAttributeAccessIssue: "warning" — which is
the rule that flagged this very bug. So including scripts/ makes pyright
report this bug class without failing on it. uv run pyright --warnings
would make warnings fatal, and it's free today (the repo is at 0 warnings) —
but it's a strictness change to the whole gate and re-escalates a rule that was
deliberately downgraded, so it isn't bundled here.

[Copilot]

🟡 Changes recommended

Resolve the Makefile ordering hazard and make the Pyright diagnostic enforceable.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Expands Ruff/Pyright coverage and fixes eval-fixture configuration and database path handling.

Changes:

• Adds scripts/ and contrib/ to Ruff coverage.
• Adds scripts/ to Pyright and composes make check.
• Uses nested configuration replacement with regression tests.

File summaries

File	Summary	Final review comment
tests/test_build_eval_fixtures.py	Tests fixture configuration isolation.	None.
scripts/build-eval-fixtures.py	Fixes temporary configuration and database path handling.	Nit (1 vote): Add coverage asserting main passes build_config.workspace_path / "embeddings.db" to shutil.move.
pyrightconfig.json	Includes scripts/ in Pyright.	Moderate (3 votes): The warning-only diagnostic does not fail Pyright; promote the rule or use --warnings if enforcement is intended.
Makefile	Expands Ruff coverage and composes check.	Moderate (1 vote): Preserve an ordering barrier so parallel checks cannot read files while message types are being generated.

Review details

Suppressed comments (2)

Makefile:105

• This refactor changes check-message-types from a prerequisite phase before the checks into a sibling prerequisite. With make -j check, the generator rewrites src/decafclaw/web/message_types.py and the JS/TS outputs while lint, Pyright, or check-js can read them, so a check can fail nondeterministically on a partially written generated file (the generator uses write_text). Preserve an ordering barrier from the generator to the other gates when composing this target.

check: install-js check-message-types lint typecheck check-js

scripts/build-eval-fixtures.py:69

• The new tests exercise fixture_config, but none exercises main's pickup path. If the old Path(tmp) / "workspace" / "fixture" is reintroduced here, all three tests still pass because they only assert the helper's workspace_path, not the source passed to shutil.move. Add a focused seam/test (or mock main's dependencies) that asserts the move source is build_config.workspace_path / "embeddings.db".

        built_db = build_config.workspace_path / "embeddings.db"

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.