feat: generated INDEX, .claude/workflows scaffolding, orchestration docs - #95
Conversation
594d7f7 to
d857a04
Compare
25902c4 to
6849cb2
Compare
|
VERDICT: BLOCK |
|
VERDICT: BLOCK Blocking: Scope creep -- two unrelated features in one PR (CONTRIBUTING.md requires one logical change per PR). Feature 1 (commit 94c284b): feat: generate templates/INDEX.md and scaffold .claude/workflows/ -- adds render_template_index(), the --write-index flag, spec-fanout.js workflow, docs/04 additions, and --check rules for meta blocks and the generated index (configure.py:689-800). Feature 2 (commit 6849cb2): feat(hooks): PowerShell hook variants behind --hook-shell powershell -- adds POWERSHELL_INLINE_COMMANDS, powershell_hook_stems(), swap_hook_shell(), apply_hook_shell_to_settings(), powershell_hook_command(), --hook-shell CLI flag, ASCII/BOM enforcement in run_check(), README and docs/03 updates (configure.py:2265-2363). Both have their own CHANGELOG entries and separate commits. Neither depends on the other. Rolling either back would require untangling shared configure.py edits. Recommend splitting into two PRs. Each is already well-scoped and would PASS on its own -- the blocking issue is structural, not technical. Advisory: none. Both features are internally consistent and well-documented. |
6849cb2 to
94c284b
Compare
tigers1997
changed the title
|
Block accepted — the scope-creep call was right, and the PR body had already conceded the risk ("the PowerShell commit is self-contained and independently revertible if you'd rather take it separately"). Should have split it before asking for review rather than offering it as an option. Done, at the commit boundary you identified:
No rewriting was needed: both commits were already atomic and each carried its own CHANGELOG entry, so the split is a branch cut, not a re-authoring. One thing I checked rather than assumed, since a split can look clean and not be: @claude review |
|
VERDICT: COMMENT-ONLY Advisory configure.py:189 - import detection in --check misses mid-file static imports The workflow validation gate checks: if "import(" in text or text.lstrip().startswith("import "): The first clause catches dynamic imports anywhere in the file. The second clause only catches a static ES-module import at the very first line. A static import appearing after the meta block would pass --check silently but still be rejected by the runtime. The shipped spec-fanout.js has no imports so this is not a present defect, but a future template author could be confused. Consider re.search to catch all top-level static imports. configure.py:187 - meta field presence check uses the first closing brace as a delimiter This works for the shipped spec-fanout.js (name: and description: appear before the first } in the nested phases array). It would produce a false positive if a future workflow author placed a top-level meta key after the first nested object closes. Minor; a multiline regex or balanced-brace parse would be more robust. Both are nits; neither blocks. |
|
VERDICT: COMMENT-ONLY BlockingNone. Advisory1. Schema-claim hygiene —
Both are above the project's 2. Workflow The check: if field not in text.split("}", 1)[0]:splits on the first 3. Static if "import(" in text or text.lstrip().startswith("import "):The |
… line Two defects in rule 4b, both raised as advisory findings on #95. The meta block was located with text.split("}", 1)[0], which truncates at the first nested brace. `phases: [{ title, detail }]` is a legitimate meta field, so a workflow that wrote `phases:` ahead of `name:` would be reported as missing a field it actually had. Replaced with a brace matcher that skips string literals, so a brace inside a description no longer unbalances the scan either. The static-import guard was text.lstrip().startswith("import "), which only fires when `import` is the first token in the entire file. A mid-file `import x from "y"` passed the check and then failed at invocation time, which is exactly what this rule exists to prevent. Now matched at the start of any line, with the dynamic-import test tightened so it doesn't fire on an identifier that merely ends in "import". Adds test/schema-hygiene/test-workflow-meta-rule.sh -- the rule had no test at all. Covers both regressions, the string-brace case, a genuinely missing field, an unterminated literal, and asserts the shipped spec-fanout.js stays clean.
|
Both advisories were real bugs, not style nits. Fixed in
The rule had no test at all — that's the gap underneath both findings, and the more useful thing to close. Added Verified the fix by running the old and new predicates side by side over each case — the two regressions flip from wrong to right, the previously-working cases stay working, and |
|
VERDICT: COMMENT-ONLY All three changes are coherent and on-topic for the same audit batch. No correctness or license issues found. Two advisory notes below. BlockingNone. Advisory1. Missing SchemaStore citation for
"// workflowSizeGuideline": "medium",
"// teammateMode": "auto",CONTRIBUTING.md requires citing SchemaStore when adding new keys to settings templates. These stubs use the The compat survey entry already in 2. Confirm all persona snapshots that include Only |
d857a04 to
c0e614a
Compare
… line Two defects in rule 4b, both raised as advisory findings on #95. The meta block was located with text.split("}", 1)[0], which truncates at the first nested brace. `phases: [{ title, detail }]` is a legitimate meta field, so a workflow that wrote `phases:` ahead of `name:` would be reported as missing a field it actually had. Replaced with a brace matcher that skips string literals, so a brace inside a description no longer unbalances the scan either. The static-import guard was text.lstrip().startswith("import "), which only fires when `import` is the first token in the entire file. A mid-file `import x from "y"` passed the check and then failed at invocation time, which is exactly what this rule exists to prevent. Now matched at the start of any line, with the dynamic-import test tightened so it doesn't fire on an identifier that merely ends in "import". Adds test/schema-hygiene/test-workflow-meta-rule.sh -- the rule had no test at all. Covers both regressions, the string-brace case, a genuinely missing field, an unterminated literal, and asserts the shipped spec-fanout.js stays clean.
93aaa46 to
e3c6009
Compare
|
VERDICT: PASS Everything in scope is clean. Brief notes for the record: INDEX.md generation — spec-fanout.js — workflow check (rule 4b) — the import guard covers any line via the multiline flag, plus a negative-lookbehind dynamic-import guard; test fixtures assert both. The two brace-truncation regression cases (phases-before-name; brace inside string) are explicitly covered. Schema keys — AGPL / MIT — no third-party code added; CHANGELOG — Minor observation (not blocking): the |
c0e614a to
62db375
Compare
c0e614a to
d0a2ae8
Compare
… line Two defects in rule 4b, both raised as advisory findings on #95. The meta block was located with text.split("}", 1)[0], which truncates at the first nested brace. `phases: [{ title, detail }]` is a legitimate meta field, so a workflow that wrote `phases:` ahead of `name:` would be reported as missing a field it actually had. Replaced with a brace matcher that skips string literals, so a brace inside a description no longer unbalances the scan either. The static-import guard was text.lstrip().startswith("import "), which only fires when `import` is the first token in the entire file. A mid-file `import x from "y"` passed the check and then failed at invocation time, which is exactly what this rule exists to prevent. Now matched at the start of any line, with the dynamic-import test tightened so it doesn't fire on an identifier that merely ends in "import". Adds test/schema-hygiene/test-workflow-meta-rule.sh -- the rule had no test at all. Covers both regressions, the string-brace case, a genuinely missing field, an unterminated literal, and asserts the shipped spec-fanout.js stays clean.
e3c6009 to
23f2180
Compare
|
VERDICT: COMMENT-ONLY - Advisory findings only, nothing blocking. (1) ps1 routing residue in config_schema.py:48-51: adds .ps1 routing for microbit-enforcer but no .ps1 file ships in this PR, belonging to the stacked PowerShell PR -- dead code, no harm, soft-violates one-logical-change. (2) SchemaStore citation missing for workflowSizeGuideline and teammateMode stubs added to settings.local.json.example files -- risk low (comment-prefixed, example-only, cannot leak), but per established pattern a validation stamp note would close the loop before next tested_up_to bump. Everything else solid: _js_meta_block is brace-matched not split-on-first-brace, import guard covers mid-file statics and dynamic import(), new test covers both regression classes, persona snapshot updated, CHANGELOG entry present under Unreleased. No AGPL contamination; templates/discipline-skills/ untouched. |
d0a2ae8 to
a2b6c08
Compare
…ent teams and routines
Three gaps the currency audit left open.
1. templates/INDEX.md was hand-maintained and had gone stale enough to
mislead: it still referenced a configurator.html that no longer exists and
was missing half the modules. It is now generated from MODULES by
`python3 configure.py --write-index`, and --check fails when the committed
copy and the generator disagree, so it cannot drift again.
2. Dynamic workflows have been a first-class Claude Code surface since
2.1.154 and the configurator scaffolded nothing for them. The multi-agent
module now ships .claude/workflows/spec-fanout.js, which runs as
/spec-fanout: it generates N variants of one spec into disjoint slots, then
screens each variant against the spec and the diversification axis before
reporting. It is the workflow-native successor to the /infinite skill in
the same module -- same job, but the runtime holds the loop and the
intermediate results, the run is resumable, and the screening pass is a
real gate rather than a suggestion. Project workflows under
.claude/workflows/ are shared with everyone who clones the repo.
--check gained a rule validating that every shipped workflow declares a
usable `meta` block and uses no import(); the runtime rejects both, and a
broken workflow would otherwise fail at invocation time in the user's
project rather than at scaffold time here. workflowSizeGuideline is stubbed
in settings.local.json.example.
3. docs/04 gains a table comparing the five ways to run work in parallel
(subagent / skill / agent team / workflow / worktree session) by who holds
the plan and where intermediate results live -- and states plainly why the
configurator ships no templates for agent teams, channels or routines:
- Teams are spawned in conversation and live for a session, so there is
no project-level file that defines one. The only knob worth setting is
teammateMode, a per-machine terminal preference, now stubbed in
settings.local.
- The channel gate keys channelsEnabled and allowedChannelPlugins are
managed-settings only, so a project cannot enable them; an administrator
does. Once enabled a channel is an MCP server like any other.
- Routines are scheduled cloud agents that run against a repo rather than
from your checkout, so a committed template has nothing to bind to. A
Stop or SessionStart hook is the project-scoped equivalent, and the
git-workflow module already ships one.
The microbit-enforcer routing rule in target_path_for is generalized from
".sh" to ".sh"/".ps1" so the directory's scripts route to .claude/hooks/ by
kind rather than by extension.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVndNviHZSnbKJnWP7jFbV
… line Two defects in rule 4b, both raised as advisory findings on #95. The meta block was located with text.split("}", 1)[0], which truncates at the first nested brace. `phases: [{ title, detail }]` is a legitimate meta field, so a workflow that wrote `phases:` ahead of `name:` would be reported as missing a field it actually had. Replaced with a brace matcher that skips string literals, so a brace inside a description no longer unbalances the scan either. The static-import guard was text.lstrip().startswith("import "), which only fires when `import` is the first token in the entire file. A mid-file `import x from "y"` passed the check and then failed at invocation time, which is exactly what this rule exists to prevent. Now matched at the start of any line, with the dynamic-import test tightened so it doesn't fire on an identifier that merely ends in "import". Adds test/schema-hygiene/test-workflow-meta-rule.sh -- the rule had no test at all. Covers both regressions, the string-brace case, a genuinely missing field, an unterminated literal, and asserts the shipped spec-fanout.js stays clean.
23f2180 to
dfe8323
Compare
|
VERDICT: PASS Clean PR. All three sub-changes address one theme (gaps in parallelism-surface coverage left by the currency audit), the CHANGELOG entry is well-formed under Spot-checks:
|
|
VERDICT: COMMENT-ONLY |
|
VERDICT: COMMENT-ONLY Advisory config_schema.py:1104-1108 - PS1 routing forward-reference belongs in the stacked PR The change to target_path_for extending .endswith to include .ps1 is dead code in this PR. No .ps1 templates exist under templates/commands/microbit-enforcer/ in the current tree -- those were the PowerShell hooks explicitly split into the stacked PR. The change is harmless (it matches nothing, changes no behavior), but it is the one remnant of the scope that was split out. It would be cleaner to carry this 2-line routing prep in the stacked PowerShell PR alongside the templates it serves. Everything else is clean: CHANGELOG entry present, no AGPL/license issues, discipline-skills/ untouched, persona snapshots correctly regenerated, the _js_meta_block brace-matcher handles nested objects and string-embedded braces correctly (verified by the new test fixtures in test/schema-hygiene/test-workflow-meta-rule.sh), the import() guard covers mid-file occurrences, and workflowSizeGuideline/teammateMode are confined to .local.json.example where the //-stub pattern is safe. |
1 participant
What & why
Closes three of the four gaps the currency audit left open. All three are the same kind of gap — a Claude Code surface the configurator had not caught up to.
1.
templates/INDEX.mdis now generated. It was hand-maintained and had gone stale enough to mislead — still referencing aconfigurator.htmlthat no longer exists, and missing half the modules. Produced fromMODULESbypython3 configure.py --write-index, with--checkfailing when the committed copy and the generator disagree. It cannot drift again.2.
.claude/workflows/scaffolding. Dynamic workflows have been a first-class surface since CC 2.1.154 and the configurator shipped nothing for them. Themulti-agentmodule now scaffolds.claude/workflows/spec-fanout.js(runs as/spec-fanout): N variants of one spec into disjoint slots, then each variant screened against the spec before reporting.It is deliberately not a review workflow —
/code-reviewalready does multi-agent review, and shipping our own would repeat the overclaiming the previous PR just fixed. It is instead the workflow-native successor to the/infiniteskill in the same module, which is exactly where that PR told users to go.--checkgained a rule validating every shipped workflow'smetablock and rejectingimport(), both of which the runtime refuses.3. Agent teams, channels and routines — documented, not templated.
docs/04gains a table comparing the five ways to run work in parallel by who holds the plan, and states plainly why there's nothing to scaffold: teams are spawned in conversation and live for a session; the channel gate keyschannelsEnabledandallowedChannelPluginsare managed-settings only, so a project cannot enable them; routines are scheduled cloud agents running against a repo rather than your checkout, where aStophook is the project-scoped equivalent. OnlyteammateModewas worth a stub.Type of change
Scope
multi-agent,coresmall-team(gains.claude/workflows/spec-fanout.js); snapshots regenerated.Tests
python3 configure.py --checkpasses locally — verified on this commit alone, not just at the top of the stack, so the split is genuinely clean.CHANGELOG
## Unreleased.License & NOTICE
templates/discipline-skills/untouched by this PR.LICENSE/NOTICEuntouched.Signing
I understand
CONTRIBUTING.md.Fourth of five stacked PRs — based on
fix/currency-corrections, withfeat/powershell-hooksstacked on top.