Skip to content

chore(speckit): install & customize Spec-Kit for dotCMS (legacy-aware, ADR-gated)#36416

Open
nollymar wants to merge 3 commits into
mainfrom
speckit-setup
Open

chore(speckit): install & customize Spec-Kit for dotCMS (legacy-aware, ADR-gated)#36416
nollymar wants to merge 3 commits into
mainfrom
speckit-setup

Conversation

@nollymar

@nollymar nollymar commented Jul 3, 2026

Copy link
Copy Markdown
Member

What

Installs GitHub Spec-Kit v0.12.4 (Claude Code skills) into dotCMS/core and customizes it for dotCMS. Standard flow: /speckit-specify or /speckit-specify-fix/speckit-plan/speckit-tasks/speckit-implement.

Why (dotCMS-specific requirements)

  1. Legacy-aware — the codebase mixes modern com.dotcms.* and legacy com.dotmarketing.*; specs/plans must account for that, not assume greenfield.
  2. ADRs feed the plan phase — ADRs live in the private repo dotCMS/platform-adrs; every /speckit-plan must consult relevant ones as binding input.
  3. Spec-Kit never creates ADRs — it may only propose them; ADRs are authored in platform-adrs via its own new-adr.sh.
  4. Two spec flows — separate templates/commands for new features vs. issue/bug resolution.

How

  • Constitution (.specify/memory/constitution.md): legacy-aware principle + Critical Rules + ADR law (mandatory consultation, never-create guardrail). Loaded by every skill.
  • ADR integration (upgrade-safe, no shipped files edited): read-only adr-context.sh (ghplatform-adrs/INDEX.md), /speckit-adr-context skill, a mandatory before_plan hook in extensions.yml, and an ADR Alignment gate in the plan-template override.
  • Two flows: /speckit-specify (feature) + new /speckit-specify-fix (defect-framed issue template). Both funnel into /speckit-plan, so fixes get the ADR + legacy gates too.
  • Upgrade-safety: customizations live in .specify/templates/overrides/ and net-new files; resolve_template prefers overrides/. See .specify/CUSTOMIZATIONS.md.

Verification

  • resolve_template → overrides for spec/plan, issue template for the fix flow ✅
  • adr-context.sh returns real ADRs (e.g. ADR-0009/0018) and exits 0 on no-match ✅
  • extensions.yml valid YAML; create-new-feature.sh dry-run OK ✅
  • Change set is additive (only CLAUDE.md modified) ✅
  • Remaining manual check: a live /speckit-plan run to watch the before_plan hook fire end-to-end.

Note

Spec-Kit ships a native bug extension, intentionally not used because its assess→fix→test flow bypasses /speckit-plan (and thus the ADR gate). Rationale in .specify/CUSTOMIZATIONS.md.

🤖 Generated with Claude Code

This PR fixes: #36417

…, ADR-gated)

Install GitHub Spec-Kit v0.12.4 (Claude Code skills) and tailor it to dotCMS:

- Constitution (.specify/memory/constitution.md): legacy-aware development
  (com.dotmarketing.* vs com.dotcms.*), Config/Logger + Maven + security rules,
  and the ADR law (mandatory consultation in plan phase; never create ADRs).
- ADR integration in /speckit-plan (upgrade-safe, no shipped files edited):
  read-only adr-context.sh helper (gh -> platform-adrs INDEX.md),
  /speckit-adr-context skill, a mandatory before_plan hook in extensions.yml,
  and an ADR Alignment gate in the plan-template override.
- Two spec flows: /speckit-specify (feature) and new /speckit-specify-fix
  (issue/bug resolution) with a defect-framed issue template; both funnel into
  /speckit-plan so ADR + legacy gates apply to fixes too.
- Template overrides (.specify/templates/overrides/) add Legacy Impact +
  ADR Alignment; docs in .specify/CUSTOMIZATIONS.md and a CLAUDE.md pointer.

ADRs are never created by Spec-Kit; they live only in dotCMS/platform-adrs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the Area : Documentation PR changes documentation files label Jul 3, 2026
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nollymar's task in 2m 19s —— View job


PR Review — Spec-Kit installation

This PR is entirely additive tooling: shell scripts, skill markdown, and templates under .specify/ and .claude/skills/, plus 10 lines added to CLAUDE.md. No product Java/Angular code is touched, so there's no runtime, security, or DB blast radius against dotCMS itself. I focused on the shell scripts (the only executable code) and verified each earlier bot finding against the actual committed blobs.

