fix(core): make Markdown note permalinks mandatory - #1477
Conversation
Signed-off-by: phernandez <paul@basicmachines.co>
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. |
|
/review |
![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: aac7d28794
ℹ️ 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".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1bc9044ce
ℹ️ 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".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review Added an integration regression for the collision finding; it passes with no production changes. See the resolved thread for the existing database ownership check that prevents this collision. Local review found no actionable defects, all six permalink integration tests pass, and typecheck/lint/format pass. The previously discussed whitespace serialization changes remain out of scope per maintainer direction. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e6824c9ae
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e6824c9ae
ℹ️ 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".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review Latest follow-up removes the retired config validator entirely. The PR description records the maintainer-approved last-write-wins behavior and unchanged FileService serialization. Local review found no actionable regressions; all 147 targeted tests and fast-check passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a086e22955
ℹ️ 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".
1 participant
Why
Markdown notes need a canonical semantic address throughout indexing and accepted writes. Removing the opt-out resolves #1395 and eliminates the null-permalink identity regime.
What Changed
disable_permalinks; legacy config values andBASIC_MEMORY_DISABLE_PERMALINKSare ignored, not rejected. Normalization removes the retired JSON key.Implementation Details
Config uses its existing unknown-key handling without a special legacy validator. Preparation and indexing enforce Markdown identity; non-Markdown resources remain unchanged. Integration coverage exercises real config loading, indexing, collisions, and moves.
Testing
uv run pytest tests/test_config.py test-int/test_mandatory_permalinks_integration.py --no-cov -q: 147 passed.just fast-check: lint, formatting, and typecheck passed.just doctor, andjust package-checkpassed.codex review --uncommittedis required before pushing each follow-up.Risks / Follow-ups
Permalinks are mandatory even for installations with an old opt-out. Per maintainer direction, concurrent permalink writes retain last-write-wins behavior and eventual consistency; this change adds no checksum guards, locks, reparsing, or retries for that race. A later normalization write can win over an intervening custom permalink edit. Existing FileService whitespace serialization is unchanged and outside this PR.
Closes #1395