Ci/fleet standard rollout - #50
Closed
behindthedash wants to merge 16 commits into
Closed
behindthedash wants to merge 16 commits into
behindthedash wants to merge 16 commits into
Conversation
Fixes discovered and fixed while dogfooding aspens v0.9.0 against
behindthedash/datalena (1,701 files) via the OpenCode backend, then
reproduced with the Claude backend too:
- chooseReuseSourceTarget picked the reuse-context source by raw file
existence, not content substance. A CLAUDE.md that's just a one-line
`@AGENTS.md` import (a standard convention) was fed to the model as
"existing content to preserve" instead of the real content in
AGENTS.md, causing --strategy improve to collapse hundreds of lines
of real docs down to a handful. Now checks for a trivial import stub
and prefers whichever target actually has substantive content.
- The root-instructions-file generation prompt never received the
improve-mode strategyNote instruction ("preserve existing content,
don't summarize away") that every other generation call gets --
a straight omission.
- doc-init.md / doc-init-domain.md / doc-init-claudemd.md all used the
diff-based preservation-contract partial (written for doc-sync, which
has an actual git diff to reason from) even though doc init has no
diff -- swapped to the existing preservation-contract-refresh variant
that's worded for verify-against-the-codebase instead.
- New guard: when the model second-guesses itself and writes a
clarifying question instead of real file content (observed live: a
numbered list of options ending in "let me know"), the existing
file-tag retry loop now also catches and retries on that, instead of
writing it to disk as-is.
- installGitHook (and its two existence-check call sites) assumed
`<repo>/.git/hooks` is always a valid path. In a linked git worktree
`.git` is a file, not a directory, so mkdir'ing under it threw
ENOTDIR. Now resolves the real hooks dir via
`git rev-parse --git-common-dir`.
- --dry-run and the two unconditional write-confirm prompts have no
non-interactive bypass at all -- added --yes for CI/scripted use.
- Added 4 scanner detection features from open good-first-issues
(CI/CD platform, DB migration tool -- scoped to Alembic + Drizzle,
the tools actually in use across our own repos -- API specs, Next.js
App Router architecture).
Known remaining issue: CLAUDE.md is still sometimes written even with
--target codex only, in ~2/3 live runs. buildOutputFilesForTargets was
the prime suspect but is proven correct in isolated testing (see the
new regression test) -- the actual leak mechanism is still unknown.
Not yet opened upstream -- staying in the fork pending further
investigation of the CLAUDE.md leak.
…ctions content Real-world observation running against datalena's AGENTS.md (434 lines): even with the earlier reuse-context and strategyNote fixes in place, the model can still non-deterministically collapse existing content down to a small fraction of its length on any given run (6 lines on one run, 109 on another, same code/prompt). looksLikeDrasticContentLoss catches this by comparing generated length against the existing reuse-source content's length (30% floor, only applied when there's at least 500 chars of existing content to judge against) and retries with an explicit "you discarded too much, preserve nearly all of it" instruction, reusing the same retry infrastructure as the conversational-non-answer guard.
doc-init.js referenced BACKENDS (for the no-backend-installed error message and the interactive backend-select prompt) without importing it from lib/backend.js, causing a ReferenceError at runtime. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CoKkBwnTpsrBdBsZwNBh3A
Fixes 8 bugs surfaced by code review across the claude/codex/opencode backend generalization (commits d096ee1, d059e39): - generateAllAtOnce (the default path for most repos) never applied the conversational-non-answer/drastic-content-loss retry checks, and never retried or warned when CLAUDE.md was missing from the output entirely — only generateChunked had these protections. - Selecting claude + opencode targets together silently corrupted skill files (both write .claude/skills); the conflict guard only checked instructionsFile collisions, not skillsDir. - Re-running doc init on an OpenCode-only repo misidentified it as Codex (both share AGENTS.md as instructionsFile), causing reuse-source lookup to search the wrong skills directory and find nothing. - `aspens doc impact --backend opencode` silently no-op'd even with OpenCode installed — the analysis gate never checked available.opencode. - Removing a doc-sync hook from a linked git worktree silently failed — removeGitHook still resolved `.git/hooks` directly instead of using the worktree-aware common-dir path installGitHook already uses. - `--yes` (for scripted/CI use) didn't gate the post-commit hook install confirm, so scripted runs still blocked on stdin. - OpenCode output skipped the Claude-Code-only content sanitization that Codex output gets, since only the directory-scoped transform applied it. - runOpenCode's raw-output fallback buffer stopped accumulating as soon as any text part existed, even an empty one — truncating the fallback on runs where an early empty text event precedes real content or a crash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CoKkBwnTpsrBdBsZwNBh3A
Fix doc-init reliability: BACKENDS import + multi-backend generation gaps
`aspens doc init` hung indefinitely on non-interactive stdin (worktrail's addon runs it via subprocess.run(capture_output=True) with no --yes and often no --backend/--target either): the reuse-domains p.confirm() at doc-init.js:434 was missing the `!options.yes` guard every sibling confirm in the file already had, and no prompt in the file checked for a TTY at all — so with a genuinely open, never-closed stdin pipe, @clack/prompts blocks forever on the very first prompt reached, which in production is usually the backend/target selection, not just the reuse-domains confirm. Every p.confirm/p.select/p.multiselect in doc-init.js now either resolves via its own flag or --yes's documented default, or fails fast with a clear stderr message when stdin isn't a TTY, instead of hanging. doc-sync.js's one prompt was already TTY-guarded and needed no change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both new tests hardcoded --backend claude, so they'd fail (not just skip) on stock CI runners with no claude/codex/opencode CLI installed — doc-init.js's Step 0 backend-availability check throws before either test's guarded prompt code is ever reached. Detect an available backend at test time (mirroring backend.js's own --version probe) and skip cleanly when none is present, matching this repo's actual CI (ubuntu-latest, no backend CLI installed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: guard every doc-init prompt against non-interactive stdin hangs
…nteractive stdin hangs Extends the non-interactive-stdin fix from doc-init.js (PR #2) to the four remaining commands that still hung indefinitely when stdin has no data and never closes (e.g. worktrail's subprocess.run(capture_output=True)): - doc-impact.js: --apply's confirm had no bypass at all; added --yes - add.js: the resource picker now fails fast when no name is given - customize.js: the update confirm now fails fast; added --yes - save-tokens.js: the feature picker/install confirm now fail fast (message points to --recommended, the existing non-interactive path) Extracted the isInteractive()/failNonInteractive() guard pair from doc-init.js into src/lib/interactive.js so the four new call sites share one implementation instead of re-duplicating it.
…ing-commands fix: guard doc-impact/add/customize/save-tokens prompts against non-interactive stdin hangs
…aph writes (#4) Three independently reproducible doc-init reliability bugs from the soak-period handoff: - parseLLMOutput's untagged-text fallback (meant to catch Codex/OpenCode responses missing <file> tags) gated on an allowedPaths-shape check that was always false at every real call site (canonical generation always passes allowedPaths: null), so it could never fire for any backend. Replaced with an explicit singleFile flag each call site already knows statically, since the fallback is only safe on a true single-file prompt (wrapping a multi-file all-at-once response would stuff the whole response into one file). - OpenCode's unrestricted agentic tool-calling loop adds wall-clock overhead beyond model latency that Claude/Codex's more restricted execution doesn't incur; autoTimeout now applies a 1.5x multiplier to the size-based default specifically for the opencode backend, without overriding an explicit --timeout/ASPENS_TIMEOUT. - persistGraphArtifacts wrote .claude/graph.json, code-map.md, graph-index.json, and appended to .gitignore unconditionally, even during --dry-run, contradicting the "no files written" guarantee doc-init prints. Found via the regression test for the dry-run kill-mid-run bug report, not the originally reported code path — dry-run now short-circuits before any of those writes. Claude-Session: https://claude.ai/code/session_01TnSRM5L5zAzFw2tdp5YNZB Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…non-destructive rewrite redesign (#5) * fix: use --tools (real allowlist) instead of --allowedTools for read-only generation calls Root cause of the CLAUDE.md/AGENTS.md content-leak class of bugs (aspens work-queue brief 20260815-135359, items #6/#9): `--allowedTools` only pre-approves tools for Claude Code's interactive permission prompt. In non-interactive `-p` mode (which runClaude always uses) there's no prompt to skip, so `--allowedTools` has no restrictive effect and the CLI's full default tool set (Write, Edit, Bash, MCP servers, ...) stays available regardless. Confirmed via live probes against the installed Claude CLI (2.1.233): `claude -p --allowedTools Read,Glob,Grep` still wrote a file when asked; `claude -p --tools Read,Glob,Grep` correctly refused (`--tools` defines the actually-available tool set). This let doc-init's "read-only" generation calls (makeClaudeOptions) write canonical files directly via their own Write tool, bypassing aspens' own target-based filtering entirely — reproduced live: a `--target codex` run still wrote CLAUDE.md and a partial .claude/skills/* tree to disk, neither of which ever appeared in aspens' own write-tracking (buildOutputFilesForTargets correctly excludes them; the LLM just wrote them itself). Regression test spawns a fake claude CLI that records its argv and asserts `--tools` (not `--allowedTools`) is passed when a tool restriction is requested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TnSRM5L5zAzFw2tdp5YNZB * fix: stop leaking base-skill content into root instructions; write Skills index to an aspens-owned file Two more contributions to brief 20260815-135359 (items #6/#9) plus the companion brief 20260816-171547 redesign, on top of the --tools fix: 1. buildRootInstructions (target-transform.js) fell back to the base skill's own raw content (frontmatter and all) as a stand-in for root instructions prose whenever no instructionsFile was available. During doc-init's chunked incremental writes, the root instructions file doesn't exist yet while base/domain skills are still generating, so this fallback wrote the base skill's content directly as AGENTS.md/CLAUDE.md — and because that write wasn't forced, a later correct write (once the real instructions file existed) got silently skipped as "already exists". Reproduced live (base skill's YAML frontmatter and all landing in AGENTS.md on a --target codex run) and fixed by removing the fallback: when there's nothing real to write yet, write nothing. 2. Item #9 (Skills-section under-population): the canonical CLAUDE.md's own `## Skills` section computed its skill list from only the in-memory `allFiles` generated in the current invocation, unlike the codex/opencode directory-scoped transform, which already merges on-disk skills via collectSkillsForList. A partial-retry run (e.g. `--mode base-only` recovering a failed root-instructions generation) only regenerates the base skill, so CLAUDE.md's Skills section undercounted every domain skill already on disk. Now reuses the same collectSkillsForList on-disk-merge helper. 3. Companion brief 20260816-171547: stop injecting `## Skills`/`## Behavior` content directly into CLAUDE.md. Write that content to an aspens-owned index file (.claude/aspens-index.md, always safe to regenerate wholesale) and maintain a single delimited `<!-- aspens:start -->`/`<!-- aspens:end -->` block in CLAUDE.md that imports it via Claude Code's `@path` syntax — only ever replacing content between the markers, appending them at the end rather than inserting mid-document, and migrating away any pre-existing inline `## Skills`/`## Behavior` section from an older aspens run. Wired into both doc-init's generation path and doc-sync's repairDeterministicSections. Scoped to the claude target only: AGENTS.md has no working `@path` import mechanism, so codex/opencode keep the existing inline-injection behavior (buildRootInstructions, unchanged by this commit). Live-verified end to end against a fresh scratch fixture with all three fixes applied (plus the --tools fix from the prior commit): a `--target codex --mode chunked` run produced no CLAUDE.md, no .claude/ directory, and a correctly-populated AGENTS.md — the exact symptoms reported in the brief's pullhook incident no longer reproduce. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TnSRM5L5zAzFw2tdp5YNZB --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the placeholder no-op lint script with a real OXLint gate in CI, and pins actions/checkout to the doctrine baseline version.
Review flagged npx oxlint . as an unpinned dependency fetch on every CI run, deviating from both the task's literal instruction and fleet doctrine's oxlint . invocation.
The bare `oxlint .` step failed with "command not found" since nothing put the binary on PATH (no devDependency, no npx, no global install).
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (38)
WalkthroughThe CLI now supports non-interactive execution and ChangesCLI and documentation generation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant scanRepo
participant Backend
participant runOpenCode
participant targetTransform
participant FileSystem
CLI->>scanRepo: scan repository
CLI->>Backend: select available backend
CLI->>runOpenCode: generate instruction content
runOpenCode-->>CLI: parsed or raw output
CLI->>targetTransform: build target files
targetTransform->>FileSystem: persist generated files
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Opened by mistake — gh pr create defaulted to the upstream parent repo instead of the behindthedash/aspens fork. Closing; this repo is in a soak period with no upstream PRs. Correct PR will be opened against the fork instead. |
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
Why
Closes #
How I tested
npm testpasses--dry-runoutput looks correct (if applicable)Checklist
Summary by CodeRabbit
--yesoptions for scripted documentation workflows.