Skip to content

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

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

chore(speckit): install & customize Spec-Kit for dotCMS (legacy-aware, ADR-gated)#36416
nollymar wants to merge 4 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 <[email protected]>
@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 @sfreudenthaler's task in 3m 13s —— View job


PR Review

I reviewed the latest commit (6ba4d18 — the spec-folder commit policy) plus the executable code (shell scripts) and re-checked the six prior dotBot findings against the committed content.

New Issues

None blocking. The commit-policy change (.gitignore + CUSTOMIZATIONS.md) is clean, additive, and internally consistent:

  • The .gitignore never-commit set (specs/*/plan.md, research.md, tasks.md, quickstart.md, checklists/) exactly matches the "Never" row of the CUSTOMIZATIONS.md table and the prose enumerations — no drift.
  • data-model.md and contracts/ are deliberately left tracked, consistent with the commit-worthiness bar. Good.
  • Patterns are one-level (specs/*/…), which matches the layout create-new-feature.sh actually produces ($REPO_ROOT/specs/$BRANCH_NAME/). ✅

🟡 Minor (non-blocking, doc-only): The comment in .gitignore and the CUSTOMIZATIONS.md table both point at "durable reference" for data-model.md/contracts/, but nothing enforces that they get removed when a given feature's data-model.md is just ephemeral restatement. That's inherent to a judgment-based policy — worth a one-line reminder in /speckit-implement output ("before committing, confirm data-model.md meets the bar"), but not a merge blocker.

Resolved / Not Reproduced (prior dotBot findings)

All six confirmed dotBot findings target .claude/skills/*/SKILL.md files. These are markdown instruction documents for the LLM, not executable shell — the finder misclassified template placeholders as running code. Verified against HEAD:

  • speckit-constitution/SKILL.md "missing speckit- prefix" / "dotted command name" — not a bug. The file explicitly documents the mapping: "replace dots (.) with hyphens (-)… speckit.git.commit/speckit-git-commit". EXECUTE_COMMAND: {command} is a literal template placeholder rendered by the skill, not a shell invocation.
  • speckit-analyze/SKILL.md:70 / :47 "missing error handling / jq pipe corruption" — no such code exists. There is no adr-context.sh | jq pipeline in that file (grep for jq/adr_data/adr-context.sh in the file returns nothing). The lines cited are prose about the hook contract.
  • speckit-adr-context/SKILL.md "command injection via cat $adr_file" — not present. No unquoted cat $adr_file anywhere in .specify/. The real script adr-context.sh runs set -euo pipefail, quotes its variables, and only does HTTP GET via gh api.
  • speckit-adr-context/SKILL.md:34 "missing gh preflight checks" — already handled in the actual script: command -v gh + gh auth status guards with explicit remediation messages (adr-context.sh:45-54).
  • speckit-analyze/SKILL.md:135 "conflict detection lacks methodology" and speckit-checklist/SKILL.md:373 "truncated line" — prose/quality observations about doc content, not defects; line 373 is the file's normal final content (376 lines total, no truncation).

Executable code — checked directly

The only runnable artifacts are .specify/scripts/bash/*.sh. Spot-checked the two with real logic:

  • adr-context.sh — strictly read-only, set -euo pipefail, quoted vars, gh preflight, exits 0 on no-match by design. Sound.
  • create-new-feature.sh — base-10 forcing (10#$…) guards against octal, 244-byte branch truncation, whitespace validation, portable acronym handling. Sound.

Verdict

The substantive commit in this push is documentation/config only and is correct. The prior dotBot findings do not reproduce against the committed content — they read Spec-Kit's markdown skill files as shell. No merge blockers from my pass.
· branch speckit-setup

@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 <[email protected]>
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

sfreudenthaler commented Jul 17, 2026

Copy link
Copy Markdown
Member

@nollymar 👍 on option 1.

Good question, honestly I'd be willing to drop it because in most cases I think you're right, data-model.md isn't automatically special, but I noticed when using speckit for the auto-release notes that my litmus test was durable reference vs. process artifact and instinctively added it:

  • 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

data-model.md earns its place only when it's the concrete data contract. If a given feature's data-model.md just restates entities already obvious from the spec, it's as ephemeral as the rest.

RE: "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, could we add to .specify/CUSTOMIZATIONS.md:

  1. that structure verbatim,
  2. the commit-worthiness bar above ("commit only if a future dev needs it to know the shapes without reading the code").

Concrete .gitignore for the always-local set:

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

Gitignore Spec-Kit process artifacts (plan.md, research.md, tasks.md,
quickstart.md, checklists/) so only durable references ship in PRs.
Document the durable-reference-vs-process-artifact rule and the
data-model.md commit-worthiness bar in CUSTOMIZATIONS.md.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_018pBayFCPNBw5s4veuUg7m5
@sfreudenthaler

Copy link
Copy Markdown
Member

Pushed the commit policy to this branch (6ba4d18384):

  • .gitignore — the always-local set: specs/*/plan.md, research.md, tasks.md, quickstart.md, checklists/. Files still land on disk, so /speckit-implement and /speckit-converge are unaffected.
  • .specify/CUSTOMIZATIONS.md — new "Spec-folder commit policy" section with the durable-reference-vs-process-artifact rule, the commit table, and the data-model.md commit-worthiness bar (structure pulled from the plan template's Phase 1 spec, so it doubles as the dev guidance you flagged was missing).

data-model.md and contracts/ stay tracked — committed deliberately per the bar, not by default. No git rm --cached needed since no specs/<feature>/ artifacts are tracked yet. Take a look and adjust wording if you'd like.

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