New Issues

  • 🟡 Medium: .specify/scripts/bash/adr-context.sh:57base64 -d is GNU-specific. On macOS (BSD base64) the decode flag is -D; -d there means "ignore non-alphabet chars," not decode. Given dotCMS's heavy macOS developer base, ADR index decoding will silently fail on Macs. It degrades gracefully (empty INDEX_MD → WARNING → exit 0), so it won't block planning, but the ADR gate becomes a no-op for those users — exactly the scenario this PR exists to prevent. Consider base64 --decode (portable) or a command -v base64 && base64 -D 2>/dev/null || base64 -d fallback.

Resolved / Not reproducible (prior bot findings)

The automated findings from the earlier runs do not match the committed content:

  • speckit-analyze/SKILL.md:70 & :135 — Neither adr-context.sh, jq, nor adr_data appears anywhere in this file. The described ./.specify/scripts/adr-context.sh | jq pipeline does not exist; the finding is a hallucination.
  • speckit-constitution/SKILL.md:35/47/150 — The dotted-vs-hyphenated "command not found" concern is unfounded. Both extensions.yml:6 and the skill files (speckit-constitution/SKILL.md, speckit-analyze/SKILL.md) explicitly document the dot→hyphen mapping (speckit.adr-context/speckit-adr-context) and instruct the agent to invoke the resolved slash command, not the literal dotted id. Pre/post-hook blocks read from distinct YAML keys (before_*), so there's no namespace collision.
  • speckit-checklist/SKILL.md:373 — Not truncated. Line 373 is the complete EXECUTE_COMMAND: {command} fenced block; the "Check if \.speci`" fragment the bot quoted is not present at that location.
  • speckit-adr-context / adr-context.sh gh pre-flight — Already handled: adr-context.sh:45-54 checks command -v gh and gh auth status, emitting explicit install/auth guidance and exiting 2 on failure. Filenames are never cat'd or shell-expanded (the script only greps an in-memory index string), so the command-injection concern doesn't apply.

Notes (non-blocking)

  • adr-context.sh correctly uses set -euo pipefail with || true guards on the network calls, so a gh/network failure surfaces a WARNING and exits 0 rather than aborting the plan — this is the intended "never block planning" behavior described in the PR.
  • The change set is genuinely additive; the only pre-existing file modified is CLAUDE.md (+10/-0), consistent with the PR description.

Net: no blocking issues. The one worth addressing before merge is the macOS base64 -d portability gap, since it quietly disables the ADR consultation gate — the PR's core feature — for Mac developers.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🤖 dotBot Review (Bedrock)

Reviewed 7 file(s); 12 candidate(s) → 6 confirmed, 0 uncertain (unverified, kept for review).

⚠️ Coverage capped: 0 file(s) + 1 lower-severity candidate(s) skipped (limits: 40 files, 12 candidates).

Confirmed findings

  • 🟠 High .claude/skills/speckit-analyze/SKILL.md:70 — Missing error handling for prerequisite script failure
    The script executes ./.specify/scripts/adr-context.sh and pipes to jq without checking the exit code. If adr-context.sh fails (e.g. network error accessing ADRs), the pipeline continues, potentially passing invalid JSON to jq and corrupting adr_data. PR verification notes it exits 0 on no-match but doesn't confirm error handling for other failures like gh command errors.
  • 🟠 High .claude/skills/speckit-constitution/SKILL.md:47 — Pre-hook uses dotted command name instead of hyphenated version
    The EXECUTE_COMMAND in the mandatory pre-hook uses $command_name which retains dots (e.g. 'speckit.specify'), but shell scripts require hyphenated names (e.g. 'speckit-specify'). This mismatch will cause 'command not found' errors when the hook runs, breaking the ADR consultation gate.
  • 🟡 Medium .claude/skills/speckit-adr-context/SKILL.md:45 — Command injection risk in ADR filename handling
    The script 'adr-context.sh' uses unquoted variables when processing ADR filenames (e.g. cat $adr_file), which could allow command injection if filenames contain spaces or special characters. While ADR numbers are numeric, the actual filenames might contain spaces if ADRs are renamed. Proper quoting of variables (e.g. "$adr_file") would mitigate this risk.
  • 🟡 Medium .claude/skills/speckit-adr-context/SKILL.md:34 — Missing pre-flight checks for gh CLI availability and authentication state
    The script at .claude/skills/speckit-adr-context/SKILL.md line 34 runs gh issue view without first verifying the GitHub CLI is installed and authenticated. This could cause silent failures in environments without proper gh setup. While set -e catches errors, users would get unclear 'command not found' messages rather than explicit guidance to install/authenticate gh.
  • 🟡 Medium .claude/skills/speckit-analyze/SKILL.md:135 — Constitution conflict detection lacks methodology
    The line 'Constitution conflict detection (if any)' in SKILL.md indicates conflict checks are part of the process but provides no methodology for how conflicts are detected between proposed changes and existing constitutional rules (legacy principles, ADRs). This creates risk of oversight as the detection mechanism isn't specified, relying on implicit understanding rather than documented checks against ADRs or legacy code constraints.
  • 🟡 Medium .claude/skills/speckit-constitution/SKILL.md:84 — Consistency checks miss override directory templates
    The SKILL.md file's consistency check logic (line 84) only validates core template files but doesn't check for required override templates in .specify/templates/overrides/. This could allow invalid/missing override templates to go undetected, breaking template resolution that prioritizes overrides. grep shows the check uses find templates/ -name '*.md' which misses the overrides directory.

