bugfix: surface --format markdown and --exclude glob filter#238
Conversation
, #237) - Add markdown as valid --format value (index.ts + surface.ts); reuses existing formatSurfaceMarkdown logic already exported from @stackbilt/surface - Add --exclude <glob> flag to charter surface; supports plain path prefixes and * / ** wildcards to exclude directories from scanning - Extend SurfaceInputSchema with excludeGlobs field + pass through in analyze() - Add matchGlob() helper to surface package (no new deps) - 5 new regression tests: excludeGlobs schema default, plain path prefix, ** wildcard, SurfaceInputSchema defaults check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
stackbilt-admin
left a comment
There was a problem hiding this comment.
CHANGES_REQUESTED
Reviewed against issues #236 (--format markdown rejected) and #237 (scaffold template false-positives). The markdown format fix and the glob filtering mechanism are both structurally correct. Three issues require changes before merge.
1. --exclude flag-value theft from adjacent positional flags (BLOCKING)
packages/cli/src/commands/surface.ts, lines 22–30
The --exclude collection loop and getFlag both operate on the same raw args array with no coordination. When --exclude immediately follows --root or --schema with no value between them, getFlag returns the string '--exclude' as the value for that flag.
Concrete failure:
```
charter surface --root --exclude src/gen
```
getFlag(args, '--root')returns'--exclude'→rootArg = '--exclude'→ resolves to/cwd/--exclude(non-existent)- The exclude loop correctly collects
'src/gen' walkFilescatches the missing directory and returns empty; the command throws "No routes or schema tables detected" with no hint that flags were misparsed
Symmetric case: charter surface --schema --exclude src/gen → schemaFlag = '--exclude' → path.resolve('--exclude') passed as schema path → silently reads nothing.
Fix: validate that args[i+1] is not a flag string (does not start with --) before pushing it as a glob value. Alternatively, use getFlag-style multi-value parsing so all flag reads go through one consistent pass.
2. --format markdown falls through to text output on all commands except surface (BLOCKING)
packages/cli/src/index.ts, line 112 — CLIOptions.format is now 'text' | 'json' | 'markdown'
Every other command (score, doctor, validate, drift, audit, bootstrap, etc.) uses the pattern:
```typescript
if (options.format === 'json') { ... } else { /* text output */ }
```
--format markdown lands in the else arm and emits ANSI-styled text output. There is no markdown formatter for those commands (which is fine), but there is also no error. A user who runs:
```
charter validate --format markdown
```
gets plain text with no warning that markdown is not supported for this command.
The fix does not require implementing markdown formatters everywhere. The cleanest fix is to scope 'markdown' as a surface-only flag — either validate it in surfaceCommand (accept it there, reject it globally), or emit a warning in run() when rawFormat === 'markdown' and the command is not surface. At minimum, the help text for the global --format option should document which commands support markdown.
3. matchGlob regex construction for \x00/ (**/) replacement
packages/surface/src/index.ts, lines 139–142
The replacement string for **/ is '(?:[^/]+/)*' — confirmed correct with the * quantifier present. However, a subtle ordering issue exists: the \x00/ substitution replaces **/ (two stars then a slash), but a pattern like a/**/b first converts ** to \x00, yielding a/\x00/b. Then \x00/ (null + slash) matches and produces a/(?:[^/]+/)*b. This is correct.
The edge case worth documenting: ** appearing at the very end of a pattern without a following slash (e.g., packages/**) hits the trailing \x00 → .* branch, not the \x00/ branch. This produces packages/.* which is correct for excluding all files under packages/. Both tests in the PR exercise this path correctly ('**/codegen/**' and 'packages/codegen'), but there is no test for packages/** (trailing **). Please add a test case for this pattern to the matchGlob or excludeGlobs test block to lock in the behavior.
This is not a blocker but is the pattern most likely to be used in real --exclude invocations.
Checklist for the issues being fixed:
-
--format markdownaccepted and routes toformatSurfaceMarkdown()—surface.tsline 24 is correct -
options.format === 'markdown'correctly branches at line 51 -
--excludeflag collected multi-value correctly (when not adjacent to another flag) -
excludeGlobspassed throughSurfaceInputSchema→analyze()→extractSurface()— complete - Both
walkFilescall sites inextractSurfacepassrootcorrectly (lines 511, 533) -
SurfaceInputSchema.excludeGlobsdefault[]—analyze()at line 690 passes it - Issue #1 above (flag-value theft)
- Issue #2 above (
--format markdownsilently degrades on other commands) - Issue #3 above (test coverage for
packages/**trailing-**pattern)
- Scope --format markdown to 'charter surface' only; other subcommands now reject it with a clear error instead of silently emitting text - Guard --exclude value consumption: skip args[i+1] if it starts with '--' to prevent adjacent flag names being consumed as glob patterns - Add test for trailing ** pattern (packages/**) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CHANGELOG, README, surface README, and CLI help text updated for: - charter surface --format markdown (issue #236) - charter surface --exclude <glob> (issue #237) - extractSurface() / SurfaceInputSchema excludeGlobs field Packages bumped 1.5.0 → 1.6.0 (minor: new additive features). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
charter surface --format markdownnow works; previously rejected withInvalid --format value: markdown. Addedmarkdownas a valid format, routing to the existingformatSurfaceMarkdownexport in@stackbilt/surface.charter surface --exclude <glob>now filters paths from the scan. Supports plain directory prefixes (packages/scaffold-core) and glob wildcards (**/codegen/**,src/templates/*). Prevents false-positive routes from scaffold template files when runningcharter surfaceinside the charter repo itself.Changes
packages/cli/src/index.ts—CLIOptions.formatextended to'text' | 'json' | 'markdown'; validation and help text updatedpackages/cli/src/commands/surface.ts— handlesoptions.format === 'markdown'; collects multiple--excludevaluespackages/surface/src/index.ts— addsmatchGlob()helper, updateswalkFiles()to acceptroot+excludeGlobs, extendsSurfaceInputSchemaandExtractOptionswithexcludeGlobspackages/surface/src/__tests__/surface.test.ts— 5 new regression tests covering schema default, plain path exclusion, and**wildcard exclusionTest plan
charter surface --format markdownproduces markdown output (no error)charter surface --format textand--format jsonunaffectedcharter surface --exclude packages/scaffold-coreskips that directorycharter surface --exclude "**/codegen/**"skips any nestedcodegen/directorypnpm exec vitest run)🤖 Generated with Claude Code