Skip to content

fix(SUP-3268): harden ontology packaging and timezone precedence - #91

Merged
levineam merged 1 commit into
mainfrom
SUP-3268/ontology-p3-fixes
Jun 27, 2026
Merged

fix(SUP-3268): harden ontology packaging and timezone precedence#91
levineam merged 1 commit into
mainfrom
SUP-3268/ontology-p3-fixes

Conversation

@levineam

@levineam levineam commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • demote generic TZ behind explicit jarvOS timezone config while preserving JARVOS_TIMEZONE and journal-maintenance-specific overrides
  • move public ontology sample files to schema/examples/public-ontology/ and exclude live ontology/ write targets from package allowlists
  • add regression coverage for timezone precedence and ontology packaging allowlists

Autoreview finding dispositions

  • Ambient TZ overriding configured timezone: fixed. Configured user/global timezones now win over generic shell/CI TZ; JARVOS_TIMEZONE remains the explicit env override.
  • modules/jarvos-ontology/ontology/ published while also serving as extractor default output: fixed. Public fixtures moved under schema/examples/public-ontology/; root and module package allowlists exclude live ontology output.

Review of record

CE-review pass over this branch found no remaining actionable findings after the fixes above.

Verification

  • git diff --check
  • node --test modules/jarvos-secondbrain/tests/journal-maintenance-schedule.test.js modules/jarvos-secondbrain/tests/config-resolution.test.js modules/jarvos-secondbrain/tests/jarvos-paths-shim.test.js — 29 pass
  • node --test modules/jarvos-ontology/test/*.test.js — 69 pass (existing Node MODULE_TYPELESS_PACKAGE_JSON warnings)
  • node tests/modules-smoke-test.js — 72 checks / 0 failed
  • npm test — pass
  • root npm pack --dry-run --json: live ontology files [], public example count 6
  • modules/jarvos-ontology npm pack --dry-run --json: live ontology files [], public example count 6
  • npm run release:drift — OK
  • npm run release:check:candidate — expected non-ready only because v0.6.2 tag already exists; all other checks pass

Summary by CodeRabbit

  • Bug Fixes
    • Improved timezone handling so journals and notes use explicit configuration instead of falling back to the system timezone, reducing date drift.
    • Updated package publishing rules to keep internal ontology data out of published artifacts.
    • Adjusted tests and examples to use the public sample ontology layout, aligning documentation and validation with what users should access.

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Two bug fixes: timezone resolution in resolveConfig, getTimeZone, and getJournalMaintenanceTimeZone is reordered so explicit jarvOS config values take priority over the ambient TZ environment variable. Separately, the live ontology/ directory is removed from publish allowlists and public sample fixtures are relocated to schema/examples/public-ontology/, with tests and docs updated accordingly.

Changes

Timezone Resolution Precedence

Layer / File(s) Summary
Fallback chain reordering
modules/jarvos-secondbrain/bridge/config/src/resolve-config.js, modules/jarvos-secondbrain/bridge/config/jarvos-paths.js
resolveUserTimezone, getTimeZone, and getJournalMaintenanceTimeZone reorder their candidate arrays so env.TZ is evaluated after config-provided values and JARVOS_TIMEZONE is checked earlier.
Precedence tests
modules/jarvos-secondbrain/tests/config-resolution.test.js, modules/jarvos-secondbrain/tests/journal-maintenance-schedule.test.js
New tests verify user.timezone from config beats TZ, and JARVOS_TIMEZONE beats configured user timezone in journal maintenance scheduling.

Ontology Publish Allowlist and Fixture Relocation

Layer / File(s) Summary
Publish allowlist changes
package.json, modules/jarvos-ontology/package.json
Root package.json removes modules/jarvos-ontology/ontology/ from files; module package.json gains an explicit files whitelist that excludes the live ontology directory.
Tests pointing to public fixture
modules/jarvos-ontology/test/reader.test.js, modules/jarvos-ontology/test/validator.test.js, tests/modules-smoke-test.js
ONTOLOGY_DIR redirected to schema/examples/public-ontology; smoke test added to assert both package.json files exclude the live ontology path.
README and CHANGELOG
modules/jarvos-ontology/README.md, CHANGELOG.md
Directory layout and ontology layers guidance updated to reference schema/examples/public-ontology/; both fixes documented in the unreleased changelog.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • levineam/jarvOS#11: Introduced tests/modules-smoke-test.js and jarvos-ontology module stubs that this PR extends with a publishability check.
  • levineam/jarvOS#16: Modified the same getTimeZone() and journal maintenance timezone logic in jarvos-paths.js that this PR reorders.

Poem

🐇 Hop hop, the timezone's set just right,
No ambient TZ to cause date-drift fright.
The ontology stays off the publish list,
In schema/examples/ nothing is missed.
Fixtures are public, the live dir is clear—
A tidy release, says the rabbit with cheer! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two main changes: ontology packaging hardening and timezone precedence fixes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch SUP-3268/ontology-p3-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@levineam
levineam merged commit 0addbe0 into main Jun 27, 2026
6 of 7 checks passed
@levineam
levineam deleted the SUP-3268/ontology-p3-fixes branch June 27, 2026 21:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 689f2e298d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

`ontology/index.md` is the layer map — read it first when navigating manually.
`schema/templates/index.md` is the blank layer map for new ontologies. The public
example fixture lives under `schema/examples/public-ontology/`; keep your filled-in
private ontology in a local `ontology/` directory outside published package files.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore the new private ontology write target

When users follow this guidance or run existing defaults such as scripts/bootstrap.js/scripts/extract.js, the private files are created under modules/jarvos-ontology/ontology/, but I checked the repo and module ignore files and there is no .gitignore entry for that directory. Removing it from npm files prevents publication, but it still leaves private ontology data as untracked files that can be accidentally added to git; add an ignore rule for the live ontology/ target.

Useful? React with 👍 / 👎.

Comment on lines 126 to +130
|| rest.user?.timezone
|| rest.user?.timeZone
|| rest.timezone
|| rest.timeZone,
|| rest.timeZone
|| env.TZ,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip invalid config timezones before TZ fallback

With a configured but invalid timezone, for example user.timezone: "Not/A_Timezone", and a valid ambient TZ, this truthy || chain passes the invalid config value into validTimezone(), which returns America/New_York instead of trying the later TZ fallback. That regresses the fallback path for date-sensitive note/journal writers using resolveConfig().user.timezone; iterate candidates and validate each one before falling back.

Useful? React with 👍 / 👎.

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