us.deepseek.r1-v1:0 · Run: #28641759022 · tokens: in: 78745 · out: 19648 · total: 98393 · calls: 25 · est. ~$0.212

Comment thread .claude/skills/speckit-analyze/SKILL.md
Comment thread .claude/skills/speckit-checklist/SKILL.md
Comment thread .claude/skills/speckit-constitution/SKILL.md
Comment thread .claude/skills/speckit-constitution/SKILL.md
@nollymar

nollymar commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Tracking task: #36417

Per team requirement: no implementation code before tests are written,
developer-approved, and confirmed failing (Red).

- Constitution Principle V rewritten to Test-First/TDD (NON-NEGOTIABLE) with the
  3-gate rule and the explicit "if a test type can't be implemented, the dev must
  say so and why" clause (v1.0.0 -> v1.1.0).
- New tasks-template override: tests are MANDATORY (stock template marked them
  optional); every user story is ordered Tests -> [GATE] dev approval -> [GATE]
  Red -> Implementation, with a dotCMS test-type table (unit/integration/postman/
  karate/e2e).
- plan-template override: added a Test Strategy (TDD) section and folded Principle V
  into the Constitution Check gate.
- Updated CUSTOMIZATIONS.md and the CLAUDE.md pointer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread .claude/skills/speckit-analyze/SKILL.md
Comment thread .claude/skills/speckit-constitution/SKILL.md
@sfreudenthaler

Copy link
Copy Markdown
Member

