[Spec 723] Improve arch.md / lessons-learned.md governance#724
Merged
waleedkadous merged 35 commits intomainfrom May 6, 2026
Merged
[Spec 723] Improve arch.md / lessons-learned.md governance#724waleedkadous merged 35 commits intomainfrom
waleedkadous merged 35 commits intomainfrom
Conversation
Incorporated feedback from gemini, codex, claude: - Fixed factual error: codev-skeleton/.claude/skills/ does exist - Renamed skill to update-arch-docs to avoid collision with global agent - Added explicit lessons-learned.md template criteria - Restructured success criteria into deterministic literal-content checks - Added skill-is-guidance-only constraint (no destructive commands) - Clarified Step 3 sub-step split semantics (3a/3b, not renumber) - Documented template-vs-skill propagation via codev update - Narrowed test target to scaffold tests
Spec ready for human approval at the spec-approval gate. 3-way consultation results (gemini COMMENT, codex REQUEST_CHANGES, claude COMMENT) captured under codev/projects/723-* (txt outputs are gitignored). Rebuttal documents resolution of every reviewer point — no rejected feedback; all 5 Codex items, all 4 Gemini items, all 4 Claude items addressed.
5 changes from architect review: 1. Drop arch-md-guide.md template entirely; fold into arch.md preface + skill body 2. Relax 'guidance-only' to 'no destructive shell commands' — skill MAY edit arch.md/lessons-learned.md directly via normal file-edit tooling 3. Reword skeleton/main parity constraint to explain the why (codev/ is what we use, codev-skeleton/ is what we ship; byte-identical prevents drift) 4. Drop coexistence framing — skill replaces the legacy agent. Removed naming- collision risk row, added 'forgets to delete legacy file' risk and post-merge cutover note 5. Confirm skill name 'update-arch-docs' (architect direction) No new design surface introduced; per architect, no fresh 3-way review needed.
Codex (REQUEST_CHANGES) → all 3 items addressed: 1. Locked the 'Lives where' matrix (8 rows; added 'retired component' row) 2. codev init smoke test — explicit command sequence with pnpm build first; added skeleton-template fallback if dist invocation breaks 3. Tightened Phase 1 wording: skill lives at repo-root .claude/skills/, not codev/ Gemini (APPROVE) → Vitest CLI fix (positional arg, not --testPathPatterns) Claude (APPROVE) → audit-mode echoes 'when in doubt, KEEP'; skill-propagation test spelled out as concrete bash sequence Rebuttal at codev/projects/723-*/723-plan-iter1-rebuttals.md.
…cher arch.md/lessons-learned.md templates Authored artifacts (Phase 1): - .claude/skills/update-arch-docs/SKILL.md — new skill replacing the user-global architecture-documenter agent. Body has 6 named sections covering 'what NOT to include', two-doc framing, purpose-driven sizing, diff-mode, audit-mode (with 'when in doubt, KEEP' echo), and output contract. - codev-skeleton/.claude/skills/update-arch-docs/SKILL.md — byte-identical copy. - codev/templates/arch.md — replaced 56-line stub with richer template: TL;DR, Repository Layout & Stack, Per-Subsystem Mechanism, Apps Roster, Packages Roster, Verified-Wrong Assumptions, Updating This Document. The 'Updating This Document' section folds in the maintenance guide (when/how to update, what NOT to put in, sanity-check checklist). - codev-skeleton/templates/arch.md — byte-identical copy. - codev/templates/lessons-learned.md — added preface with two-doc framing, 'do NOT add' guidance, sanity-check checklist; topical sections preserved. - codev-skeleton/templates/lessons-learned.md — byte-identical copy. Verification: - All three diff -r parity checks: zero output. - Skill literal-content check: frontmatter contains all 4 required phrases; body has all 6 required ## sections. - Scaffold tests: 21/21 passing (cd packages/codev && pnpm exec vitest run scaffold). 3-way review: Gemini APPROVE, Codex REQUEST_CHANGES (untracked-files admin note, resolved by this commit), Claude APPROVE. Rebuttal at codev/projects/723-*/723-implement-phase_1-iter1-rebuttals.md.
…protocol
Phase 2 deliverables:
- codev/protocols/maintain/protocol.md updated with:
- 'Lives where: routing facts to the right home' matrix (8 rows)
placed under 'Key Documents MAINTAIN keeps current'
- Step 3 split into 3a (Audit) and 3b (Update). Steps 1, 2, 4 numbering
preserved (sub-step split, not a renumber).
- Per-arch.md-section pruning checklist + per-lessons-learned.md-entry
pruning checklist inline under Step 3a.
- Sample audit prompt (paste-able) under Step 3a.
- 'Audit Findings' section added to the run file template.
- Cross-references to update-arch-docs skill in both 3a and 3b.
- 'When in doubt, KEEP' rule preserved.
- codev-skeleton/protocols/maintain/protocol.md byte-identical.
Verification (acceptance criteria met):
- diff -r parity: all 4 pairs zero output (templates, protocol, skill).
- Skill literal-content check: passes.
- Scaffold tests: 21/21 passing.
- codev init smoke test: skill propagates correctly. (Note: codev init does
not copy resource templates — pre-existing init.ts behavior; not introduced
here. Skeleton template content verified via direct cat inspection.)
- codev update skill propagation: '+ (new) .claude/skills/update-arch-docs/'
confirmed; skill returns after deletion + update.
- Self-consistency check on live codev/resources/arch.md: 4 candidate-cut
categories found:
1. Per-spec changelog framing (60 'Spec NNNN' references)
2. Exhaustive 'Complete Directory Structure' section (lines 1010-1190 = 180 lines)
3. Per-file enumerations (11 occurrences)
4. Date-stamped/'Historical note' narrative framing
Also tracking the previously-uncommitted Phase 1 rebuttal file.
Addresses Phase 2 REQUEST_CHANGES from gemini and codex (both flagged the same two missing deliverables; Claude APPROVED). The MAINTAIN protocol edits themselves were complete in commit 7e36d98; what was missing were the verification artifacts and PR-prep, both listed as Phase 2 deliverables in the plan. - codev/reviews/723-improve-arch-md-lessons-learne.md (new) - Summary, files-changed list partitioned by phase - All 4 parity checks (zero diff each) - Skill literal-content + MAINTAIN six-edit checklists (all green) - Scaffold tests 21/21 passing - codev init smoke test (skill propagates; templates pre-existing init.ts limitation documented) - codev update skill-propagation success - Self-consistency audit-mode check: 4 candidate-cut categories found - Spec acceptance-criteria final check - 5 lessons-learned candidates for next MAINTAIN run - Pre-existing flaky tests noted (session-manager, not bypassed) - PR description draft inline with post-merge cutover note prominent - codev/projects/723-*/723-phase_2-iter1-rebuttals.md (new)
…ltation Feedback to review Required sections per porch's review-phase checks: - ## Architecture Updates: explains why arch.md needs no updates (governance- only change; no new subsystems or data flows) - ## Lessons Learned Updates: explains why no live lessons-learned.md edits in this PR (consistent with spec out-of-scope; lessons recorded in this review for next MAINTAIN run to evaluate) - ## Consultation Feedback: full per-phase summary across specify (round 1 + architect iteration), plan (round 1), implement phase_1 (round 1), implement phase_2 (round 1)
Codex review-phase REQUEST_CHANGES (HIGH): the spec's success criterion #103 ('codev init produces arch.md/lessons-learned.md') and the new arch.md template's claim ('propagates via codev init/adopt') both overstate what the framework actually does. Verified in init.ts:92-93 and adopt.ts:128: both files carry the explicit comment 'Framework files (protocols, roles, consult-types, templates) are NOT copied. They resolve at runtime from the installed npm package via the unified file resolver.' This is a deliberate foundational design decision that affects every framework template, not just arch.md/lessons-learned.md. Aligning the artifacts with reality (option chosen) rather than flipping the design decision (rejected as out of scope): - codev/templates/arch.md: 'Note on propagation' section rewritten to state honestly that init/adopt/update do NOT copy framework templates; provides a manual-copy command (require.resolve) for projects opting in. - codev-skeleton/templates/arch.md: byte-identical mirror. - codev/reviews/723-*.md: smoke-test section updated with 'Spec deviation noted' explanation; success-criteria final check updated to reference it. Gemini APPROVE and Claude APPROVE stand. Claude's two minor observations (porch artifact duplication; stale Vitest flag in approved spec) are acknowledged but not actioned as documented in the rebuttal.
…lessons-learned.md
Architect feedback (post-PR review): the SPIR protocol's review-phase Step 3
('Update Architecture Documentation') should explicitly invoke the new
update-arch-docs skill and mention lessons-learned.md alongside arch.md.
Three small changes to Step 3 in both copies:
1. Mention lessons-learned.md alongside arch.md (the skill maintains both;
SPIR review previously only mentioned arch.md).
2. Name-drop the update-arch-docs skill explicitly with its path so SPIR-
review readers find the skill without chasing a pointer through MAINTAIN.
3. Update the cross-reference from MAINTAIN's old 'Update arch.md task' to
the current 'Step 3 (Sync Documentation)', which is what this work
reorganized into Step 3a/3b with the Lives-where matrix and pruning
checklists.
The new edit text is byte-identical between codev/ and codev-skeleton/.
Pre-existing drift on this file (SPIR acronym expansion, clean-worktree
warning) was NOT introduced by this work and not addressed here per
architect direction.
waleedkadous
added a commit
that referenced
this pull request
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 Post-merge cutover required (architect action)
After merging this PR, the architect must run:
The user-global agent is being replaced, not coexisting with, the new
update-arch-docsskill. The PR cannot delete this file directly because the path is outside the repo. This step retires the legacy add-biased prompt.Summary
Closes #723. Shifts arch.md / lessons-learned.md governance from "append by default" to "audit, then update." Six scope items from the issue:
update-arch-docsskill: new skill at.claude/skills/update-arch-docs/SKILL.md(and skeleton copy). Body codifies what NOT to include, two-doc framing, purpose-driven sizing, diff-mode and audit-mode (with "when in doubt, KEEP" echo), and an output contract.arch.mdtemplate: replaces the 56-line stub with TL;DR, Repository Layout & Stack, Per-Subsystem Mechanism, Apps Roster, Packages Roster, Verified-Wrong Assumptions, and Updating This Document. The "Updating This Document" preface folds in maintenance guidance (when, how, what NOT, sanity-check checklist).lessons-learned.mdtemplate: adds preface with two-doc framing, "do NOT add" guidance, sanity-check checklist; preserves topical sections.codev-skeleton/carries identical copies of every artifact so other projects pick this up via `codev init` (templates, skills) and `codev update` (skills only — templates are init-time only).Out of scope
Verification
Files changed
Phase 1 (skill + templates):
Phase 2 (MAINTAIN wiring):
Spec / Plan / Review