docs: add CLAUDE.md with the rules an agent would otherwise get wrong - #26
Merged
Conversation
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 SummaryAdds an agent-facing repository instruction file covering development, release, architecture, CLI, error-handling, testing, and environment conventions. Confidence Score: 4/5The 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
|
| 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
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
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 free
to 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.
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:
package.json#bin→dist/index.js, notsrc/. That is the opposite of the usual Bun setup, and a staledist/behind a linked binary silently swallows changes.package.json#versionand theVERSIONconstant insrc/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 at0.4.4, sooura-cli --versionunder-reports. See below.id-token: write+--provenance, release.yml:10,54).CONTRIBUTING.mdstill instructs maintainers to configure anNPM_TOKENsecret the workflow does not use.Also recorded
citty, not Commander (docs/ARCHITECTURE.mdstill names Commander; there is no such dependency) · the downward-only layer rulelib → api/db → commands → index· both output modes mandatory, withdocs/schemas/as a published external contract ·CliError+ErrorCodeas the only errors that may reach the surface, soemitError/exitCodeForare not bypassed · the--frozen-lockfiletrap that blocked CI twice (#15, #16) · co-located tests, named exports only,.jssuffix on local imports · Git LFS forassets/*.gif· whatdocs/loops/*-state.mdare.Three rot findings — not fixed here
I deliberately kept this PR docs-only. Each is a one-line fix if you want it separately:
VERSION = '0.4.4'vspackage.json0.4.5 —--versionliessrc/index.ts:14NPM_TOKENsetup instructions for a workflow that uses Trusted PublishingCONTRIBUTING.mddocs/ARCHITECTURE.mdThe 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-lockfile→bunx tsc --noEmitclean →bun test130 pass, 0 fail.bun.lockuntouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01DRnTxyiiF3XHiyFqZJ3soz