feat(migrate): declared _gates key for sidebars + full lint:frontmatter coverage - #166
feat(migrate): declared _gates key for sidebars + full lint:frontmatter coverage#166herosjourney wants to merge 1 commit into
Conversation
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Reviewed all 14 changed files end-to-end: the validator implementation (parse.ts, types.ts, check.ts), all 4 test cases, the INTERPRETER.md additions (shared + 3 vendored copies), workshop-invariants (shared + 3 vendored), the vercel workshop.md frontmatter line, and mise.toml.
Summary: Clean, well-structured extension. _gates correctly mirrors the existing _advances_to on-disk resolution pattern, the closed-vocab addition is minimal, tests cover the four key invariants (valid, dangling, terminal, backbone-rejects), and the prose documentation is precise. One unimplemented check noted below (non-blocking).
Merge note: PR shows CONFLICTING state — main is 23 commits ahead and 8 of this PR's files were also touched on main (likely from other PRs merging into the vendored trees and mise.toml). A rebase will be needed before merge; the conflicts should be additive/trivial since the _gates content is entirely new.
Recommended for merge after rebase (no blocking findings).
| // (a) the target must not be a terminal; | ||
| // (b) the target phase must EXIST ON DISK (same partial-rollout-tolerant | ||
| // resolution as the _advances_to dangling-edge check); | ||
| // (c) when the target's frontmatter is present, it must be a backbone phase |
There was a problem hiding this comment.
[🤖 AI review 🤖]
Optional: The comment on lines 63-65 describes check (c): "when the target's frontmatter is present, it must be a backbone phase (gating one sidebar on another expresses nothing the trigger doesn't)." — but this check isn't actually implemented in the code below. The code only checks (a) terminal and (b) exists-on-disk.
This is fine to leave as a documented future intent (the partial-rollout tolerance means you'd need to look up whether the target has frontmatter and what its role is, which adds complexity for a scenario that may never arise). But if the comment is meant to be a guarantee rather than a TODO, the implementation is missing. Consider either implementing it or rewriting the comment as a // TODO: or // NOTE: not yet enforced to make the gap explicit.
…ter coverage Declare `_gates` as a validator-enforced sidebar frontmatter key (hold a backbone phase while unresolved). Cover heroku/agent-advisor/gcp in lint:frontmatter. Enforce check (c): when the gate target declares frontmatter it must be backbone. Carry `_gates: generate` on heroku's workshop (vercel-to-aws was removed on main in awslabs#179). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Rebased onto current Merge conflicts (from #179 unshipping vercel-to-aws):
Review comment (check c): Implemented — when the
|
f00679d to
8e4f2ce
Compare
2 participants
Summary
Two related CI-hardening changes, completing the sidebar-grammar proposal from the what-if workshop review (the
checkpoint → sidebarrename landed in #162; this adds the third piece):_gatesdeclared key. "Hold Generate until the workshop resolves" lived only in interpreter/invariants prose. It's now a declared, validator-enforced frontmatter key: a sidebar may carry_gates: <backbone-phase>, meaning that phase must not start while the sidebar is unresolved. Declining still resolves the sidebar ("completed"), so the key holds ordering, never participation. The construct generalizes — a future "re-probe Discover" sidebar would use the same key anchored elsewhere.lint:frontmatternow covers all four skills. It previously validated only heroku-to-aws and agent-advisor; vercel-to-aws (8 phase files with frontmatter today) and gcp-to-aws (0 today; guarded as frontmatter rolls out) are added. The validator already caught a real bug on vercel during the feat(heroku-to-aws): post-Estimate what-if workshop checkpoint #152 review round — it just wasn't wired into CI for that skill.Changes
tools/frontmatter-validator/:_gatesadded to the closed phase-key vocab (parse.ts), typed model (types.ts), and checks (check.ts): sidebar-only; target must not be a terminal; target must exist on disk (same partial-rollout-tolerant resolution as the_advances_todangling-edge check); backbone phases must not declare it.tests/tools/frontmatter-validator.test.ts: 4 new cases (valid gate, dangling gate, terminal gate, gate on backbone) — 61 pass.skills/shared/dsl/INTERPRETER.md(canonical):_gatesrow in the phase-key table + semantics in the sidebar section; synced to the vendored copies.skills/shared/workshop/workshop-invariants.md(canonical): sidebar-state section now points at the declared key; synced to the 3 vendored copies.skills/vercel-to-aws/references/phases/workshop/workshop.md:_gates: generateadded to the frontmatter (the only workshop file with frontmatter today; heroku/gcp pick the key up when they adopt phase frontmatter).mise.toml: twolint:frontmatterinvocations added (vercel-to-aws, gcp-to-aws).Relationship to other PRs
Independent — based on current
main(post-#162, which did the sidebar rename this builds on). Touches the validator (no open PR does), vendored DSL/invariants trees viashared:sync(no open PR touches them), and one frontmatter line of vercel workshop.md.Test plan
_gatescases)lint:frontmattergreen on all four skills (heroku 10, agent-advisor, vercel 8 incl. the new_gates: generate, gcp 0)shared:checkgreen across the 4 vendored trees after syncmise run buildgreen (160/160 checks)