feat(core): T2 foundation — types, UUID, path, frontmatter, atomic FS - #3
Merged
Conversation
….1〜T2.7)
types (T2.1 base):
- NoteSource = "note" (extension point)
- NoteFrontmatter: createdAt/updatedAt (ISO 8601 with offset), source,
optional title, projects, tags
- NoteMetadata = NoteFrontmatter & { id: string }
uuid (T2.1):
- generateId() uses the uuidv7 package
- isValidUuid() strict regex (version=7, RFC 4122 variant)
- 1000-entry uniqueness + 500-entry monotonic order tests
path (T2.2, T2.6):
- DENNOH_DIR = ".dennoh" constant (the single source of truth)
- buildNoteDir / buildNotePath produce <vault>/YYYY/MM/DD/<UUID>.md
in local time (matches the user's mental model of "today")
- isNotePath is case-insensitive on the .md extension
- parseIdFromPath returns null for non-.md, basename otherwise;
UUID-shape validation is the caller's responsibility (isValidUuid)
- Tests cover year/month/leap-day boundaries and round-trip
frontmatter (T2.3, T2.4):
- isoWithLocalOffset(date) → "YYYY-MM-DDTHH:MM:SS±HH:MM"
(Date.toISOString is UTC-only and cannot satisfy the spec example
+09:00)
- serializeFrontmatter never emits an id field; empty projects/tags
serialize as []; title is omitted when undefined
- parseFrontmatter splits on ---, runs yaml.parse, throws on:
invalid YAML, missing createdAt/updatedAt, non-"note" source,
non-array projects/tags, non-string elements. Defaults source="note"
and empty arrays only when the keys are absent.
- Body extraction skips the closing delimiter's newline plus one
blank-line convention so round-trips are exact
- updateFrontmatter protects createdAt by re-applying it after the
spread; bumpUpdatedAt option wins over an explicit updates.updatedAt
file (T2.5, T2.6):
- writeFileAtomic writes to .tmp.<UUID> in the same directory then
fs.rename onto the target; mkdir -p first; best-effort unlink of
the temp file on error before rethrowing
- readNote integrates parseIdFromPath + fs.readFile + parseFrontmatter,
throwing with a clear prefix on each failure mode
- writeNote builds the path, serializes, atomically writes, returns
the resolved path
init refactor (T2.7 already implemented in phase-1, now consistent):
- src/cli/commands/init.ts uses the DENNOH_DIR constant for the
directory creation, gitignore entry, and success message instead
of hard-coded ".dennoh" literals
- Two new init.test.ts cases assert ${DENNOH_DIR}/ ends up in
.gitignore on a fresh vault and that the entry tracks the constant
barrel:
- src/core/index.ts re-exports types, uuid, path (incl. DENNOH_DIR),
frontmatter, and file APIs with type-only exports separated
deps:
- uuidv7@1.2.1, yaml@2.9.0
.tmp/tasks.md: mark T2.1〜T2.7 done. 127 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPR ChangesNote persistence implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/file.ts`:
- Around line 32-56: The id is used directly to build/read filesystem paths
which allows path traversal; validate that id conforms to a UUID v7 before using
it in both readNote and writeNote: in readNote (function parseIdFromPath is
currently used) verify the returned id matches a UUID v7 pattern (or use a UUID
library) and throw a clear Error if it does not, and in writeNote validate the
incoming id parameter the same way before calling buildNotePath (reject inputs
containing "../" or path separators as part of the validation); update both
functions to fail fast on invalid ids rather than attempting to read/write
paths.
In `@src/core/frontmatter.ts`:
- Around line 82-87: The checks in parseFrontmatter that only assert typeof
parsed.createdAt/updatedAt === "string" are insufficient; update
parseFrontmatter to validate that parsed.createdAt and parsed.updatedAt are
valid ISO 8601 timestamps with an offset (e.g., end with 'Z' or a ±hh:mm offset)
by attempting to parse them and rejecting on failure: for each of
parsed.createdAt and parsed.updatedAt, verify the value is a string, then parse
it (e.g., Date.parse or a strict ISO8601-with-offset regex), ensure the
resulting Date is valid (not NaN) and that the original string includes an
offset or 'Z', and throw a clear Error mentioning which field is invalid (e.g.,
"Frontmatter 'createdAt' must be an ISO 8601 string with offset") so downstream
code receives guaranteed ISO8601-with-offset timestamps.
In `@src/core/path.ts`:
- Around line 18-20: buildNotePath currently concatenates the raw id into a file
name which permits path traversal (absolute paths, ".." segments or path
separators); update buildNotePath to validate/sanitize the id before joining:
reject or throw if id contains path.sep, "/" or "\" or startsWith path.sep or
contains ".." or is an absolute path, or alternatively use a sanitized basename
(e.g., path.basename(id)) and then validate the result; ensure the function
(buildNotePath) fails fast with a clear error message so callers like writeNote
(src/core/file.ts) cannot write outside the intended vault directory.
In `@tests/core/file.test.ts`:
- Around line 106-109: The test's exception assertion is too weak: update the
test case in tests/core/file.test.ts that calls readNote(target) to assert the
specific error message (or a distinctive substring/regex) thrown by
parseFrontmatter instead of using a generic rejects.toThrow(), so change the
assertion to expect(readNote(target)).rejects.toThrow(/expected message
fragment/) (or an equivalent toThrowError with a substring) to verify the
intended failure reason from readNote/parseFrontmatter.
- Around line 58-65: The test uses an unnecessary if-check inside the for loop;
replace the manual for loop and the forbidden if branch by iterating with
targets.entries() so you get both index and path without a conditional: for
(const [i, file] of targets.entries()) { expect(fs.readFileSync(file,
"utf-8")).toBe(`body-${i}`); } — remove the if (file === undefined) throw ...
line and keep the preceding setup (targets and writeFileAtomic) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 86c8819d-1695-411d-8102-4087548c53fe
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (14)
.tmp/tasks.mdpackage.jsonsrc/cli/commands/init.tssrc/core/file.tssrc/core/frontmatter.tssrc/core/index.tssrc/core/path.tssrc/core/types.tssrc/core/uuid.tstests/cli/init.test.tstests/core/file.test.tstests/core/frontmatter.test.tstests/core/path.test.tstests/core/uuid.test.ts
Reject ids that are empty, contain /, \, NUL, or are exactly . / .. so callers like writeNote cannot write outside the date subdirectory even when the id is sourced from external input (MCP save_memory, CLI restore <id>, vault scans). generateId() output (UUID v7) is unaffected; existing tests still pass. Adds 6 negative cases to tests/core/path.test.ts covering '..', forward slash, backslash, absolute-looking, null byte, and empty id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
frontmatter: parseFrontmatter now requires createdAt / updatedAt to be ISO 8601 with an offset (Z or ±HH:MM) and to parse to a valid Date. Catches "yesterday", date-only "2026-06-12", offset-less "2026-06-12T10:30:00", and impossible components like "2026-13-99T...". +7 cases. file: readNote and writeNote now reject ids that are not valid UUID v7 via assertValidNoteId(isValidUuid). assertSafeNoteId inside buildNotePath stays as the lower-level path-traversal guard. +3 cases (non-UUID basename rejection on read, non-UUID id rejection on write, v4 id rejection on write); existing readNote error tests now use generateId()-derived names. file.test: drop the noUncheckedIndexedAccess defensive `if (file === undefined) throw` in the parallel-writes test by iterating with targets.entries(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/path.test.ts`:
- Around line 62-64: Add a second test case alongside the existing one that
asserts buildNotePath rejects the single-dot id: mirror the existing it("rejects
an id that contains '..'") test but call buildNotePath("/vault", ".", new
Date(...)) and expect it toThrow(), so both "." and ".." are covered for the
buildNotePath function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1aef15d7-bef3-45ef-a94f-b8a2a1eeccb7
📒 Files selected for processing (2)
src/core/path.tstests/core/path.test.ts
Comment on lines
+62
to
+64
| it("rejects an id that contains '..'", () => { | ||
| expect(() => buildNotePath("/vault", "..", new Date(2026, 5, 13))).toThrow(); | ||
| }); |
Contributor
There was a problem hiding this comment.
"." を拒否する分岐の回帰テストを追加してください。
buildNotePath 実装は "." も拒否していますが、現状の追加テストは ".." のみを固定化しています。"." ケースも追加して、この分岐の退行を防いでください。
diff案
it("rejects an id that contains '..'", () => {
expect(() => buildNotePath("/vault", "..", new Date(2026, 5, 13))).toThrow();
});
+
+ it("rejects an id that is '.'", () => {
+ expect(() => buildNotePath("/vault", ".", new Date(2026, 5, 13))).toThrow();
+ });As per coding guidelines, tests/**/*.ts では「エッジケース・エラー経路のカバレッジが十分か。」を重点的に確認する必要があります。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("rejects an id that contains '..'", () => { | |
| expect(() => buildNotePath("/vault", "..", new Date(2026, 5, 13))).toThrow(); | |
| }); | |
| it("rejects an id that contains '..'", () => { | |
| expect(() => buildNotePath("/vault", "..", new Date(2026, 5, 13))).toThrow(); | |
| }); | |
| it("rejects an id that is '.'", () => { | |
| expect(() => buildNotePath("/vault", ".", new Date(2026, 5, 13))).toThrow(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/core/path.test.ts` around lines 62 - 64, Add a second test case
alongside the existing one that asserts buildNotePath rejects the single-dot id:
mirror the existing it("rejects an id that contains '..'") test but call
buildNotePath("/vault", ".", new Date(...)) and expect it toThrow(), so both "."
and ".." are covered for the buildNotePath function.
Source: Coding guidelines
Match the specific parseFrontmatter error ("Frontmatter delimiter
'---' not found at the start of the file") instead of bare
rejects.toThrow(), so the test fails loud if readNote ever starts
throwing for a different reason.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/frontmatter.ts`:
- Around line 15-16: The current timestamp validation in assertIsoWithOffset
relies solely on Date.parse after matching ISO_8601_WITH_OFFSET, which can
accept invalid calendar dates (e.g., 2021-02-30). Change assertIsoWithOffset to,
after matching ISO_8601_WITH_OFFSET, extract the year, month, day, hour, minute,
second (and optional fraction) and timezone offset from the regex groups and
perform explicit numeric range and cross-field checks: month 1–12, day
1–maxDaysForMonth(year, month) (handle leap years for February), hour 0–23,
minute 0–59, second 0–59 (or allow 60 only if you intend to support leap
second), and validate fractional seconds if present; only after these checks
succeed use Date.parse/Date.UTC or epoch conversion. Keep the existing error
throw (e.g., the Error with frontmatter '${field}' message) on failure. Update
the code paths in assertIsoWithOffset and any helper functions (add a
maxDaysForMonth/yearIsLeap helper) accordingly.
In `@tests/core/frontmatter.test.ts`:
- Around line 153-163: The test currently asserts only frontmatter.createdAt but
omits verifying frontmatter.updatedAt; update the "accepts ISO 8601 with
fractional seconds and offset" test to also assert the updatedAt value by adding
an assertion like
expect(frontmatter.updatedAt).toBe("2026-06-12T10:35:00.456-05:00") after
obtaining frontmatter from parseFrontmatter so both timestamps are validated.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b2d17387-da32-404c-bd8c-5a3f6707b4aa
📒 Files selected for processing (4)
src/core/file.tssrc/core/frontmatter.tstests/core/file.test.tstests/core/frontmatter.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/frontmatter.ts`:
- Around line 13-16: In maxDaysForMonth, stop silently falling back to 31:
validate the month argument (ensure 1 <= month <= 12), compute daysInMonth using
yearIsLeap(year), and if daysInMonth[month - 1] is undefined throw an explicit
Error (include year and month in the message) instead of returning a default;
this makes the function fail-fast on invalid month inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 404ea580-7e34-414c-bbae-f0d20f17c641
📒 Files selected for processing (2)
src/core/frontmatter.tstests/core/frontmatter.test.ts
Replace the silent '?? 31' fallback with explicit range and lookup checks: throw when month is outside 1-12 or when the daysInMonth lookup is undefined (the message includes both year and month). The existing caller (assertIsoWithOffset) already validates month, so this is defense in depth, but it removes a "looks like it works" fallback that could mask future misuse. Also picks up Biome's safe parseInt -> Number.parseInt rewrites that came in with 3f10fc1, so lint is green again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 13, 2026
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
1 participant
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.
Summary
Implements T2 (データモデル / Markdown 永続化) foundation from
.tmp/tasks.md(T2.1〜T2.7). All seven subtasks are checked off.This phase delivers the pure library layer that downstream T5 (memo CRUD core) and T10/T11 (MCP / CLI command wiring) will sit on top of. No CLI commands are added in this PR; the existing
dennoh init/dennoh configsurface is unchanged.Review plan: https://app.coderabbit.ai/plan/d096d8b3-4fde-468b-90bf-3e9f7e8980a0/7b23c2b1-300d-4f45-ab86-49a2798e50b1
What landed
Types (
src/core/types.ts)NoteSource = "note"— extension point for v0.x url/youtube/etc.NoteFrontmatter—createdAt/updatedAt(ISO 8601 with offset),source, optionaltitle,projects,tagsNoteMetadata = NoteFrontmatter & { id: string }UUID (
src/core/uuid.ts)generateId()viauuidv7@1.2.1(monotonic within the same millisecond)isValidUuid()strict regex (version=7, RFC 4122 variant)Path (
src/core/path.ts)DENNOH_DIR = ".dennoh"constant (single source of truth for the metadata dir name)buildNoteDir/buildNotePathproduce<vault>/YYYY/MM/DD/<UUID>.mdusing local time (matches "today" semantics)isNotePathcase-insensitive on.mdparseIdFromPathreturnsnullfor non-.md, basename otherwise (UUID validity is the caller's concern)Frontmatter (
src/core/frontmatter.ts)isoWithLocalOffset(date)→YYYY-MM-DDTHH:MM:SS±HH:MM(Date.toISOStringis UTC-only and cannot produce the spec example+09:00)serializeFrontmatter— never emits anidfield; emptyprojects/tagsserialize as[];titleomitted when undefinedparseFrontmatter— splits on---, parses YAML, throws on invalid YAML / missingcreatedAt-updatedAt/ non-notesource / non-array projects-tags / non-string elements; defaultssource="note"and empty arrays only when the keys are absentupdateFrontmatter— re-appliesexisting.createdAtafter the spread to protect it;bumpUpdatedAtopt-in overrides any explicitupdates.updatedAtFile I/O (
src/core/file.ts)writeFileAtomic(path, content)—.tmp.<UUID>in the same directory, thenfs.rename;mkdir -pfirst; best-effort unlink of temp on error before rethrowingreadNote(path)— integratesparseIdFromPath+fs.readFile+parseFrontmatterwith clear error prefixes per failure modewriteNote(vaultPath, id, date, frontmatter, body)— builds the path, serializes, atomically writes, returns the resolved pathdennoh initrefactor (T2.7)src/cli/commands/init.tsnow referencesDENNOH_DIRinstead of literal".dennoh"in four places (directory creation, gitignore entry, gitignore dedupe, success message)tests/cli/init.test.tsassert${DENNOH_DIR}/ends up in.gitignoreon a fresh vault and that the entry tracks the constantBarrel (
src/core/index.ts)Re-exports types (type-only), uuid, path (incl.
DENNOH_DIR), frontmatter, and file APIs.Why the core layer doesn't read config
writeNote(vaultPath, ...)deliberately takesvaultPathas a parameter; it does not callreadConfig(). Layering:src/core/*src/config/*vaultPath/langThe user-observable wiring (
dennoh add "hello"writes into the configured vault) lands in T5.1 (memo CRUD core helper) → T11.4 (dennoh add) / T10.4 (save_memoryMCP tool).Test coverage
87 → 127 tests (+40 cases):
tests/core/uuid.test.ts(11): valid v7, uniqueness over 1000, monotonic over 500, rejection of v4 / bad variant / non-hex / wrong segment lengths / missing hyphens / emptytests/core/path.test.ts(19): year / month / leap-day boundaries, zero-padding,isNotePathcase-insensitive,parseIdFromPathnull cases,buildNotePath↔parseIdFromPathround-triptests/core/frontmatter.test.ts(28): serialize 6 / parse 14 / update 7 /isoWithLocalOffset1 — including CRLF normalization, blank-line conventions, missing-required-field errors, source validationtests/core/file.test.ts(12): atomic write paths, recursive mkdir, no.tmp.*residue, overwrite, 20-way parallel writes, round-trip, missing file / non-.mderrorstests/cli/init.test.ts(+2): fresh-vault.gitignorecreation,DENNOH_DIRconstant trackingTest plan
bun installbun run lint— cleanbun run typecheck— cleanbun run test— 127 passbun run build— producesdist/cli.jsecho /tmp/dennoh-phase2 | bun dist/cli.js init—.dennoh/,.git/, and.gitignore(containing.dennoh/) are created at the chosen vault.tmp.*residue:find /tmp/dennoh-phase2 -name ".tmp.*"returns nothing after initrm -rf /tmp/dennoh-phase2 ~/Library/Application\ Support/dennoh/config.json🤖 Generated with Claude Code
Summary by CodeRabbit