Refactor effective plan API and local vault sync - #9
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9be04c9ed
ℹ️ 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".
| const lastDate = (state.planned_days ?? []) | ||
| .filter((/** @type {any} */ day) => day.date >= from) | ||
| .at(-1)?.date ?? from; |
There was a problem hiding this comment.
A Plan Update Package accepts any valid future effective_from, but this chooses that unrestricted last date as the end of the automatic Plan range. A mistaken or malicious update in a distant year therefore makes agentDatedPlan() call dateRange() across decades or millennia, producing an enormous response that can exceed Worker CPU/memory limits; because the far Planned Days remain materialized, subsequent Plan reads stay broken. Bound future plan dates or return a bounded/sparse projection.
Useful? React with 👍 / 👎.
| const priorWeekPaths = Array.isArray(priorManifest?.week_paths) ? priorManifest.week_paths.filter(isManagedWeekPath) : []; | ||
| const staleWeekPaths = priorWeekPaths.filter((/** @type {string} */ path) => !currentWeekPaths.has(path)); | ||
| const removedPaths = []; | ||
| for (const relativePath of [...staleWeekPaths, ...LEGACY_PLAN_PATHS]) if (await unlinkIfPresent(root, relativePath)) removedPaths.push(relativePath); |
There was a problem hiding this comment.
On every run, these legacy paths are deleted even when neither a prior manifest nor a legacy receipt establishes that plan2local created them. Thus a first run against an existing vault containing a human-authored plan/current.md or data/plan/current.json permanently removes that user data, despite the cleanup contract promising to preserve user-created notes. Only remove these paths when legacy ownership can be verified.
Useful? React with 👍 / 👎.
| for (const [relativePath, content] of weekFiles) await writeAtomicFile(join(root, relativePath), content, "utf8"); | ||
| await writeAtomicFile(join(root, PLAN_SOURCE_PATH), sourceJson, "utf8"); | ||
| await writeAtomicFile(join(root, PLAN_INDEX_PATH), indexMarkdown, "utf8"); |
There was a problem hiding this comment.
When refreshing an existing successful projection, the managed week/source/index files are overwritten before the pending manifest is written. If any later write fails or the process is interrupted, some files may contain the new generation while the previous manifest still says write_status: complete and readback.status: verified, so later consumers can trust a mixed projection. Publish pending state before the first replacement or stage the full generation and switch it atomically.
Useful? React with 👍 / 👎.
1 participant
Problem
The Agent Plan read returned immutable revision packages, so repeated writes for the same dates produced redundant payloads and duplicate Obsidian week files. Plan readback also duplicated a Schedule request even though the effective dated Plan can prove the applied result.
Result
/workout plan2local, stable natural-week vault files, effective source/manifest receipts, and vault README/Agent templatesContract impact
GET /api/agent/v1/planchanges from revision-shaped schema version 1 to effective dated schema version 2./scheduleremains the explicit operational date-window resource with Session linkage. The local projection contract is added astraining-plan-local-v1.Verification
npm run checknpm run release-checkSkill is valid!)Privacy, migration, and deployment
No credentials, private host paths, or personal training records are committed. The local vault remains a derived read-only projection. GitHub merge to
maintriggers the existing automatic Cloudflare deployment and operator acceptance workflow; no D1 migration is required.