Skip to content

docs: add CLAUDE.md with the rules an agent would otherwise get wrong - #26

Merged
drakulavich merged 3 commits into
mainfrom
docs/add-claude-md
Jul 26, 2026
Merged

docs: add CLAUDE.md with the rules an agent would otherwise get wrong#26
drakulavich merged 3 commits into
mainfrom
docs/add-claude-md

Conversation

@drakulavich

Copy link
Copy Markdown
Owner

This repo had no agent instruction file. Rather than write a tour of the codebase, I applied Anthropic's criterion from the start — "would removing this cause the agent to make mistakes?" — so it opens at 68 lines / 679 words instead of growing into something that has to be trimmed later.

Every claim is verified against the source.

The rules that matter most

Three of them are things this repo actively contradicts elsewhere, which is exactly why an agent needs them stated:

  1. There is a build step. package.json#bindist/index.js, not src/. That is the opposite of the usual Bun setup, and a stale dist/ behind a linked binary silently swallows changes.
  2. The version lives in two placespackage.json#version and the VERSION constant in src/index.ts, with nothing enforcing the pair. They are drifted right now: 0.4.5 shipped (chore(release): 0.4.5 #17) with the constant left at 0.4.4, so oura-cli --version under-reports. See below.
  3. Publishing is npm Trusted Publishing (id-token: write + --provenance, release.yml:10,54). CONTRIBUTING.md still instructs maintainers to configure an NPM_TOKEN secret the workflow does not use.

Also recorded

citty, not Commander (docs/ARCHITECTURE.md still names Commander; there is no such dependency) · the downward-only layer rule lib → api/db → commands → index · both output modes mandatory, with docs/schemas/ as a published external contract · CliError + ErrorCode as the only errors that may reach the surface, so emitError/exitCodeFor are not bypassed · the --frozen-lockfile trap that blocked CI twice (#15, #16) · co-located tests, named exports only, .js suffix on local imports · Git LFS for assets/*.gif · what docs/loops/*-state.md are.

Three rot findings — not fixed here

I deliberately kept this PR docs-only. Each is a one-line fix if you want it separately:

Finding Where
VERSION = '0.4.4' vs package.json 0.4.5 — --version lies src/index.ts:14
NPM_TOKEN setup instructions for a workflow that uses Trusted Publishing CONTRIBUTING.md
"Commander" naming docs/ARCHITECTURE.md

The VERSION one is a real user-facing bug; I left it alone in case you'd rather roll it into the next release bump.

Verification

bun install --frozen-lockfilebunx tsc --noEmit clean → bun test 130 pass, 0 fail. bun.lock untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DRnTxyiiF3XHiyFqZJ3soz

The repo had no agent instruction file. Written to Anthropic's criterion
— only what an agent cannot infer from the code, and would get wrong
without being told — rather than as a tour of the codebase. 68 lines.

Every claim was verified against the source. The three that matter most
are ones this repo actively contradicts elsewhere:

- There IS a build step. `bin` points at `dist/index.js`, not `src/`,
  which is the opposite of the usual Bun setup; a stale `dist/` behind a
  linked binary silently swallows changes.
- The version lives in `package.json` AND the `VERSION` constant in
  `src/index.ts`, with nothing enforcing the pair — they are drifted
  right now: 0.4.5 shipped with the constant left at 0.4.4, so
  `--version` under-reports.
- Publishing is npm Trusted Publishing (`id-token: write`,
  `--provenance`). CONTRIBUTING.md still tells maintainers to configure
  an `NPM_TOKEN` secret that the workflow does not use.

Also records: citty (not Commander, which docs/ARCHITECTURE.md still
names), the downward-only layer rule, mandatory dual output with
docs/schemas/ as a published contract, CliError/ErrorCode as the only
surface errors, the frozen-lockfile trap that blocked CI twice, and the
co-located test / named-export / `.js`-suffix conventions.

No CHANGELOG entry: this is agent tooling, not a user-visible change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DRnTxyiiF3XHiyFqZJ3soz
@greptile-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown

Greptile Summary

Adds an agent-facing repository instruction file covering development, release, architecture, CLI, error-handling, testing, and environment conventions.

Confidence Score: 4/5

The PR appears safe to merge after correcting two remaining inaccuracies in the agent guidance.

The revised layer and error-boundary sections still describe stricter behavior than the current injected logging and db reset error paths implement, which could steer future automated changes toward unnecessary or inconsistent refactors.

Files Needing Attention: CLAUDE.md

Important Files Changed

Filename Overview
CLAUDE.md Introduces repository-specific operational and architectural guidance for coding agents.

Reviews (3): Last reviewed commit: "docs: correct four claims and add the tr..." | Re-trigger Greptile

Comment thread CLAUDE.md Outdated
Comment thread CLAUDE.md Outdated
drakulavich and others added 2 commits July 26, 2026 17:34
Both claims came from docs/ARCHITECTURE.md rather than the code, and
ARCHITECTURE is stale in exactly those spots — it describes a Commander
`parseAsync().catch()` that no longer exists.

- src/index.ts calls `runMain` and catches nothing. The boundary is
  `handleError()` in src/commands/common.ts, wired per command in its own
  catch, so the failure mode is a command that forgets to call it.
- `src/db/` imports `../api/client.js` and `../api/types.js` by design, so
  framing api and db as peers made a legitimate dependency look like a
  violation. The chain is lib → api → db → commands → index; the rule
  worth stating is no upward imports.

Greptile P2s on #26.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DRnTxyiiF3XHiyFqZJ3soz
An independent `omc ask grok` pass checked every claim against the
source. All verified before changing anything.

Corrected:
- "each command wires handleError in its own catch" is false: only the
  data-path commands do. healthcheck deliberately swallows into
  { ok: false, error } as a probe, and login/describe/manifest have no
  catch at all. Stated the real shape, including that index.ts has no
  global handler.
- docs/ARCHITECTURE.md is stale on layering too, not just citty — its
  diagram makes api and db peers when db imports api, and it omits the
  root-level format modules. Withdrew the "accurate on layering"
  endorsement.
- npm audit is not a gate: the step swallows failures into a
  ::warning::. Also noted release.yml runs no tsc.
- Dropped the invented "no default exports outside src/index.ts"
  exception; there are no default exports in src/ at all.

Added, all non-obvious enough that an agent would get them wrong:
- Registering a command touches four files. Missing the SUBCOMMANDS set
  in src/lib/argv-normalize.ts silently breaks `--format json <cmd>`,
  because citty does not hoist root flags and that normalizer is what
  moves them. The describe/manifest agent surface is hard-coded too.
- Schema migrations are append-only (`version > current`), so editing a
  shipped migration is a no-op on existing databases.
- Oura API fields should be treated as nullable — #23 retyped three of
  them after upstream drift.
- Both output modes apply to new data commands; the JSON-only and
  interactive commands are deliberate and must not be "fixed".
- The OURA_TOKEN / OURA_TOKEN_PATH / OURA_DB_PATH / OURA_TZ / NO_COLOR
  environment surface, which the file omitted entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DRnTxyiiF3XHiyFqZJ3soz
@drakulavich
drakulavich merged commit 852441c into main Jul 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant