Uh oh!
There was an error while loading. Please reload this page.
docs: an RFC-style SPEC.md for the whole product - #44
Merged
Conversation
The full service specification — schemas, lifecycle, tool policy, the send path — precise enough to reimplement cerber from, written against the code and tests (which win on disagreement; known divergences are recorded in its Appendix B rather than silently resolved). It already describes the review history (#42) and the settled-row reopen rule (#43), so it should land after them. README and CLAUDE.md point at it, CLAUDE.md with the keep-it-true rule that stops it rotting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an RFC-style SPEC.md intended to normatively describe Cerber’s behavior (artifact schema, lifecycle, daemon, runner/tool policy, API, CLI, and GitHub write boundary), and updates existing documentation to point readers at the spec as the engineering reference.
Changes:
- Add
SPEC.mdas a full service specification for Cerber. - Link to
SPEC.mdfromREADME.mdin the lifecycle/docs area. - Update
CLAUDE.mdarchitecture guidance to require keeping the spec in sync with behavior changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| SPEC.md | Adds a comprehensive RFC-style specification covering data model, lifecycle, daemon, runner, API, CLI, and security invariants. |
| README.md | Adds a link and brief explanation positioning SPEC.md vs docs/lifecycle.md. |
| CLAUDE.md | Adds guidance that behavior changes described by the spec must update SPEC.md (or be recorded as divergences). |
Suppressed comments (4)
SPEC.md:1268
- The HTTP API table says
PATCH /api/reviews/:key“stampssettledAt, clearsfiled”, but the current handler only updatesstatus/verdictRecommendationand doesn’t setsettledAtor clearfiled(src/server/index.ts:351-379). Either update the route implementation or adjust the spec (and/or record this in Appendix B) so the API contract matches reality.
| `GET /api/reviews/:key` | one artifact | 404 |
| `PATCH /api/reviews/:key` | settle | only `reviewed`/`skipped` accepted (§8.1); stamps `settledAt`, clears `filed` |
| `POST/PATCH/DELETE …/comments[/:id]` | comment CRUD | delete is user-origin only in the UI |
SPEC.md:1607
- Appendix A maps “§5 state store, history” to
src/core/history.ts, but that file does not exist in the current repository. If history is meant to live elsewhere (or isn’t implemented yet), this mapping should be corrected so implementers can actually find the reference code.
| §4 domain model | `src/core/artifact.ts` |
| §5 state store, history | `src/core/state.ts`, `src/core/history.ts` |
| §6 configuration | `src/core/config.ts` |
SPEC.md:1383
- The CLI section documents a
cerber history <pr>command, but the current CLI does not define ahistorysubcommand (seesrc/cli/index.ts, no.command("history")). If the command is planned but not yet shipped, it should be called out as a divergence / future work; otherwise update the CLI or remove this from the spec.
and `running` artifacts.
- **`history <pr>`** — prints the review's history (§5.4): one line per
entry with local timestamp, actor, what happened, and the cause; says
plainly when a review predates the history being kept.
- **`list`**, **`export <pr>`** (renders the markdown document; never touches
SPEC.md:379
- §5.4 specifies history behavior (store re-reads from disk on every save, ignores caller-supplied history, appends derived entries). None of this exists in the current
saveArtifactpath, which is a straightforward tmp+rename write (src/core/state.ts:18-25) with no pre-read/merge or derived history. The spec should either be rebased onto the implementation that adds this, or softened/flagged as a known divergence so it doesn’t mis-specify current behavior.
**Appended by the store, never by callers.** The save path itself derives and
appends history: it re-reads the file from disk on **every** save — even when
the caller just read it, because two writers share these files and appending
to the caller's copy would drop whatever the other recorded in between — and
**ignores any history the caller hands in**; disk is the only current copy.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+187
to
+190
| | `filed` | FiledInfo \| null | default `null`; never set on a sent artifact | | ||
| | `settledAt` | ISO-8601 string \| null | default `null`; when the row was settled — see below | | ||
| | `refresh` | RefreshInfo \| null | default `null` | | ||
| | `calibration` | Calibration \| null | default `null` | |
Comment on lines
+192
to
+195
| | `pendingChat` | PendingChat \| null | default `null` | | ||
| | `preChat` | ReviewSnapshot \| null | default `null` | | ||
| | `history` | HistoryEntry[] | OPTIONAL, **no default** — absent means the artifact predates history being kept, which surfaces MUST say rather than showing an empty log (§5.4) | | ||
Uh oh!
There was an error while loading. Please reload this page.
🎉 This PR is included in version 0.27.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds SPEC.md, a full service specification in the style of RFC-numbered specs: the artifact schema and every enum, the state store and history log, the seven-status lifecycle with its guards, the daemon's discovery/filing/reopen rules, the runner's tool policy and credential hygiene, the chat protocol, re-anchoring, the single GitHub write and auto-send, plus reference algorithms and a conformance checklist. It is written against the code and tests — which win on disagreement — and its Appendix B records the divergences found while writing it (stale CLI version string, README's re-review carryover claim, the CODE_REVIEW.md severity ladder,
allowUserComments) rather than resolving them silently. The spec already describes the review history (#42) and the settled-row reopen rule (#43) as landed, so this should merge after those two. README's "How it works" and CLAUDE.md's Architecture section now point at it, with CLAUDE.md carrying the keep-it-true rule: a PR that changes specified behavior updates the spec in the same PR.🤖 Generated with Claude Code
Open workspace in Conductor