feat: Adding architectural decision records suggested changes #566
Conversation
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
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds first-class support for Architectural Decision Records (ADRs): a two-file ADR format (decision.md + adr.md), parallel adrs/ directory and delta workflows, template resolution and config, CLI commands (list/show/validate/archive) extended for ADRs, AI/instruction-loader context integration, migration plan, and proactive ADR detection. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant CLI as "openspec CLI"
participant FS as "Repository (specs/, adrs/, templates/)"
participant Instr as "Instruction-loader / AI"
participant Repo as "Change Processor / Archive"
rect rgba(200,230,255,0.5)
User->>CLI: run command (list/show/validate/archive)
CLI->>FS: read adrs/ and openspec/config.yaml
CLI->>Instr: request ADR context (decision.md)
Instr->>FS: load decision.md (on-demand adr.md)
end
rect rgba(200,255,200,0.5)
CLI->>Repo: apply change deltas (RENAMED→REMOVED→MODIFIED→ADDED)
Repo->>FS: create/update/archive adrs/[decision]/{decision.md,adr.md}
Repo-->>CLI: report result / errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Greptile SummaryThis PR adds comprehensive Architectural Decision Records (ADRs) support to OpenSpec, introducing a two-file structure ( Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Parser
participant Validator
participant FileSystem
participant AIAgent
Note over User,AIAgent: Creating ADR Change Proposal
User->>CLI: openspec create change add-auth-decision
CLI->>FileSystem: Create changes/add-auth-decision/
CLI->>FileSystem: Create adrs/auth-strategy/decision.md
CLI->>FileSystem: Create adrs/auth-strategy/adr.md
FileSystem-->>User: ADR scaffold created
Note over User,AIAgent: Validating ADR Change
User->>CLI: openspec validate add-auth-decision --strict
CLI->>Parser: Parse ADR deltas
Parser->>Parser: Extract ADDED/MODIFIED/REMOVED Decisions
Parser-->>CLI: Parsed deltas
CLI->>Validator: Validate ADR format
Validator->>Validator: Check required sections (Context, Options, Decision Rationale, Consequences)
Validator->>Validator: Verify no conflicts with existing ADRs
Validator-->>CLI: Validation results
CLI-->>User: Validation passed
Note over User,AIAgent: Archiving ADR Change
User->>CLI: openspec archive add-auth-decision
CLI->>Validator: Pre-archive validation
Validator-->>CLI: Valid
CLI->>Parser: Process ADR deltas in order
Parser->>FileSystem: Apply RENAMED decisions
Parser->>FileSystem: Apply REMOVED decisions
Parser->>FileSystem: Apply MODIFIED decisions (update files)
Parser->>FileSystem: Apply ADDED decisions (create directories and both files)
FileSystem-->>CLI: ADRs updated
CLI->>FileSystem: Move change to archive/
CLI-->>User: Archive complete
Note over User,AIAgent: AI Agent Using ADRs
AIAgent->>CLI: Request project context
CLI->>FileSystem: Load all decision.md files (~200 tokens each)
FileSystem-->>CLI: ADR summaries
CLI-->>AIAgent: Context with ADR summaries
AIAgent->>AIAgent: Create proposal referencing relevant ADRs
AIAgent->>CLI: Need full rationale for auth-strategy
CLI->>FileSystem: Load adrs/auth-strategy/adr.md
FileSystem-->>CLI: Full ADR content
CLI-->>AIAgent: Full rationale provided
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #### Scenario: Creating change with ADR deltas | ||
|
|
||
| - **WHEN** creating a change proposal that includes architectural decisions | ||
| - **THEN** create ADR delta files under `openspec/changes/[change-name]/adrs/[decision-name]/adr.md` |
There was a problem hiding this comment.
syntax: Path should reference both files in the ADR, not just adr.md. The two-file structure requires both decision.md and adr.md.
Suggested change
| - **THEN** create ADR delta files under `openspec/changes/[change-name]/adrs/[decision-name]/adr.md` | |
| - **THEN** create ADR delta files under `openspec/changes/[change-name]/adrs/[decision-name]/` with both `decision.md` and `adr.md` |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openspec/changes/add-architectural-decision-records/specs/change-creation/spec.md
Line: 10:10
Comment:
**syntax:** Path should reference both files in the ADR, not just `adr.md`. The two-file structure requires both `decision.md` and `adr.md`.
```suggestion
- **THEN** create ADR delta files under `openspec/changes/[change-name]/adrs/[decision-name]/` with both `decision.md` and `adr.md`
```
How can I resolve this? If you propose a fix, please make it concise.| #### Scenario: Validating ADR format | ||
|
|
||
| - **WHEN** validating an ADR delta | ||
| - **THEN** the command SHALL check for required sections: Context, Decision, Options Considered, Consequences |
There was a problem hiding this comment.
syntax: Listed section name Decision doesn't match the actual adr.md format which uses Decision Rationale (per adr-management/spec.md:56 and openspec-conventions/spec.md:149).
Suggested change
| - **THEN** the command SHALL check for required sections: Context, Decision, Options Considered, Consequences | |
| - **THEN** the command SHALL check for required sections: Context, Options Considered, Decision Rationale, Consequences |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openspec/changes/add-architectural-decision-records/specs/cli-validate/spec.md
Line: 18:18
Comment:
**syntax:** Listed section name `Decision` doesn't match the actual `adr.md` format which uses `Decision Rationale` (per `adr-management/spec.md:56` and `openspec-conventions/spec.md:149`).
```suggestion
- **THEN** the command SHALL check for required sections: Context, Options Considered, Decision Rationale, Consequences
```
How can I resolve this? If you propose a fix, please make it concise.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@openspec/changes/add-architectural-decision-records/design.md`:
- Around line 36-171: The document has conflicting guidance about ADR file
layout and the Status field: Decision 1 describes a single-file adr.md, while
Decision 4 mandates a two-file layout (decision.md + adr.md) and includes a
Status field that Open Question `#1` argues against; reconcile by choosing one
canonical layout and updating all sections to match it (either adopt the
two-file approach and add the Status field everywhere and remove the single-file
references in Decision 1 and Open Question `#1`, or adopt the single-file approach
and remove decision.md/Status references in Decision 4), and ensure the
examples, directory listing (e.g., adrs/database-strategy/), and template paths
(templates.adr) consistently reflect that choice across Decision 1, Decision 4,
Open Question `#1`, and lines ~280-285.
In
`@openspec/changes/add-architectural-decision-records/specs/cli-validate/spec.md`:
- Around line 58-63: The scenario "Validating standalone ADR" introduces an
undocumented CLI pattern by using `openspec validate [decision-name] --type
adr`; update this by either adding the `--type adr` validate pattern to the CLI
Commands section in proposal.md (documenting `openspec validate [decision-name]
--type adr` and its behavior to validate files under `openspec/adrs/`) or change
the scenario to match the existing documented validate behavior (use `openspec
validate [change] --strict` to validate ADR deltas within changes); refer to the
scenario title "Validating standalone ADR", the command examples `openspec
validate [decision-name] --type adr`, `openspec validate [change] --strict`, and
`openspec show [adr-name] --type adr` when making the edit so the spec and
proposal.md remain consistent.
🧹 Nitpick comments (5)
openspec/changes/add-architectural-decision-records/specs/cli-show/spec.md (1)
7-47: Align scenario step formatting with spec conventions.Spec guidance requires
#### Scenario:headers with WHEN/THEN step lines (no bullets or bold) and avoids AND-only steps. Please reformat these scenarios accordingly to keep the delta spec consistent. As per coding guidelines, use WHEN/THEN structure without bullets/bold.openspec/changes/add-architectural-decision-records/specs/change-creation/spec.md (1)
7-77: Reformat scenario steps to the required WHEN/THEN style.Delta spec files should avoid bullets/bold and use plain WHEN/THEN lines under
#### Scenario:headers. Please convert the bulleted WHEN/THEN/AND steps accordingly. As per coding guidelines, avoid bullets/bold in scenarios.openspec/changes/add-architectural-decision-records/specs/openspec-conventions/spec.md (2)
7-179: Scenario steps need the required WHEN/THEN plain-line format.This spec uses bulleted WHEN/THEN/AND steps; spec conventions require plain WHEN/THEN lines under
#### Scenario:and no bullets/bold. Please reformat throughout. As per coding guidelines, use WHEN/THEN structure without bullets/bold.
11-40: Add a language tag to the fenced code block.Markdownlint flags the project structure block because it lacks a language identifier. Use
text (ormarkdown) to satisfy MD040.openspec/changes/add-architectural-decision-records/specs/adr-management/spec.md (1)
7-283: Normalize scenario steps to the required WHEN/THEN format.This spec uses bulleted WHEN/THEN/AND steps. Spec conventions require plain WHEN/THEN lines (no bullets/bold) under
#### Scenario:headers. Please reformat to comply. As per coding guidelines, use WHEN/THEN structure without bullets/bold.
Comment on lines
+36
to
+171
| **What**: Create `adrs/` directory at the same level as `specs/`, with similar structure: | ||
| ``` | ||
| openspec/ | ||
| ├── specs/ # Functional specifications | ||
| │ └── [capability]/ | ||
| │ └── spec.md | ||
| ├── adrs/ # Architectural decisions | ||
| │ └── [decision-name]/ | ||
| │ └── adr.md | ||
| └── changes/ | ||
| └── [change-name]/ | ||
| ├── specs/ # Spec deltas | ||
| └── adrs/ # ADR deltas | ||
| ``` | ||
|
|
||
| **Why**: | ||
| - **Cross-cutting nature**: ADRs affect multiple specs, so they can't be nested within a single spec | ||
| - Example: "Use DynamoDB for non-SQL storage" affects user-auth, session-management, analytics, etc. | ||
| - Placing ADRs at the top level (parallel to specs) reflects their one-to-many relationship | ||
| - Clear separation of concerns (functional vs architectural) | ||
| - Familiar structure for users already using specs | ||
| - Easy to understand which documents serve which purpose | ||
| - Each ADR declares its scope (which specs it affects) rather than being owned by one spec | ||
|
|
||
| **Alternatives considered**: | ||
| - ADRs nested within specs → Rejected: ADRs are cross-cutting; they don't belong to one spec | ||
| - Single unified document type with metadata tags → Rejected: Too complex, loses clarity | ||
| - ADRs as metadata within specs → Rejected: Pollutes functional specs with architectural details | ||
| - Separate repo for ADRs → Rejected: Breaks cohesion, harder to keep in sync | ||
| - Multiple ADR copies (one per affected spec) → Rejected: Duplication, inconsistency risk | ||
|
|
||
| ### Decision 2: Delta-Based Change Workflow for ADRs | ||
|
|
||
| **What**: ADR changes follow the same pattern as spec changes: | ||
| ```markdown | ||
| ## ADDED Decisions | ||
| ### Decision: Technology Choice | ||
| ... | ||
|
|
||
| ## MODIFIED Decisions | ||
| ### Decision: Existing Decision | ||
| ... | ||
|
|
||
| ## REMOVED Decisions | ||
| ### Decision: Deprecated Pattern | ||
| ... | ||
| ``` | ||
|
|
||
| **Why**: | ||
| - Consistency with existing workflow reduces cognitive load | ||
| - Reuse existing validation and archiving infrastructure | ||
| - AI agents already understand delta format | ||
|
|
||
| **Alternatives considered**: | ||
| - Immutable ADRs (new ADR supersedes old) → Rejected: Doesn't match OpenSpec's delta approach | ||
| - Freeform ADR updates without deltas → Rejected: Loses traceability and validation | ||
|
|
||
| ### Decision 3: Template System | ||
|
|
||
| **What**: Three-tier template resolution: | ||
| 1. Project-specific template: `openspec/templates/adr.md` | ||
| 2. Global config template (future): `~/.openspec/templates/adr.md` | ||
| 3. Built-in default template: `src/core/templates/adr.md` | ||
|
|
||
| Template configuration in `openspec/config.yaml`: | ||
| ```yaml | ||
| templates: | ||
| adr: ./templates/adr.md # Optional override | ||
| ``` | ||
|
|
||
| **Why**: | ||
| - Projects have different ADR needs (microservices vs monolith, security-focused vs startup) | ||
| - Default template provides good starting point | ||
| - Gradual adoption - templates are optional | ||
|
|
||
| **Alternatives considered**: | ||
| - No templates, freeform ADRs → Rejected: Loses structure, harder to validate | ||
| - Only built-in template → Rejected: Inflexible for diverse projects | ||
| - Multiple template types → Rejected: Over-engineering for v1 | ||
|
|
||
| ### Decision 4: Two-File ADR Structure | ||
|
|
||
| **What**: Split ADRs into two files for context efficiency: | ||
|
|
||
| **File 1: decision.md** (Concise summary, always loaded) | ||
| ```markdown | ||
| # Decision: [Name] | ||
|
|
||
| **Status:** Accepted | Proposed | Superseded | ||
|
|
||
| **Scope:** [Pattern-based description, not exhaustive list] | ||
| Examples: "All capabilities requiring non-relational data storage" | ||
|
|
||
| **What:** [One-sentence summary of the decision] | ||
|
|
||
| **Key Trade-offs:** | ||
| - ✓ Managed service, scales automatically | ||
| - ✓ Proven at scale | ||
| - ✗ Vendor lock-in | ||
| - ✗ Higher cost at low scale | ||
|
|
||
| **See:** [Full ADR](./adr.md) for detailed context and analysis. | ||
| ``` | ||
|
|
||
| **File 2: adr.md** (Full rationale, loaded on-demand) | ||
| ```markdown | ||
| # ADR: [Decision Name] | ||
|
|
||
| ## Context | ||
| [Detailed background, problem statement, constraints, stakeholders] | ||
|
|
||
| ## Options Considered | ||
| ### Option 1: [Name] | ||
| - Pros: [Comprehensive list] | ||
| - Cons: [Comprehensive list] | ||
| - Cost analysis: [Details] | ||
|
|
||
| ### Option 2: [Name] | ||
| [Full analysis...] | ||
|
|
||
| ## Decision Rationale | ||
| [Detailed explanation of why this option was chosen] | ||
|
|
||
| ## Consequences | ||
| [Detailed impacts, migration considerations, rollback strategy] | ||
|
|
||
| ## References | ||
| - [Related specs, ADRs, external resources] | ||
| ``` | ||
|
|
||
| **Directory structure:** | ||
| ``` | ||
| adrs/database-strategy/ | ||
| ├── decision.md # ~200 tokens - always in context | ||
| └── adr.md # ~800-1500 tokens - reference/on-demand | ||
| ``` |
Contributor
There was a problem hiding this comment.
Resolve internal inconsistencies on ADR file layout and status fields.
Decision 1’s structure shows only adr.md, while Decision 4 requires both decision.md and adr.md. Also, Open Question #1 leans against status fields even though Decision 4 mandates Status in decision.md. Please reconcile these to avoid conflicting guidance.
Also applies to: 280-285
🤖 Prompt for AI Agents
In `@openspec/changes/add-architectural-decision-records/design.md` around lines
36 - 171, The document has conflicting guidance about ADR file layout and the
Status field: Decision 1 describes a single-file adr.md, while Decision 4
mandates a two-file layout (decision.md + adr.md) and includes a Status field
that Open Question `#1` argues against; reconcile by choosing one canonical layout
and updating all sections to match it (either adopt the two-file approach and
add the Status field everywhere and remove the single-file references in
Decision 1 and Open Question `#1`, or adopt the single-file approach and remove
decision.md/Status references in Decision 4), and ensure the examples, directory
listing (e.g., adrs/database-strategy/), and template paths (templates.adr)
consistently reflect that choice across Decision 1, Decision 4, Open Question
`#1`, and lines ~280-285.
Comment on lines
+58
to
+63
| #### Scenario: Validating standalone ADR | ||
|
|
||
| - **WHEN** running `openspec validate [decision-name] --type adr` | ||
| - **THEN** the command SHALL validate the ADR file directly in `openspec/adrs/` | ||
| - **AND** check for required sections and proper format | ||
| - **AND** report any format or structural issues |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, read the spec file to understand the requirement structure and context
echo "=== Reading spec file structure ==="
cat -n openspec/changes/add-architectural-decision-records/specs/cli-validate/spec.mdRepository: Fission-AI/OpenSpec
Length of output: 3748
🏁 Script executed:
#!/bin/bash
# Check proposal.md for CLI command documentation
echo "=== Checking proposal.md for CLI documentation ==="
cat -n openspec/changes/add-architectural-decision-records/proposal.mdRepository: Fission-AI/OpenSpec
Length of output: 6547
🏁 Script executed:
#!/bin/bash
# Search for existing validate command patterns in spec files
echo "=== Searching for validate command patterns ==="
rg -n 'validate' openspec/changes/add-architectural-decision-records/specs --glob '*.md' -B2 -A2Repository: Fission-AI/OpenSpec
Length of output: 10859
The "Validating standalone ADR" scenario uses an undocumented CLI pattern.
The scenario requires openspec validate [decision-name] --type adr to validate ADRs directly in openspec/adrs/, but this pattern is not documented in proposal.md. The proposal documents:
openspec validate [change] --strictfor validating ADR deltas within changes (line 47)openspec show [adr-name] --type adrfor showing ADRs (lines 45-46)
The --type adr flag is only documented for the show command, not validate. Either add this pattern to proposal.md's CLI Commands section, or modify the scenario to align with the documented validate behavior (change-scoped only).
🤖 Prompt for AI Agents
In
`@openspec/changes/add-architectural-decision-records/specs/cli-validate/spec.md`
around lines 58 - 63, The scenario "Validating standalone ADR" introduces an
undocumented CLI pattern by using `openspec validate [decision-name] --type
adr`; update this by either adding the `--type adr` validate pattern to the CLI
Commands section in proposal.md (documenting `openspec validate [decision-name]
--type adr` and its behavior to validate files under `openspec/adrs/`) or change
the scenario to match the existing documented validate behavior (use `openspec
validate [change] --strict` to validate ADR deltas within changes); refer to the
scenario title "Validating standalone ADR", the command examples `openspec
validate [decision-name] --type adr`, `openspec validate [change] --strict`, and
`openspec show [adr-name] --type adr` when making the edit so the spec and
proposal.md remain consistent.
…ompt for ADR creation during SPEC workflow
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@openspec/changes/add-architectural-decision-records/design.md`:
- Around line 37-49: The markdown code fence showing the directory tree (the
triple-backtick block containing openspec/, specs/, adrs/, changes/) is missing
a language tag and triggers markdownlint MD040; update that fenced code block
and all other fenced blocks referenced (lines ~70-82, 101-105, 121-165, 167-171,
206-211, 229-244) to include an appropriate language identifier (e.g., use
"text" for plain trees, "markdown" for markdown snippets, or "yaml" where
applicable) so each triple-backtick block becomes ```text or ```markdown or
```yaml as appropriate.
In `@openspec/changes/add-architectural-decision-records/proposal.md`:
- Around line 97-113: The fenced code block in proposal.md is missing a language
tag causing markdownlint MD040; update the triple-backtick fence that wraps the
example block (the block starting with 'User: "Create a login feature with
session storage"') to include a language identifier (e.g., change ``` to
```text) so the block is properly recognized by markdownlint and renders
consistently.
In
`@openspec/changes/add-architectural-decision-records/specs/docs-agent-instructions/spec.md`:
- Around line 57-69: The fenced code block containing the ADR checklist is
missing a language tag which triggers markdownlint MD040; update the
triple-backtick fence in specs/docs-agent-instructions/spec.md to include a
language tag (e.g., change "```" to "```text") so the block is explicitly marked
as plaintext; ensure only the opening fence is changed and no content inside the
checklist is altered.
In
`@openspec/changes/add-architectural-decision-records/specs/instruction-loader/spec.md`:
- Around line 17-20: Update the scenario line wording to be clearer by changing
“an AI agent needs detailed rationale for a specific ADR” to a tighter phrasing
such as “an AI agent needs a detailed rationale for a specific ADR” or “an AI
agent needs detailed rationale to be loaded for a specific ADR”; modify the line
in the spec block that currently starts with “**WHEN** an AI agent needs
detailed rationale for a specific ADR” to use one of these suggested
alternatives so the intent reads naturally.
♻️ Duplicate comments (1)
openspec/changes/add-architectural-decision-records/specs/change-creation/spec.md (1)
9-11: Align path with the two-file ADR structure.Line 10 mentions only
adr.md; the ADR format requires bothdecision.mdandadr.md.
Comment on lines
+37
to
+49
| ``` | ||
| openspec/ | ||
| ├── specs/ # Functional specifications | ||
| │ └── [capability]/ | ||
| │ └── spec.md | ||
| ├── adrs/ # Architectural decisions | ||
| │ └── [decision-name]/ | ||
| │ └── adr.md | ||
| └── changes/ | ||
| └── [change-name]/ | ||
| ├── specs/ # Spec deltas | ||
| └── adrs/ # ADR deltas | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language tags to fenced code blocks.
markdownlint MD040 flags these blocks; add a language like text, markdown, or yaml as appropriate.
Also applies to: 70-82, 101-105, 121-165, 167-171, 206-211, 229-244
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@openspec/changes/add-architectural-decision-records/design.md` around lines
37 - 49, The markdown code fence showing the directory tree (the triple-backtick
block containing openspec/, specs/, adrs/, changes/) is missing a language tag
and triggers markdownlint MD040; update that fenced code block and all other
fenced blocks referenced (lines ~70-82, 101-105, 121-165, 167-171, 206-211,
229-244) to include an appropriate language identifier (e.g., use "text" for
plain trees, "markdown" for markdown snippets, or "yaml" where applicable) so
each triple-backtick block becomes ```text or ```markdown or ```yaml as
appropriate.
Comment on lines
+97
to
+113
| ``` | ||
| User: "Create a login feature with session storage" | ||
|
|
||
| AI detects: | ||
| - "Login feature" → Functional spec needed | ||
| - "Session storage" → Architectural decision (database choice) | ||
|
|
||
| AI checks: | ||
| - Is there an ADR for data storage? No → Create one | ||
|
|
||
| AI creates change with BOTH: | ||
| ├── adrs/database-strategy/ # NEW ADR for database choice | ||
| │ ├── decision.md | ||
| │ └── adr.md | ||
| └── specs/user-auth/ # Spec references the ADR | ||
| └── spec.md | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced block.
markdownlint MD040 flags missing language.
✅ Proposed fix
- ```
+ ```text
User: "Create a login feature with session storage"
AI detects:
- "Login feature" → Functional spec needed
- "Session storage" → Architectural decision (database choice)
AI checks:
- Is there an ADR for data storage? No → Create one
AI creates change with BOTH:
├── adrs/database-strategy/ # NEW ADR for database choice
│ ├── decision.md
│ └── adr.md
└── specs/user-auth/ # Spec references the ADR
└── spec.md
```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| User: "Create a login feature with session storage" | |
| AI detects: | |
| - "Login feature" → Functional spec needed | |
| - "Session storage" → Architectural decision (database choice) | |
| AI checks: | |
| - Is there an ADR for data storage? No → Create one | |
| AI creates change with BOTH: | |
| ├── adrs/database-strategy/ # NEW ADR for database choice | |
| │ ├── decision.md | |
| │ └── adr.md | |
| └── specs/user-auth/ # Spec references the ADR | |
| └── spec.md | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
97-97: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@openspec/changes/add-architectural-decision-records/proposal.md` around lines
97 - 113, The fenced code block in proposal.md is missing a language tag causing
markdownlint MD040; update the triple-backtick fence that wraps the example
block (the block starting with 'User: "Create a login feature with session
storage"') to include a language identifier (e.g., change ``` to ```text) so the
block is properly recognized by markdownlint and renders consistently.
Comment on lines
+57
to
+69
| ``` | ||
| Does this request involve: | ||
| ☐ Technology selection? (database, framework, library) | ||
| ☐ Infrastructure choice? (hosting, deployment, CI/CD) | ||
| ☐ Security approach? (auth method, encryption) | ||
| ☐ Cross-cutting concern? (logging, monitoring, error handling) | ||
| ☐ Decision affecting multiple specs? | ||
|
|
||
| If YES to any → Check for existing ADR | ||
| - ADR exists and applies → Reference it | ||
| - ADR exists but needs update → Modify it | ||
| - No ADR exists → Create new ADR in same change | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced block.
markdownlint MD040 flags missing language.
✅ Proposed fix
- ```
+ ```text
Does this request involve:
☐ Technology selection? (database, framework, library)
☐ Infrastructure choice? (hosting, deployment, CI/CD)
☐ Security approach? (auth method, encryption)
☐ Cross-cutting concern? (logging, monitoring, error handling)
☐ Decision affecting multiple specs?
If YES to any → Check for existing ADR
- ADR exists and applies → Reference it
- ADR exists but needs update → Modify it
- No ADR exists → Create new ADR in same change</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In
`@openspec/changes/add-architectural-decision-records/specs/docs-agent-instructions/spec.md`
around lines 57 - 69, The fenced code block containing the ADR checklist is
missing a language tag which triggers markdownlint MD040; update the
triple-backtick fence in specs/docs-agent-instructions/spec.md to include a
language tag (e.g., change "```" to "```text") so the block is explicitly marked
as plaintext; ensure only the opening fence is changed and no content inside the
checklist is altered.
Comment on lines
+17
to
+20
| - **WHEN** an AI agent needs detailed rationale for a specific ADR | ||
| - **THEN** the instruction loader MAY load the full adr.md file | ||
| - **AND** provide complete context, options analysis, and consequences | ||
| - **AND** load only when explicitly needed (e.g., when modifying an ADR or resolving conflicts) |
Contributor
There was a problem hiding this comment.
Tighten wording in the scenario line.
“needs detailed rationale” reads awkwardly; consider “needs detailed rationale to be loaded” or “needs a detailed rationale.”
🧰 Tools
🪛 LanguageTool
[style] ~17-~17: The double modal “needs detailed” is nonstandard (only accepted in certain dialects). Consider “to be detailed”.
Context: ...on demand - WHEN an AI agent needs detailed rationale for a specific ADR - THEN...
(NEEDS_FIXED)
🤖 Prompt for AI Agents
In
`@openspec/changes/add-architectural-decision-records/specs/instruction-loader/spec.md`
around lines 17 - 20, Update the scenario line wording to be clearer by changing
“an AI agent needs detailed rationale for a specific ADR” to a tighter phrasing
such as “an AI agent needs a detailed rationale for a specific ADR” or “an AI
agent needs detailed rationale to be loaded for a specific ADR”; modify the line
in the spec block that currently starts with “**WHEN** an AI agent needs
detailed rationale for a specific ADR” to use one of these suggested
alternatives so the intent reads naturally.
Contributor
|
Have addressed this proposal in here: #557 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Changes required to support architectural decision records that would help users manage decisions that cross multiple specs.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.