Modernize the public resource kit and make MCP writes atomic - #89
Conversation
![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: fe71f2dbed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review Please review the replacement head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2311881113
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two failures shared a root: the store treated work that had already landed as work that could still be refused. The lock release sat in a finally, so a rejection there replaced a successful return after #replace had renamed the file. MCP reported failure for an add_case_study or add_changelog_entry that was already visible, and an ordinary retry appended the entry twice. This is the same bug class as the directory fsync fixed one level down; the sweep missed the caller. The exit paths are now explicit, and a release failure after publication is a stderr diagnostic rather than a lie to the caller. Validating the current document with the full validator created a deadlock with the reachability rules added in this branch: add_decision_node refused to run precisely when the tree had the dangling edge it would have resolved. Each validator now splits into a shape check for the document being edited and a full check for the result, so a broken catalog can still be repaired while an invalid one still cannot be published. lockApi joins fsApi as an injectable dependency because neither failure was reachable in a test otherwise. The test assertions match the server's failure marker through an escaped constant so no literal emoji lands in source.
This branch adds Claude Fable 5 across the model catalog, comparison table and decision tree, so the calculator was recommending a model it could not price. A Fable session fell through to the Sonnet default and understated API-equivalent cost by more than three times. The snippet, the button grid and the in-page pricing table are one contract held together by a guard test, so all three move at once rather than the snippet alone. The guard now asserts the Fable tier reaches the UI too, which is what the test claims to check.
The new v2.1 template sections landed in Title Case and three sentences used an unspaced em dash, both against the house style the rest of these files follow. The template exists in two places and the freshness suite asserts they are byte-identical, so the downloads copy moves with the root one.
The best-practice tips render into innerHTML, and this branch routes a new data source into that sink. Every neighbouring renderer sanitizes first; this one did not. Tips are now allowlisted through sanitizeRichHTML rather than escaped, because several deliberately carry strong, code and anchor markup that users are meant to see rendered. CODE joins the allowlist so routing them there does not demote it to text, and anchors pick up the safe-URL check. The vibe-coding guide advertised an og:image that does not exist and pointed at it with a relative path, so its card was broken twice over. It now carries the same absolute image, dimensions, canonical url and twitter fields as its sibling pages.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6aa350ec53
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Splitting the validators into accept and publish let a broken catalog be repaired through mutate, but update_model_info never got that far: its preflight ran the full validator through getValidModels before the mutation started. A model entry whose link is javascript: could not be given a valid URL through the tool that exists to set it, leaving hand-editing as the only recovery. The existence check now runs inside the mutator against the document read under the lock, which also closes the gap between that separate unlocked read and the mutation it was guarding. An unknown name is still refused with the same message. This is the third place in this branch where an invariant sat at the wrong boundary, so the sweep now covers callers rather than only the reported site: getValidModels has one remaining use, building tool descriptions, where a failure already degrades to a generic description instead of blocking.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2656c99f2e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two ways update_model_info mishandled an entry it was meant to fix. JSON.parse produces an own "__proto__" key, and Object.assign copying that key runs the Object.prototype setter, repointing the entry's prototype instead of adding a field. The validator then read description, features and link through that prototype and passed, while JSON.stringify writes only own properties, so the catalog would have published an empty entry and reported success. That defeats the publish validation this branch exists to add. Merges now refuse prototype keys by name, and the model validator requires the three fields to be own properties, so neither layer alone has to be right. Separately, an entry that is null, a primitive or an array reached the merge as its target. Object.assign throws on null and discards the result for a primitive, so the one shape most in need of repair was the one shape that could not be repaired. The entry is replaced before merging. Swept the same class in add_decision_node, where a "__proto__" node id would have repointed the tree's prototype. The truthy existence check happened to block it and "constructor" too, but reported a collision that had not happened, and "prototype" was not blocked at all. Both are now explicit. The repair path has a documented boundary, asserted by test: publish validation covers the whole catalog, so a repair cannot land while a different entry is still corrupt. Never publishing an invalid document wins over repairability.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Summary
This burns down the remaining production Tailwind migration and LLM Advisor data-integrity work while refreshing the public agent/model documentation that had drifted out of date.
fsync, and atomic renameumasksettings and sync parent-directory entries after renameImpact
The public resource kit no longer depends on runtime Tailwind compilation in production, concurrent MCP writers cannot publish partial JSON, acknowledged writes preserve their permissions and crash durability, and current guidance is easier to audit and keep fresh.
Verification
git diff --checkpassed23118811133be3f35deda2eb5c9512ba34c09d3bExplicitly deferred follow-ups
The capped ninth local review identified two additional improvements that the owner directed into separate issues rather than another local review round:
Closes #73
Closes #78
Closes #84
Closes #85
Closes #82