# Repo Conventions

> NeuroLink's review standards — the critical rules to enforce, what NOT to comment on, the security bar, hot paths. Load before reviewing any change in this repository.

- Skill: `juspay/repo-conventions` (Agent Skill)
- Install (CLI): `npx skillmds@latest add juspay/repo-conventions`
- Raw SKILL.md: https://api.skillmd.com/api/skills/juspay/repo-conventions/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: juspay (https://skillmd.com/u/juspay)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/juspay/repo-conventions

---


# Reviewing a change in NeuroLink

Authoritative sources in this checkout (the repository root is `../`):

- `../CLAUDE.md` — the engineering rules, long form. When a finding violates one,
  cite the specific rule (e.g. "Critical Rule 1: dynamic imports only in the registry").
- `../CONTRIBUTING.md` — contribution and commit conventions.

This skill is the review-focused digest; those files win on any conflict.

## Do NOT comment on — CI already owns it

Formatting, lint and type errors — including CLAUDE.md rules 2 and 7–15 — are
enforced by ESLint + Prettier + tsc and the custom AST rules in `eslint-rules/`
(`interface` vs `type`, type file locations, barrel rules, double assertions,
e2e-only tests). Re-reporting mechanical violations buries the findings that
matter. Also skip: performance micro-costs, naming and prose taste, dependency
choice.

## The critical rules the review DOES enforce (cite the rule number)

- **Rule 1 — dynamic imports only in the registry.** Providers are imported
  dynamically inside factory functions in `providerRegistry.ts`; a static
  provider import is a circular-dependency bug.
- **Rule 3 — Gemini tools + JSON-schema structured output are mutually
  exclusive.** Gated on `isGeminiProvider` in `structuredOutputPolicy.ts`, NOT
  on Vertex as a whole (Vertex Claude supports both). Claude paths must default
  `max_tokens` via `resolveClaudeMaxTokens`, never a hardcoded 4096; truncation
  must surface (`jsonRepaired` / `jsonTruncated`), never silently.
- **Rule 4 — CLI ≠ SDK.** Manual MCP connections are CLI-only; CLI concerns
  must not leak into the SDK path.
- **Rule 5 — backward compatibility of the public SDK API.** Non-negotiable.
  Name the unmodified callers a change breaks; use the code graph to find them.
- **Rule 6 — `formatProviderError` returns the error, never throws.**
- **Rule 15 — tests are end-to-end only.** Suites drive `dist/index.js` or the
  built CLI (`node dist/cli/index.js`); ONE module graph per suite — mixing
  `src/` and `dist/` imports breaks stubs/spies/`instanceof` silently. A
  determinism exception must be declared in the file header and in the ESLint
  `allow` list.
- Provider `executeStream` goes through `BaseProvider.stream()` tool-merge.
- Factory + Registry is the extension pattern (providers, processors,
  chunkers, rerankers) — a new extensible thing that bypasses it is a finding.

## Security — the CRITICAL bar

Hardcoded secrets or credentials in source; secrets leaking into logs (confirm
`transformParamsForLogging` / secret stripping before logging provider params);
injection, unsafe eval/innerHTML/dynamic require, SSRF, path traversal; unsafe
handling of user or model input. Every CRITICAL claim must be anchored in the
change: quote the exact added line(s) from the diff. Placeholder values (docs,
tests, "sk-your-key-here", `${ENV_VAR}` references) are NOT leaked secrets — a
fabricated credential-leak report is worse than a missed real one.

## What the review spends itself on

Logic and correctness bugs, races, unhandled rejections; provider / MCP /
streaming / proxy-pool changes; missing error handling; backward compatibility
(rule 5); missing coverage in the matching `test/continuous-test-suite-*.ts`
for new behaviour.

## Out-of-diff impact — use the code graph when available

The diff alone does not show the damage a change does elsewhere. Orient once
(architecture overview, impact radius of the changed files, affected flows),
order the file-by-file pass by blast radius, and for each changed export check
its callers and dependents — signature, return shape, nullability, thrown
errors, async behaviour, side effects an UNMODIFIED caller does not handle.
Name the exact out-of-diff call sites. If the graph is unavailable, say impact
analysis was skipped — never fabricate impact claims.

## Hot paths — report at MAJOR or higher, and name the blast radius

| Path                                      | Why                                                 |
| ----------------------------------------- | --------------------------------------------------- |
| `src/lib/core/baseProvider.ts`            | Central `stream()` tool-merge every provider rides  |
| `src/lib/factories/providerRegistry.ts`   | Dynamic-import registry — the circular-dep tripwire |
| `src/lib/types/**`                        | The public surface rule 5 protects                  |
| `src/lib/server/routes/*ProxyRoutes.ts`   | Pool engines holding subscription credentials       |
| `src/lib/auth/**`                         | Token stores, OAuth refresh                         |
| `src/lib/mcp/**`                          | What tools an agent can reach                       |
| `src/lib/context/**`, `src/lib/memory/**` | Compaction/memory — silent data loss lives here     |

## Discipline

- Review file by file; skip lockfiles, generated or minified assets, `dist/`,
  `build/`, `coverage/`, images.
- Read code from THIS checkout — the platform is for the pull request's
  comments and metadata only.
- Conventional Commits with a required scope; ONE commit per pull request
  (squash-merged). Releases are generated from commit history.