Stale-plan archive convention + issue-state check (Build Plans rule)#236
Merged
Conversation
CLAUDE.md Build Plans & Chunks section gains two paragraphs: - "Archive plans whose chunk has shipped": move stale plans to .claude/plans/archive/ rather than deleting, so the design rationale stays grep-able locally without polluting the active listing. - "Verify issue state before treating any plan as canonical": sessions starting work on a "next chunk" must run `gh issue view <N> --json state -q .state` first. This is the structural guard that protects across clones — .claude/ is gitignored so the archive convention is local-only, but the issue-state check works for everyone. Why now. The 2026-05-23 session recommended #137 as the next chunk because build-plan.md was present at the repo root; the issue had been closed 18 days prior. The trap was structural ("plan exists" treated as "work is ready"), and now the rule explicitly requires verification. CHANGELOG entry under ### Internal — first live use of the bucket introduced by #231. Doc-only rule clarification, no behavior change. (The local plan archive itself — 19 files moved into .claude/plans/archive/ — is not in this diff because .claude/ and build-plan*.md are gitignored. See the CHANGELOG entry for the inventory.)
This was referenced May 24, 2026
Jason-Vaughan
added a commit
that referenced
this pull request
May 24, 2026
The bug. lib/engines.js#_generateClaudeMd regenerates CLAUDE.md from the DB-stored global rules on every session launch / engine PATCH / startup sync, overwriting the on-disk file in place. PR-driven raw- file edits to CLAUDE.md are silently discarded the next time TC restarts — the content lives on in git log but is functionally absent from the live ruleset. Affected this session. Two PRs: - #234: versioning bump-level decision table (Releases & Versioning section). - #236: stale-plan archive rule + gh issue view <N> --json state issue-state check (Build Plans & Chunks section). Both were clobbered after the first TC restart following each merge. Recovery already performed via store.globalRules.save() in the restart-bug investigation session. The regenerated CLAUDE.md is now byte-for-byte identical to HEAD (diff is empty), so the next TC restart produces no surprise drift. This commit adds: - A defensive comment block to lib/engines.js#_generateClaudeMd documenting the regeneration contract and pointing future readers (human or automated agent) at the three durable edit paths: landing-page editor, PUT /api/rules/global (10 KB body cap), or store.globalRules.save() from a node script. The same regeneration pattern applies to Codex / Aider / Gemini config files (cross- referenced in the comment). - A CHANGELOG ### Fixed entry summarizing the recovery + linking to #240 for the public-facing diagnostic and recovery procedure for other cloners. The structural fix (preventing the trap from ever firing again) is tracked separately in #240 — likely either a tracked-file canonical source with startup sync, or an append-only marker pattern that regeneration preserves. Design call belongs to whoever picks up the structural work. Refs #240
6 tasks
Jason-Vaughan
added a commit
that referenced
this pull request
May 24, 2026
…242) * Structural fix: git file is canonical for global rules (closes #240) Adopts Option 1 from #240's design discussion ("git file is canonical") plus Option D (drift-detection warning). The two together prevent the trap that bit #234 and #236 from ever firing silently again. (1) data/global-rules.md becomes the canonical source. Renamed from data/default-global-rules.md (git mv) and populated with the full 191-line rules content TC-v3 has accumulated. There is no per-install copy at ~/.tangleclaw/global-rules.md anymore — both UI/API edits AND PR-driven file edits land in the same tracked file, so neither path can clobber the other. (2) store.globalRules.load/save rewritten. - load() reads BUNDLED_GLOBAL_RULES directly, no per-install fallback. Auto-heal-on-read preserved. - save() writes there. Normalization preserved. - reset() becomes a no-op + warn. Under the new model "reset" means `git checkout data/global-rules.md`; the UI button should be removed in a follow-up (today it just refreshes). - New test seam: _setBundledGlobalRulesPath / _resetBundledGlobalRulesPath — lets tests redirect without clobbering the real canonical file. - Migration safety: _maybeWarnLegacyGlobalRulesFile fires once on first load() per process; if the old per-install file exists with different content, logs a clear warning with recovery steps. (3) engines.writeEngineConfig drift-detection helper. New helper in lib/engines.js (right before syncEngineHooks) that wraps generateConfig + read-existing + diff + warn + write. When the on-disk engine config file differs from regenerated content, logs `engine config drift detected — overwriting on-disk hand-edits (#240)` with byte counts and a howToInvestigate field naming the durable edit paths. Whitespace-tolerant via .trim() — auto-formatter trailing-newline changes don't false-positive. (4) All four engine-config write sites refactored to use the helper: - lib/projects.js#createProject (~line 284) - lib/projects.js#syncAllProjects (~line 763) - lib/projects.js#updateProject engine-switch (~line 1344) - lib/sessions.js#launchSession (~line 191) Uniform behavior + uniform warning regardless of which code path triggers regeneration. Tests. 6 new cases in test/engines.test.js's new `writeEngineConfig (#240 drift detection)` describe block covering writes-when-absent, writes-when-matches (idempotent), detects-drift- when-differs (the canonical failure mode), tolerates-whitespace- only-diffs, returns-error-on-empty-content, returns-error-on- missing-configFormat. test/store-globalrules.test.js reframed for the canonical-source contract: load/save round-trip via the redirected tracked path; new pin that save() writes to BUNDLED_GLOBAL_RULES; reset() asserted as no-op. test/api- globalrules.test.js updated to seed a tmp canonical file and assert the new reset contract. Full suite 2463/2464 pass (1 pre-existing case-insensitive-host skip). What this means going forward. Any future PR that wants to add or change global rules: edit data/global-rules.md directly, commit, push. The next session launch on any clone reads the updated file via store.globalRules.load() and composes into the regenerated CLAUDE.md. UI/API edits via the landing-page editor go to the same file and become uncommitted local changes the operator can commit if intentional. The previous broken model (PR edits to CLAUDE.md that get clobbered) is structurally impossible now — there's nothing to clobber. Closes #240 * Address #240 PR Critic findings MAJOR (2): regressions for engines without config files. - writeEngineConfig returned an `error` string for openclaw / genesis (engines with configFormat.filename === null) instead of treating them as a deliberate no-op. The 4 call sites would surface that as "Failed to write engine config" on every createProject / launchSession for such engines. - launchSession's guard `if (engineProfile.configFormat)` passed for openclaw (its configFormat is `{filename: null, ...}` — truthy as an object). Fix. Helper now returns `{written: false, skipped: true, skipReason, error: null}` for the two no-op cases (no configFormat.filename, or generateConfig returned empty). All 4 call sites updated to gate on `!skipped` before pushing/logging errors. Two new tests pin the shape: noformat path returns skipped, openclaw-style null-filename returns skipped. MEDIUM (2): - _maybeWarnLegacyGlobalRulesFile is still once-per-process, but now AUTO-BACKUPS the legacy file content to `<legacy>.pre-240-backup` on first warn. Operators who miss the log line have a durable recovery surface (`mv .pre-240-backup global-rules.md` to restore). Warning message now includes the backup path. - Reset-button-is-now-a-no-op UI follow-up filed as #243. MINOR (3 of 4): - Drift-detection comparator now normalizes CRLF→LF before comparing. Windows-saved files don't false-positive. New test pin. - _legacyGlobalRulesWarned flag-set moved to AFTER the work is done (so a future retry-on-load refactor is correct). - CHANGELOG entry trimmed from ~700 words to ~200. Keeps the essentials, drops the session-wrap-style detail. Skipped: explicit declination of Options B + C in PR body. The PR description already explains why Option 1 + D were chosen; declining the other two adds noise without value. Full suite 2465/2466 pass (net +2 from CRLF + null-filename test pins; 1 pre-existing case-insensitive-host skip). Refs #240
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.
What
Adds two new paragraphs to
CLAUDE.mdBuild Plans & Chunks section that codify how shipped plans get cleaned up + how future sessions avoid picking them as "next work":<project-root>/.claude/plans/archive/rather than deleting. Archive preserves design rationale for grep-ability without polluting the active listing.gh issue view <N> --json state -q .statebefore treating any plan as ready-to-execute, even if the file is in the active (non-archive) directory. Archive is the convention; issue-state check is the contract.Why
The 2026-05-23 session recommended #137 as the next chunk after #207 wrapped, on the basis that
build-plan.mdexisted at the repo root. The issue had been closed on 2026-05-05, 18 days prior. A no-op feature branch was created (no commits, rolled back cleanly) before the agent thought to verify the issue state.The trap is structural: "plan file exists" was treated as "work is ready."
.claude/plans/is gitignored (line 5 of.gitignore), so each clone accumulates its own stale plan files locally with no PR / CI gate to surface them. Neither the archive convention alone (would have hidden them fromlsbut still failed across clones) nor the issue-state check alone (would have caught it but adds friction every time) suffices. Both halves together close the loop.What's in this PR (diff)
CLAUDE.md— the two new paragraphs, plus a parenthetical noting the gitignore reality so future readers don't try to make.claude/a tracked surface.CHANGELOG.md[Unreleased]— entry under### Internal(the first live use of the bucket introduced by [enhancement] Versioning rules: introduce ### Internal subsection + tighten minor trigger #231).What's NOT in this PR (intentional, gitignored)
The local plan archive itself: 19 stale plan files moved this session into
.claude/plans/archive/. Inventory:.claude/plans/covering closed issues [feature] Claude Code engine: inject session-start prime via SessionStart hook for silent context delivery #103, [question] Research equivalent silent-prime hooks for Codex, Gemini, and Aider engines #131, [feature] Methodology-aware single-button session wrap #139, ttyd-watcher never fires: pgrep -c unsupported on macOS, also skips zombies #144, [bug] syncEngineHooks injects methodology hooks without verifying their runtime targets exist — orphan hook errors across multiple projects #145, [chore] Generalize template-reconcile beyond wrap.steps — phases, init.directories, prime.sections still drift #155, [bug] Orphan-hook loop still fires on installs whose prawduct runtime template predates the chunk-1requiresfilter (#145) #158, [bug] Dashboard project label shows stale version between session launch/wrap #165, [feature] Feature Index — session launch optimization with wrap-step parity #207, plus the external-trackercheerful-waddling-wadler.md(portfolio repo work, TC-side workflows already removed) and three unnumbered plans self-marked closed:prawduct-aicontent-prompts.md(PR Populate prawduct ai-content prompts (open-queue #2 post-#139) #200),version-bump-handler.md(PR Real version-bump wrap-step handler (open-queue #3 post-#139) #202),replicated-coalescing-journal.md(whosesyncAllProjects()shipped atlib/projects.js:728).build-plan*.mdfiles at the repo root: the [bug] PATCH /api/projects/:name with silentPrime doesn't sync .claude/settings.json until next session launch #137-specificbuild-plan.mdthat triggered this whole janitor pass, plusbuild-plan-canary.md,build-plan-eval-audit.md,build-plan-lllm.md,build-plan-peek.md,build-plan-sidecar.md..claude/plans/andbuild-plan*.mdare both gitignored — the cleanup affected only this local clone. The new rule ensures future sessions on any clone avoid the same trap via the issue-state check.Test plan
CLAUDE.md; verify the rule wording is unambiguous and theghcommand quoted matches working syntax.grep -E "^## \[" CHANGELOG.md | head -5confirms[Unreleased]and[3.18.0]headings both intact post-edit.ls .claude/plans/*.mdreturns nothing (active plans cleaned up);ls .claude/plans/archive/shows 19 entries.### Internalvalidates the [enhancement] Versioning rules: introduce ### Internal subsection + tighten minor trigger #231 wrap-step path on the next wrap (mixed### Added+### Internal→ stillminorbecause### Addedwins).This is the third doc-tier PR of the session — same recommendation as #232 and #234: don't auto-merge, you eyeball it, merge manually with
gh pr merge 236 --squash --delete-branchwhenever satisfied.