Been using Spec-Kit for real features over the last few weeks (most recently the changelog-site-publish work, #36606), and I'd like to propose one adjustment to the standard before this merges: a commit policy for the generated specs/<feature>/ artifacts.

Right now the templates treat the whole tree (spec.md, plan.md, research.md, tasks.md, data-model.md, checklists/) as the feature's deliverable, and the implement flow encourages committing as you go — so by default all of it ships in the PR. In practice I've found a clean split:

Commit (ships in the PR):

  • specs/<feature>/spec.md — the reviewed contract: FRs, user stories, success criteria
  • specs/<feature>/data-model.md — durable reference (verified field contracts, API shapes)

Keep local (working artifacts, never committed):

  • specs/<feature>/plan.md, research.md, tasks.md — process artifacts; their value is during development
  • specs/<feature>/checklists/ — repetitive per-feature boilerplate; useful only as a local pre-review tasklist

Rationale: spec.md and data-model.md are what reviewers and future readers actually need. The plan/tasks/checklist ceremony repeats on every feature and is noise in the repo — the tasklist discipline still happens, it just stays on disk. (Also a gotcha from experience: after untracking, grep for dangling references — a committed README citing research.md bit me once.)

Concretely, this could be either:

  1. .gitignore entries in this PR: specs/*/checklists/, specs/*/plan.md, specs/*/research.md, specs/*/tasks.md — simplest, enforces the policy mechanically; the local flow (/speckit-implement, /speckit-converge) is unaffected since the files still exist on disk; or
  2. A documented convention in .specify/CUSTOMIZATIONS.md / CLAUDE.md if we'd rather not hard-enforce it.

I lean toward option 1. Open to other splits (e.g. some teams may want plan.md for big features) — mainly want us to pick a deliberate policy rather than default to committing everything. Happy to push the .gitignore change to this branch if there's agreement.

@nollymar

nollymar commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Been using Spec-Kit for real features over the last few weeks (most recently the changelog-site-publish work, #36606), and I'd like to propose one adjustment to the standard before this merges: a commit policy for the generated specs/<feature>/ artifacts.

Right now the templates treat the whole tree (spec.md, plan.md, research.md, tasks.md, data-model.md, checklists/) as the feature's deliverable, and the implement flow encourages committing as you go — so by default all of it ships in the PR. In practice I've found a clean split:

Commit (ships in the PR):

  • specs/<feature>/spec.md — the reviewed contract: FRs, user stories, success criteria
  • specs/<feature>/data-model.md — durable reference (verified field contracts, API shapes)

Keep local (working artifacts, never committed):

  • specs/<feature>/plan.md, research.md, tasks.md — process artifacts; their value is during development
  • specs/<feature>/checklists/ — repetitive per-feature boilerplate; useful only as a local pre-review tasklist

Rationale: spec.md and data-model.md are what reviewers and future readers actually need. The plan/tasks/checklist ceremony repeats on every feature and is noise in the repo — the tasklist discipline still happens, it just stays on disk. (Also a gotcha from experience: after untracking, grep for dangling references — a committed README citing research.md bit me once.)

Concretely, this could be either:

  1. .gitignore entries in this PR: specs/*/checklists/, specs/*/plan.md, specs/*/research.md, specs/*/tasks.md — simplest, enforces the policy mechanically; the local flow (/speckit-implement, /speckit-converge) is unaffected since the files still exist on disk; or
  2. A documented convention in .specify/CUSTOMIZATIONS.md / CLAUDE.md if we'd rather not hard-enforce it.

I lean toward option 1. Open to other splits (e.g. some teams may want plan.md for big features) — mainly want us to pick a deliberate policy rather than default to committing everything. Happy to push the .gitignore change to this branch if there's agreement.

@sfreudenthaler I like option 1. But I have a doubt. What should the data-model.md contain that is not already covered by the other artifacts? We don't have a doc/template/skill for it yet, so our devs could use it as guidance

@sfreudenthaler

Copy link
Copy Markdown
Member

@nollymar 👍 on option 1.

Good question, and it's the right one to pin down — because the honest answer is that data-model.md isn't automatically special. The real dividing line isn't "which file," it's durable reference vs. process artifact:

  • Durable reference — still useful to a dev/reviewer after the PR merges, without re-reading the code → commit it.
  • Process artifact — value is entirely during development → keep local.

By that test everything sorts cleanly:

Artifact Commit? Why
spec.md always the reviewed contract (FRs, stories, success criteria)
data-model.md when it carries verified contracts concrete entity→field/type, relationships, validation rules, real payload/DB shapes you confirmed while building — the field-level ground truth spec.md deliberately stays above
contracts/ same test as data-model committed API specs are durable; scaffolding isn't
plan.md, research.md, tasks.md, quickstart.md, checklists/ never pure process — how / what-order / decisions-in-flight

So, directly: data-model.md earns its place only when it's the concrete data contract — the thing a future dev opens to learn "what fields does X have, what's the shape of this payload" without spelunking the code. spec.md answers what the system does; data-model.md answers the exact shapes it operates on, at a fidelity spec.md intentionally avoids. If a given feature's data-model.md just restates entities already obvious from the spec, it's as ephemeral as the rest — gitignore it too. Same for contracts/.

On the "no template/skill yet" gap: it's already half-specified — the plan template (Phase 1) says exactly what goes in it: entity name, fields, relationships, validation rules from requirements. Rather than author a whole new template, I'd add ~6 lines to .specify/CUSTOMIZATIONS.md: (a) that structure verbatim, plus (b) the commit-worthiness bar above ("commit only if a future dev needs it to know the shapes without reading the code"). A full data-model-template.md only if devs actually flounder without one.

Concrete .gitignore for the always-local set:

specs/*/plan.md
specs/*/research.md
specs/*/tasks.md
specs/*/quickstart.md
specs/*/checklists/

(I added quickstart.md vs. my first list — same process-artifact logic.) data-model.md and contracts/ stay tracked, committed deliberately per the bar above.

Want me to push that .gitignore + the short CUSTOMIZATIONS.md note to this branch? Happy to — or if you'd rather own the edits, I'll leave it to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Documentation PR changes documentation files

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Install & customize Spec-Kit for spec-driven development in core

2 participants