Repository navigation
Add: dynamic schema RFC for user-defined keys (#39) - #56
Conversation
Document the v1.0 schema.json design and link it from README. Rebind stale SpecSync evidence so Trust stays green with the README update. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
0xLeif
left a comment
There was a problem hiding this comment.
Review from the Kimi Code agent (requested by 0xLeif). Docs-only PR: RFC + README + CHG-0026 record + a mechanical SpecSync evidence rebind. Verified the RFC's claims against the code and checked the ceremony artifacts. Overall: solid, one substantive fix requested before merge.
Verified
- RFC claims match the code: state-root precedence
--state-dir > APS_HOME > ~/.aps,parseBoolexists,Schema.schemaVersionis static, and the proposed codes (unknown_key/schema_conflictat 64,schema_invalidat 65) fit the #31 taxonomy (invalid_value= 64,corrupt_state= 65). - README claims hold:
docs/windows-readiness.mdexists, linux/windows smoke CI run on hosted runners (linux-smoke green on this PR). Links resolve. No em/en dashes. - CHG-0026 artifacts are valid JSON; sequence bump 25 to 26 is clean; the stale-evidence rebind matches the #50 precedent.
- All five RFC questions from #39 are answered, with a go/no-go and a phased plan.
1. Demo-key inventory mismatch (please fix before merge)
Issue #39 says "the five demo keys", but DemoKey has seven cases: counter, message, flag, note, profile, secret, profileName. The RFC's seed schema.json example lists only 5 entries, omitting message (State/String) and flag (StoredState/Bool), while locked decision #3 says "the current demo keys ship as the initial schema.json contents". As written, the example silently drops two keys, and this example is what implementers will copy. The omission also means the seed never exercises StoredState or Bool, both of which are in the closed 1.0 enums. Add the two entries (flag also nails down the aps.user.<name> prefix from open question 1), or state the exclusion deliberately.
2. Materialize-on-read (settle in the RFC)
Missing schema.json is materialized "on first mutating command or on aps key / aps schema that needs it". aps schema is the read-only, cacheable contract endpoint from #32; a read that writes a file will fail on read-only state roots and surprise CI sandboxes. Suggest: reads fall back to the built-in default in memory; only mutations persist the file.
3. Nits
- Declaration-UX table says invalid hand-edits fail with "
invalid_value/ dedicatedschema_invalidif needed", while section 8 firmly definesschema_invalid/ 65. For an accepted design, pick one. - The RFC narrows
parseBooltotrue/false/1/0; the real one also acceptsyes/y/on/no/n/off. Implementation should reuse it as-is. - CHG-0026
change.md"Intent" restates the title, anddocs.mdAdded/Updated are empty. Cosmetic.
CI note: linux-smoke passed; build-test-smoke, trust, and windows-smoke were still pending at review time.
Verdict: approve-with-comments, but please address point 1 (and ideally settle point 2) before merge. This doc is the contract the 1.0 implementation changes will be written against.
Avoid Trust gate flakes from partial GitHub Releases downloads (curl exit 18) by seeding Trust's install root before the action runs. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Trust failed on a flaky Fledge download ( Pushed:
|
Summary
docs/design/dynamic-schema.md:<state-root>/schema.jsonas source of truth,aps key add/removeas sugar, demo keys as the default namespace, plus SpecSync strategy and phased go plan.CHG-0026(docs-only) plus a stale-evidence rebind sweep sofledge spec checkstays green after the README edit (including definition-digest repair for CHG-0022..0025).Closes #39
Test plan
fledge lanes run verifyfledge spec checkspecsync change check(no stale accepted evidence)Made with Cursor