fix(bundled-defaults): auto-generate import list, emit inline strings#1263
Merged
fix(bundled-defaults): auto-generate import list, emit inline strings#1263
Conversation
Root-cause fix for bundle drift (15 commands + 7 workflows previously
missing from binary distributions) and a prerequisite for packaging
@archon/workflows as a Node-loadable SDK.
The hand-maintained `bundled-defaults.ts` import list is replaced by
`scripts/generate-bundled-defaults.ts`, which walks
`.archon/{commands,workflows}/defaults/` and emits a generated source
file with inline string literals. `bundled-defaults.ts` becomes a thin
facade that re-exports the generated records and keeps the
`isBinaryBuild()` helper.
Inline strings (via JSON.stringify) replace Bun's
`import X from '...' with { type: 'text' }` attributes. The binary build
still embeds the data at compile time, but the module now loads under
Node too — removing SDK blocker #2.
- Generator: `scripts/generate-bundled-defaults.ts` (+ `--check` mode for CI)
- `package.json`: `generate:bundled`, `check:bundled`; wired into `validate`
- `build-binaries.sh`: regenerates defaults before compile
- Test: `bundle completeness` now derives expected set from on-disk files
- All 56 defaults (36 commands + 20 workflows) now in the bundle
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA generator was added to produce a compiled bundle of command and workflow defaults from .archon disk files. CI, build scripts, tests, and config were updated to run or ignore the generated output; the bundle is emitted to Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant CI as CI
participant Gen as Generator (scripts/generate-bundled-defaults.ts)
participant FS as File System (.archon/…)
participant Build as Build Script (scripts/build-binaries.sh)
participant Tests as Test Suite
Dev->>Gen: bun run generate:bundled (or bun run check:bundled)
Gen->>FS: read .archon/commands/defaults/*.md and .archon/workflows/defaults/*.{yaml,yml}
FS-->>Gen: file listings & contents
Gen->>Gen: validate filenames, non-empty, no collisions
Gen->>Gen: render bundled-defaults.generated.ts
Gen-->>Dev: write file (or exit 2 if --check stale)
CI->>Gen: bun run check:bundled (during validate)
Gen->>FS: read & compare generated file
Gen-->>CI: exit 0 (up-to-date) or non-zero (fail)
Build->>Gen: run generator (pre-build)
Gen-->>Build: provide updated generated bundle
Build->>Build: compile binaries using generated bundle
Tests->>FS: discover on-disk defaults
Tests-->>Tests: assert BUNDLED_* keys and contents match on-disk files
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/generate-bundled-defaults.ts (1)
36-39: Prefer a shared interface for file entries instead of repeated inline object shapes.This will reduce repetition and keep the signatures easier to maintain.
♻️ Suggested refactor
+interface BundledFileEntry { + name: string; + content: string; +} + async function collectFiles( dir: string, extensions: readonly string[] -): Promise<Array<{ name: string; content: string }>> { +): Promise<BundledFileEntry[]> { const entries = await readdir(dir); const matched = entries.filter(entry => extensions.some(ext => entry.endsWith(ext))).sort(); - const files: Array<{ name: string; content: string }> = []; + const files: BundledFileEntry[] = []; @@ function renderRecord( comment: string, typeAlias: string, exportName: string, - files: Array<{ name: string; content: string }> + files: BundledFileEntry[] ): string { @@ function renderFile( - commands: Array<{ name: string; content: string }>, - workflows: Array<{ name: string; content: string }> + commands: BundledFileEntry[], + workflows: BundledFileEntry[] ): string {As per coding guidelines, "TypeScript: Use
interfacefor defining object shapes in TypeScript".Also applies to: 43-43, 67-67, 81-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-bundled-defaults.ts` around lines 36 - 39, Introduce a shared TypeScript interface (e.g., FileEntry { name: string; content: string }) and replace all inline object types like Array<{ name: string; content: string }> in functions such as collectFiles and the other spots flagged (the other functions that return/handle { name, content } entries) with this interface; update function return types, parameter types, and any local variable annotations to use FileEntry to avoid repeated inline shapes and keep signatures consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/generate-bundled-defaults.ts`:
- Around line 47-59: The collectFiles function currently normalizes filenames
(const name = entry.slice(0, -ext.length)) and then pushes into the files array,
which allows collisions like foo.yaml vs foo.yml to silently overwrite; add a
duplicate-name guard before files.push: check whether files already contains an
item with the same name (using the normalized name variable) and if so throw a
clear Error mentioning the duplicate entry and directory (including both
conflicting filenames if available), and apply the same guard in the other
identical spot around the second files.push to prevent silent overwrites.
---
Nitpick comments:
In `@scripts/generate-bundled-defaults.ts`:
- Around line 36-39: Introduce a shared TypeScript interface (e.g., FileEntry {
name: string; content: string }) and replace all inline object types like
Array<{ name: string; content: string }> in functions such as collectFiles and
the other spots flagged (the other functions that return/handle { name, content
} entries) with this interface; update function return types, parameter types,
and any local variable annotations to use FileEntry to avoid repeated inline
shapes and keep signatures consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57cfcccb-a945-4271-a9cd-5565fd28bf24
📒 Files selected for processing (8)
.prettierignoreeslint.config.mjspackage.jsonpackages/workflows/src/defaults/bundled-defaults.generated.tspackages/workflows/src/defaults/bundled-defaults.test.tspackages/workflows/src/defaults/bundled-defaults.tsscripts/build-binaries.shscripts/generate-bundled-defaults.ts
Collaborator
Author
PR Review Summary (multi-agent)Reviewed by: Critical Issues (0)None. Core approach is sound (generator + committed generated file + drift-detection). Important Issues (4)
Suggestions (8)
Documentation Issues (2)
Plus: add a bullet under the Defaults section of Strengths
VerdictNEEDS FIXES — no blocking defects, but two of the important issues (the Recommended Actions
|
Review: #1263 (comment) Generator: - Guard against .yaml/.yml name collisions (previously silent overwrite) - Add early access() check with actionable error when run from wrong cwd - Type top-level catch as unknown; print only message for Error instances - Drop redundant /* eslint-disable */ emission (global ignore covers it) - Fix misleading CI-mechanism claim in header comment - Collapse dead `if (!ext) continue` guard into a single typed pass Scripts get real type-checking + linting: - New scripts/tsconfig.json extending root config - type-check now includes scripts/ via `tsc --noEmit -p scripts/tsconfig.json` - Drop `scripts/**` from eslint ignores; add to projectService file scope Tests: - Inline listNames helper (Rule of Three) - Drop redundant toBeDefined/typeof assertions; the Record<string, string> type plus length > 50 already cover them - Add content-fidelity round-trip assertion (defense against generator content bugs, not just key-set drift) Facade comment: drop dead reference to .claude/rules/dx-quirks.md. CI: wire `bun run check:bundled` into .github/workflows/test.yml so the header's CI-verification claim is truthful. Docs: CLAUDE.md step count four→five; add contributor bullet about `bun run generate:bundled` in the Defaults section and CONTRIBUTING.md.
gpt-5.1-codex-mini is deprecated and unavailable on ChatGPT-account Codex
auth. Plain gpt-5.2 works. Verified end-to-end:
- e2e-codex-smoke: structured output returns {category:'math'}
- e2e-mixed-providers: claude+codex both return expected tokens
This was referenced Apr 16, 2026
ztech-gthb
pushed a commit
to ztech-gthb/Archon
that referenced
this pull request
Apr 18, 2026
…coleam00#1263) * fix(bundled-defaults): auto-generate import list, emit inline strings Root-cause fix for bundle drift (15 commands + 7 workflows previously missing from binary distributions) and a prerequisite for packaging @archon/workflows as a Node-loadable SDK. The hand-maintained `bundled-defaults.ts` import list is replaced by `scripts/generate-bundled-defaults.ts`, which walks `.archon/{commands,workflows}/defaults/` and emits a generated source file with inline string literals. `bundled-defaults.ts` becomes a thin facade that re-exports the generated records and keeps the `isBinaryBuild()` helper. Inline strings (via JSON.stringify) replace Bun's `import X from '...' with { type: 'text' }` attributes. The binary build still embeds the data at compile time, but the module now loads under Node too — removing SDK blocker coleam00#2. - Generator: `scripts/generate-bundled-defaults.ts` (+ `--check` mode for CI) - `package.json`: `generate:bundled`, `check:bundled`; wired into `validate` - `build-binaries.sh`: regenerates defaults before compile - Test: `bundle completeness` now derives expected set from on-disk files - All 56 defaults (36 commands + 20 workflows) now in the bundle * fix(bundled-defaults): address PR review feedback Review: coleam00#1263 (comment) Generator: - Guard against .yaml/.yml name collisions (previously silent overwrite) - Add early access() check with actionable error when run from wrong cwd - Type top-level catch as unknown; print only message for Error instances - Drop redundant /* eslint-disable */ emission (global ignore covers it) - Fix misleading CI-mechanism claim in header comment - Collapse dead `if (!ext) continue` guard into a single typed pass Scripts get real type-checking + linting: - New scripts/tsconfig.json extending root config - type-check now includes scripts/ via `tsc --noEmit -p scripts/tsconfig.json` - Drop `scripts/**` from eslint ignores; add to projectService file scope Tests: - Inline listNames helper (Rule of Three) - Drop redundant toBeDefined/typeof assertions; the Record<string, string> type plus length > 50 already cover them - Add content-fidelity round-trip assertion (defense against generator content bugs, not just key-set drift) Facade comment: drop dead reference to .claude/rules/dx-quirks.md. CI: wire `bun run check:bundled` into .github/workflows/test.yml so the header's CI-verification claim is truthful. Docs: CLAUDE.md step count four→five; add contributor bullet about `bun run generate:bundled` in the Defaults section and CONTRIBUTING.md. * chore(e2e): bump Codex model to gpt-5.2 gpt-5.1-codex-mini is deprecated and unavailable on ChatGPT-account Codex auth. Plain gpt-5.2 works. Verified end-to-end: - e2e-codex-smoke: structured output returns {category:'math'} - e2e-mixed-providers: claude+codex both return expected tokens
This was referenced Apr 18, 2026
Closed
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…coleam00#1263) * fix(bundled-defaults): auto-generate import list, emit inline strings Root-cause fix for bundle drift (15 commands + 7 workflows previously missing from binary distributions) and a prerequisite for packaging @archon/workflows as a Node-loadable SDK. The hand-maintained `bundled-defaults.ts` import list is replaced by `scripts/generate-bundled-defaults.ts`, which walks `.archon/{commands,workflows}/defaults/` and emits a generated source file with inline string literals. `bundled-defaults.ts` becomes a thin facade that re-exports the generated records and keeps the `isBinaryBuild()` helper. Inline strings (via JSON.stringify) replace Bun's `import X from '...' with { type: 'text' }` attributes. The binary build still embeds the data at compile time, but the module now loads under Node too — removing SDK blocker #2. - Generator: `scripts/generate-bundled-defaults.ts` (+ `--check` mode for CI) - `package.json`: `generate:bundled`, `check:bundled`; wired into `validate` - `build-binaries.sh`: regenerates defaults before compile - Test: `bundle completeness` now derives expected set from on-disk files - All 56 defaults (36 commands + 20 workflows) now in the bundle * fix(bundled-defaults): address PR review feedback Review: coleam00#1263 (comment) Generator: - Guard against .yaml/.yml name collisions (previously silent overwrite) - Add early access() check with actionable error when run from wrong cwd - Type top-level catch as unknown; print only message for Error instances - Drop redundant /* eslint-disable */ emission (global ignore covers it) - Fix misleading CI-mechanism claim in header comment - Collapse dead `if (!ext) continue` guard into a single typed pass Scripts get real type-checking + linting: - New scripts/tsconfig.json extending root config - type-check now includes scripts/ via `tsc --noEmit -p scripts/tsconfig.json` - Drop `scripts/**` from eslint ignores; add to projectService file scope Tests: - Inline listNames helper (Rule of Three) - Drop redundant toBeDefined/typeof assertions; the Record<string, string> type plus length > 50 already cover them - Add content-fidelity round-trip assertion (defense against generator content bugs, not just key-set drift) Facade comment: drop dead reference to .claude/rules/dx-quirks.md. CI: wire `bun run check:bundled` into .github/workflows/test.yml so the header's CI-verification claim is truthful. Docs: CLAUDE.md step count four→five; add contributor bullet about `bun run generate:bundled` in the Defaults section and CONTRIBUTING.md. * chore(e2e): bump Codex model to gpt-5.2 gpt-5.1-codex-mini is deprecated and unavailable on ChatGPT-account Codex auth. Plain gpt-5.2 works. Verified end-to-end: - e2e-codex-smoke: structured output returns {category:'math'} - e2e-mixed-providers: claude+codex both return expected tokens
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.
Summary
bundled-defaults.tsis a hand-maintained import list. It had drifted by 15 commands + 7 workflows, so 22 on-disk defaults were silently missing from compiled binary distributions.archon-plan-to-prand other bundled workflows reference commands (archon-plan-setup,archon-confirm-plan,archon-finalize-pr, etc.) that shipped as dangling pointers in the binary. Also: thewith { type: 'text' }import attributes are Bun-specific and block loading@archon/workflowsfrom Node (SDK blocker updated code to use locally hosted llama LLM, nomic-embed-text model. #2)..archon/{commands,workflows}/defaults/and emitsbundled-defaults.generated.tswith inline string literals.bundled-defaults.tsbecomes a thin facade keepingisBinaryBuild(). CI verifies the generated file is up to date viabun run check:bundled, which is wired intobun run validate. The build script regenerates before compile.BUNDLED_IS_BINARYflip mechanism, opt-out config. Zero behavior change for existing users — just: the bundle now contains all 56 defaults instead of 34.Label Snapshot
Change Metadata
Linked Issue
Validation Evidence
```
bun run check:bundled ✔ 36 commands, 20 workflows
bun run type-check ✔ all 10 packages
bun run lint ✔
bun run format:check ✔
bun --filter @archon/workflows test ✔ all green (162 tests)
bun run cli workflow list ✔ loads 20 bundled workflows at runtime
```
Pre-existing failures on `dev` unrelated to this PR: 3 × `connection > getDatabaseType` in `@archon/core` (verified by checkout + rerun of `dev`).
Security Impact
Compatibility / Migration
Human Verification
Side Effects / Blast Radius
Rollback Plan
Risks and Mitigations
Summary by CodeRabbit
Chores
Tests
Documentation
Workflows