docs: expand DoD with security, observability, and agent-workflow gates

Restructure DoD.md into numbered sections and add axes that were previously
implicit: security, observability/operability, reversibility, and explicit
guardrails for agent-assisted workflow (scope match, evidence-based edits,
pre-edit impact analysis, embeddings preservation).

Expand the validation baseline to reflect the real CI shape (shared-first
build ordering, prettier, setup-gitnexus action, CHANGELOG ownership) and
add a Review Gates checklist plus a "Not Done" signals section that flags
contract drift, language leakage into shared code, and unrelated churn.
This commit is contained in:
Gergo Magyar 2026-04-23 07:53:39 +01:00
parent 915fcd9dc5
commit dfc05d18f1

220
DoD.md
View file

@ -1,104 +1,170 @@
# Definition of Done — GitNexus
This document defines the repo-wide completion bar for production-ready changes in GitNexus.
Last reviewed: 2026-04-23 · Version: 2.0.0
This document defines the repo-wide completion bar for production-ready changes in GitNexus. It is the stable baseline. Implementation prompts, agent behavior, and review workflows may add task-specific checks, but they must never weaken this bar.
Use it together with:
- `AGENTS.md`
- `GUARDRAILS.md`
- `CONTRIBUTING.md`
- `TESTING.md`
- `ARCHITECTURE.md`
This file is the stable baseline.
Implementation prompts, agent behavior, and review workflows may add task-specific checks, but they should not weaken this bar.
- `AGENTS.md` — agent-facing rules of engagement
- `GUARDRAILS.md` — hard safety constraints
- `CONTRIBUTING.md` — contributor workflow
- `TESTING.md` — test strategy and coverage expectations
- `ARCHITECTURE.md` — pipeline boundaries, Call-Resolution DAG, LanguageProvider contract
## Core Definition of Done
## 1. Scope and Intent
A change is done only when all relevant items below are true:
A change is **Done** when it is correct, safely integrated, appropriately tested, operationally sound, and a net improvement to the codebase — not merely "the code compiles and a test passes."
- [ ] The requested behavior is implemented end-to-end in the real runtime path for the affected surface.
No dead code, partial wiring, or test-only behavior.
This DoD applies to:
- CLI, MCP, and HTTP-bridge behavior in `gitnexus/`
- Browser UI in `gitnexus-web/`
- Shared contracts in `gitnexus-shared/`
- CI workflows, release pipelines, and repo-level docs
Out of scope: full agent personas, step-by-step implementation prompts, verbose review formatting rules, repo walkthroughs already covered elsewhere, temporary task-specific acceptance criteria. Those belong in prompts, PR templates, or other repo docs.
## 2. Core Definition of Done
Every change must satisfy **every relevant item** below. If an item does not apply, say so explicitly in the PR description.
### 2.1 Correctness and Completeness
- [ ] The requested behavior is implemented end-to-end in the **real runtime path** for the affected surface — no dead code, partial wiring, test-only shims, or "works in isolation but not in production" seams.
- [ ] Edge cases relevant to the changed surface are handled or explicitly documented as out of scope.
- [ ] Error handling is proportionate: inputs at system boundaries (user input, external APIs, filesystem, process spawn) are validated; internal, framework-guaranteed paths are trusted.
- [ ] The change produces the same result on re-run (idempotent where expected) and does not rely on accidental ordering.
### 2.2 Architecture and Placement
- [ ] The change is placed in the correct package and layer:
- `gitnexus/` for CLI, MCP, HTTP bridge, ingestion, graph, and runtime logic
- `gitnexus-web/` for browser UI
- `gitnexus-shared/` for shared contracts, types, and constants
- `gitnexus/` for CLI, MCP, HTTP bridge, ingestion, graph, and runtime logic
- `gitnexus-web/` for browser UI (thin client — no WASM workers, all queries via HTTP API)
- `gitnexus-shared/` for shared contracts, types, and constants
- [ ] Pipeline and architecture boundaries remain explicit. Shared ingestion code in `gitnexus/src/core/ingestion/` must not name languages — use `LanguageProvider` hooks (see `AGENTS.md` and `ARCHITECTURE.md` § Call-Resolution DAG).
- [ ] No hidden cross-phase coupling; no leaking of language-specific logic into shared infrastructure without a documented architectural reason.
- [ ] Runtime and graph behavior are consistent — the real source of truth is fixed at the source, not symptom-patched in a downstream layer.
- [ ] Direct imports from `gitnexus-shared` are used. No barrel re-exports introduced to paper over drift between packages.
- [ ] Existing contracts are preserved unless the task explicitly requires a contract change.
Any contract change is intentional, explicit, and reflected in direct consumers.
### 2.3 Design and Readability
- [ ] The implementation is the smallest correct solution for the requirement.
No speculative abstraction, unnecessary indirection, clever but hard-to-follow control flow, or unrelated cleanup.
- [ ] The implementation is the **smallest correct solution** for the requirement. No speculative abstraction, unnecessary indirection, clever but hard-to-follow control flow, or unrelated cleanup.
- [ ] Naming, control flow, ownership, and extension points are clear enough that the next contributor can extend the code without archaeology.
- [ ] Comments are minimal and useful — they explain intent, invariants, contracts, or non-obvious constraints. No stale comments, placeholder comments, narrated code, commented-out code, or "what" comments where a good name would do.
- [ ] No copy-paste duplication created for convenience; no premature deduplication of three similar lines.
- [ ] The code is easy for the next contributor to understand and extend.
Naming, control flow, ownership, and extension points are clear.
### 2.4 Contracts and Compatibility
- [ ] Comments are minimal and useful.
They explain intent, invariants, contracts, or non-obvious constraints.
No stale comments, placeholder comments, narrated code, or commented-out code.
- [ ] Existing contracts (types in `gitnexus-shared/`, CLI flags, MCP tools/resources, HTTP routes, graph node/edge shapes, persisted IDs) are preserved unless the task explicitly requires a contract change.
- [ ] Any contract change is intentional, explicit, and reflected in **every direct consumer** in the same change, with types aligned end-to-end.
- [ ] Persisted data changes (graph schema, IDs, embeddings) are backward-compatible or accompanied by a documented migration / reindex path.
- [ ] If user-visible behavior, public usage, CLI help, or README examples change, the relevant docs, examples, help text, or migration notes are updated in the same change.
- [ ] Performance is acceptable for the affected path.
No repeated avoidable work, unnecessary scans, unnecessary round-trips, unbounded caches, or obvious hot-path regressions.
### 2.5 Security
- [ ] Tests cover the real changed path.
They would fail if behavior, wiring, or contracts were broken.
Assertions are meaningful and fixtures are realistic enough for the risk of the change.
- [ ] No new injection surfaces (command, path, SQL/Cypher-style, prompt) introduced on paths that consume untrusted input.
- [ ] No secrets, tokens, or credentials committed to the repo, to logs, or to error messages.
- [ ] Filesystem access honors the repo-scope and indexed-repo boundaries documented in `AGENTS.md` and `GUARDRAILS.md`.
- [ ] Third-party dependencies added or bumped are justified, from reputable sources, and do not regress the supply-chain posture.
- [ ] Required validation for the touched area has been run, or any gap is explicitly called out in the final handoff / PR description.
### 2.6 Performance and Resource Use
- [ ] Residual risks, compatibility impacts, and operational concerns are either resolved or clearly stated.
- [ ] No repeated avoidable work, unnecessary scans, unnecessary round-trips, unbounded caches, or obvious hot-path regressions.
- [ ] Tree-sitter buffer sizing follows the adaptive 512KB–32MB convention (`getTreeSitterBufferSize`) — do not hard-code new buffer sizes.
- [ ] Memory and handle lifecycles are explicit: database handles (LadybugDB) close cleanly, no dangling process watchers, no leaked tree-sitter parsers.
- [ ] Long-running or large-graph paths remain bounded or are measurably streamed; degradation on large real repos is considered, not assumed benign.
## GitNexus-Specific Requirements
### 2.7 Tests
Apply the following when relevant to the change:
- [ ] Tests cover the **real changed path** — they would fail if behavior, wiring, or contracts were broken, not only if a mock were misconfigured.
- [ ] Integration tests hit a real database where the production path does; do not introduce mocks that hide migration or schema drift.
- [ ] Assertions are meaningful. Use `toBe` / `toEqual` for exact expectations; avoid `toBeGreaterThanOrEqual` and other bounds-only assertions that mask regressions.
- [ ] Fixtures are realistic enough for the risk of the change — a one-file fixture is not sufficient for a pipeline-wide behavior change.
- [ ] New tests are deterministic and do not depend on network, clock, or host-specific paths without explicit isolation.
### Shared contracts and cross-surface changes
### 2.8 Observability and Operability
- [ ] If `gitnexus-shared/` changes, direct CLI and web consumers are verified together.
- [ ] If API shapes, MCP tool/resource behavior, or HTTP bridge behavior changes, handlers and consumers remain aligned end-to-end.
- [ ] If user-visible behavior or public usage changes, the relevant docs, examples, help text, or migration notes are updated in the same change.
- [ ] Errors surfaced to users or callers are actionable: they name what failed, what input was involved (without leaking secrets), and how to recover where possible.
- [ ] Logging is proportionate — no noisy debug logs left in hot paths, no silent catches that swallow diagnostics.
- [ ] CLI exit codes and MCP tool responses are correct for each outcome (success, user error, internal error).
- [ ] Progress reporting (`PipelineProgress` and similar shared contracts) remains accurate after the change.
### Ingestion, graph, and language-support changes
### 2.9 Reversibility and Risk
- [ ] Pipeline and architecture boundaries remain explicit.
Do not introduce hidden cross-phase coupling or leak language-specific logic into shared infrastructure without a clear architectural reason.
- [ ] The change has a clear rollback story: revert is safe, or migration is accompanied by a documented rollback / reindex procedure.
- [ ] Residual risks, compatibility impacts, and operational concerns are either resolved or **clearly stated** in the PR description.
- [ ] Destructive or hard-to-reverse operations (graph rebuild, schema change, `git` state manipulation) are opt-in or guarded.
- [ ] Runtime and graph behavior remain consistent.
Do not patch symptoms in one layer while leaving the real source of truth inconsistent in another.
## 3. Agent-Assisted Workflow Guardrails
- [ ] If the change touches indexing, query, impact analysis, rename flows, or route/tool mapping, the affected runtime path is validated through the real entry points, not only isolated helpers.
When the change is produced with or reviewed by an AI agent, the following additional gates apply:
### Agent / graph-assisted workflow
- [ ] **Scope match.** The final diff matches the intended symbols, files, and processes — no speculative refactors, unrelated formatting churn, or collateral edits outside the task scope.
- [ ] **Evidence-based edits.** Claims about repo state are verified against the current code, not trusted from memory or stale documentation.
- [ ] **Impact analysis.** Where GitNexus graph tooling is available and relevant, impact of non-trivial symbol, contract, or runtime-path changes is checked **before** editing.
- [ ] **Embeddings preserved.** If an indexed repo already has embeddings and re-analysis is required, embeddings are preserved — not accidentally dropped by a destructive reindex.
- [ ] **No false-done.** "Done" is claimed only after the Validation Baseline below has been run or any gap is explicitly named. Green tests on an unrelated path do not constitute validation.
- [ ] **Five-axis self-review** before handing off: correctness, readability, architecture, security, performance.
- [ ] Where graph tooling is available and relevant, impact is checked before changing non-trivial symbols, contracts, or runtime paths.
- [ ] Before finalizing, diff scope is checked so the resulting change matches the intended symbols, files, and processes.
- [ ] If an indexed repo already has embeddings and re-analysis is required, preserve embeddings rather than accidentally dropping them.
## 4. Validation Baseline
## Validation Baseline
Run the commands relevant to the touched area. If something cannot be run in the current environment, state it explicitly in the handoff.
Run the commands relevant to the touched area.
If something cannot be run in the current environment, say so explicitly.
### 4.1 Build ordering
### If `gitnexus/` changed
- [ ] `gitnexus-shared/` dist is built before consuming packages are typechecked or tested (CI uses the `setup-gitnexus` action for this — local runs must match).
### 4.2 If `gitnexus/` changed
- [ ] `cd gitnexus && npx tsc --noEmit`
- [ ] `cd gitnexus && npm test`
- [ ] `cd gitnexus && npx prettier --check .` for files in the diff (pre-commit runs the affected-tests subset; do not expand scope)
### If `gitnexus-web/` changed
### 4.3 If `gitnexus-web/` changed
- [ ] `cd gitnexus-web && npx tsc -b --noEmit`
- [ ] `cd gitnexus-web && npm test`
- [ ] `cd gitnexus-web && npm run test:e2e` when browser flows or user-facing UI behavior changed
### If `gitnexus-shared/` changed
### 4.4 If `gitnexus-shared/` changed
- [ ] Shared package outputs stay compatible with direct consumers
- [ ] Dependent packages still typecheck after the shared change
- [ ] Shared package builds cleanly (`npm run build` in `gitnexus-shared/`)
- [ ] Dependent packages still typecheck and test after the shared change — verify both CLI and web consumers together
## Task-Specific DoD Template
### 4.5 If CI workflows or release pipelines changed
Use this in implementation and review prompts.
Keep it short and tailor it to the actual change.
- [ ] The workflow passes a dry-run or triggered run before merge; concurrency (`cancel-in-progress`) and the `setup-gitnexus` action remain wired correctly.
- [ ] `CHANGELOG.md` is **not** edited here — it is owned by the release process.
## 5. Review Gates
A reviewer (human or agent) should be able to answer **yes** to each of the following before approving:
1. **Correctness** — Does the change do what it claims on the real runtime path?
2. **Readability** — Will the next contributor understand this in six months without asking?
3. **Architecture** — Is it in the right package, layer, and phase? Are boundaries respected?
4. **Security** — No new injection, leak, or trust-boundary violation?
5. **Performance** — No obvious regression on realistic inputs?
6. **Tests** — Would a regression in the changed behavior fail loudly?
7. **Scope** — Does the diff match the intended change, with no unrelated churn?
## 6. "Not Done" Signals
A change is **not** Done if any of the following is true, even if CI is green:
- The runtime path is not actually exercised by the tests.
- A contract drifted between `gitnexus/`, `gitnexus-web/`, and `gitnexus-shared/` and only one side was updated.
- A language-specific concern leaked into shared ingestion code.
- The diff contains unrelated reformatting, refactors, or cleanup beyond the stated task.
- Logs, comments, or TODOs were added as placeholders for work not done.
- The change depends on a manual step that is not documented.
- `CHANGELOG.md` was edited during PR work.
- Pre-commit, prettier, or typecheck was bypassed without explicit justification.
## 7. Task-Specific DoD Template
Use this in implementation and review prompts. Keep it short and tailor it to the actual change:
```md
# Definition of Done for this implementation
@ -108,28 +174,36 @@ Keep it short and tailor it to the actual change.
- [ ] The design stays scoped, readable, and proportionate to the task.
- [ ] Tests prove the changed behavior and catch broken wiring.
- [ ] Required validation for touched packages has been run, or any gap is explicitly noted.
- [ ] Repo boundaries, performance expectations, and operational safety are respected.
- [ ] Repo boundaries, security, performance, and operational safety are respected.
- [ ] The diff contains only the intended change — no unrelated churn.
```
## How to Use This File in Claude Review
## 8. How to Use This File in Claude Review
Reference this file as the repo-wide completion bar.
Then add a task-specific review instruction such as:
Reference this file as the repo-wide completion bar. Add a task-specific review instruction such as:
```md
Review this change against `DoD.md` and the repo docs (`AGENTS.md`, `GUARDRAILS.md`, `CONTRIBUTING.md`, `TESTING.md`, `ARCHITECTURE.md`).
Treat `DoD.md` as the minimum bar for production readiness.
Flag anything that is partially wired, contract-unsafe, under-tested, architecturally misplaced, or harder to maintain than necessary.
Review this change against `DoD.md` and the repo docs (`AGENTS.md`, `GUARDRAILS.md`,
`CONTRIBUTING.md`, `TESTING.md`, `ARCHITECTURE.md`). Treat `DoD.md` as the minimum
bar for production readiness. Flag anything that is partially wired, contract-unsafe,
under-tested, architecturally misplaced, scope-creeping, or harder to maintain than
necessary. Apply the five-axis review gate: correctness, readability, architecture,
security, performance.
```
## What Does Not Belong in This File
## 9. Evolution
To avoid duplication and drift, this file should not contain:
This DoD is living. Revisit it when:
- full agent personas or role-play instructions
- step-by-step implementation prompts
- verbose review output formatting rules
- detailed repo walkthroughs already covered by other docs
- temporary task-specific acceptance criteria
- A class of incident slips past it (add a gate).
- A gate becomes consistently ceremonial without catching issues (remove or merge it).
- The architecture evolves in a way that changes what "done" means (update placement, validation, or contracts sections).
Those belong in prompts, PR templates, or the relevant repo docs.
Track material updates in the changelog below. Keep the file tight — if it grows past a single read-in-one-sitting, something has drifted into the wrong place.
## Changelog
| Date | Version | Change |
| ---------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 2026-04-23 | 2.0.0 | Restructured into numbered sections; added Security, Observability, Reversibility, Agent-Assisted Guardrails, Review Gates, Not-Done Signals; expanded validation baseline (shared-first build, prettier, CI workflow checks). |
| 2026-04-13 | 1.0.0 | Initial repo-wide Definition of Done. |