Skip to content

Add lightweight ADR process with Undecided state and CI linting - #39

Merged
ralphbean merged 9 commits into
mainfrom
agent-adopt-lightweight-adrs
Mar 20, 2026
Merged

Add lightweight ADR process with Undecided state and CI linting#39
ralphbean merged 9 commits into
mainfrom
agent-adopt-lightweight-adrs

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Introduce Architecture Decision Records to crystallize decisions as problem-space exploration matures. Key features:

  • ADR template supporting Proposed, Undecided, Accepted, Deprecated, and Superseded statuses. The Undecided state allows merging ADRs that frame a decision and its options before consensus forms.
  • ADR 0001 records the decision to adopt ADRs.
  • Linting scripts adapted from konflux-ci/architecture to validate ADR statuses and check for duplicate numbering.
  • Makefile with lint target for local validation.

Note: .github/workflows/lint.yml is included as a separate commit since pushing workflow files requires the 'workflow' PAT scope.

Assisted-by: OpenCode claude-opus-4-6@default

OpenCode Agent added 2 commits March 20, 2026 14:46
Introduce Architecture Decision Records to crystallize decisions as
problem-space exploration matures. Key features:

- ADR template supporting Proposed, Undecided, Accepted, Deprecated, and
  Superseded statuses. The Undecided state allows merging ADRs that frame
  a decision and its options before consensus forms.
- ADR 0001 records the decision to adopt ADRs.
- Linting scripts adapted from konflux-ci/architecture to validate ADR
  statuses and check for duplicate numbering.
- Makefile with lint target for local validation.

Note: .github/workflows/lint.yml is included as a separate commit since
pushing workflow files requires the 'workflow' PAT scope.

Assisted-by: OpenCode claude-opus-4-6@default
Runs 'make lint' on pushes to main and pull requests to validate ADR
statuses and number uniqueness.

Assisted-by: OpenCode claude-opus-4-6@default
@ralphbean
ralphbean marked this pull request as ready for review March 20, 2026 14:54
@ralphbean
ralphbean requested a review from a team as a code owner March 20, 2026 14:54
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Introduce lightweight ADR process with CI linting

✨ Enhancement 📝 Documentation

Grey Divider

Walkthroughs

Description
• Introduce Architecture Decision Records (ADRs) process with five statuses
  - Proposed, Undecided, Accepted, Deprecated, Superseded
• Undecided state enables merging ADRs before consensus forms
• Add ADR template and first ADR documenting adoption decision
• Implement linting scripts to validate ADR statuses and number uniqueness
• Add Makefile with lint targets and GitHub Actions CI workflow
Diagram
flowchart LR
  A["ADR Template<br/>0000-adr-template.md"] --> B["ADR 0001<br/>Use ADRs Decision"]
  C["Linting Scripts<br/>get-adr-status.sh<br/>lint-adr-status<br/>lint-adr-numbers"] --> D["Makefile<br/>lint targets"]
  D --> E["GitHub Actions<br/>lint.yml workflow"]
  B --> F["docs/ADRs/<br/>Directory"]
  C --> F
Loading

Grey Divider

File Changes

1. hack/util/get-adr-status.sh ✨ Enhancement +95/-0

ADR status extraction and validation utility

• Extract and validate status from ADR markdown files
• Support five valid statuses: Proposed, Undecided, Accepted, Deprecated, Superseded
• Parse ## Status section and validate against allowed values
• Provide detailed error codes and messages for debugging

hack/util/get-adr-status.sh


2. hack/lint-adr-status ✨ Enhancement +66/-0

ADR status linting script

• Validate ADR statuses for all ADR files in docs/ADRs/
• Use get-adr-status.sh utility to check each file
• Skip template file (0000-adr-template.md) from validation
• Report summary of validation results with error count

hack/lint-adr-status


3. hack/lint-adr-numbers ✨ Enhancement +63/-0

ADR number uniqueness validation script

• Check for duplicate ADR numeric identifiers
• Parse NNNN- prefix from filenames and track uniqueness
• Skip template file from duplicate checking
• Report detailed error messages for duplicate numbers found

hack/lint-adr-numbers


View more (5)
4. Makefile ⚙️ Configuration changes +16/-0

Makefile with ADR linting targets

• Define lint target that runs both ADR linting checks
• Add lint-adr-status and lint-adr-numbers subtargets
• Include help target documenting all available targets
• Enable local validation before pushing changes

Makefile


5. .github/workflows/lint.yml ⚙️ Configuration changes +19/-0

GitHub Actions CI workflow for ADR linting

• GitHub Actions workflow for automated ADR linting
• Runs on pushes to main and pull requests
• Execute make lint to validate ADR statuses and numbers
• Trigger on merge_group events for merge queue support

.github/workflows/lint.yml


6. docs/ADRs/0000-adr-template.md 📝 Documentation +32/-0

ADR template with status-aware sections

• Template for creating new ADRs with standard structure
• Include sections for Status, Context, Options, Decision, Consequences
• Mark Options and Decision sections as conditional based on ADR status
• Provide guidance for Undecided ADRs with empty Decision section

docs/ADRs/0000-adr-template.md


7. docs/ADRs/0001-use-adrs-for-decision-making.md 📝 Documentation +62/-0

ADR 0001 documenting ADR adoption decision

• First ADR documenting decision to adopt Architecture Decision Records
• Explain motivation: separate options exploration from final decisions
• Define five status values and their meanings in workflow
• Highlight Undecided state for merging ADRs before consensus
• Reference linting borrowed from konflux-ci/architecture repo

docs/ADRs/0001-use-adrs-for-decision-making.md


8. README.md 📝 Documentation +1/-0

Add ADR directory reference to README

• Add reference to docs/ADRs/ directory in main documentation
• Link to ADR 0001 explaining the decision-making process
• Position ADRs alongside problem documents in navigation

README.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. ADR 0001 lacks options 📘 Rule violation ⚙ Maintainability
Description
docs/ADRs/0001-use-adrs-for-decision-making.md records a decision but does not include an
Options/Alternatives section describing at least two approaches with explicit trade-offs. This
violates the requirement that design docs compare multiple solution options with trade-offs and
rationale for the chosen approach.
Code

docs/ADRs/0001-use-adrs-for-decision-making.md[R9-28]

+## Context
+
+Fullsend is a design exploration repo with multiple problem documents that
+evolve independently. As thinking matures in these problem areas, we need a way
+to crystallize specific decisions without rushing to conclusions. The existing
+problem documents are good for exploring the space, but they don't clearly
+separate "options we're considering" from "decisions we've made."
+
+We want a lightweight process that lets us:
+
+- Propose decisions that we know need to be made, even before we've chosen an
+  answer.
+- Describe options and trade-offs in a structured way.
+- Record the final decision and its rationale once consensus forms.
+- Keep a clear history of what was decided and why.
+
+## Decision
+
+We adopt Architecture Decision Records (ADRs), following the format described
+by Michael Nygard, adapted for this repo's needs.
Evidence
PR Compliance ID 131399 requires design docs to describe multiple solution options and trade-offs;
ADR 0001 jumps from ## Context directly to ## Decision without any Options/Alternatives
section or trade-off comparison.

Rule 131399: Design docs must compare multiple solution options with explicit trade-offs
docs/ADRs/0001-use-adrs-for-decision-making.md[9-28]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
ADR 0001 does not document multiple solution options with explicit trade-offs; it moves from context straight to a single decision.
## Issue Context
Compliance requires that design docs (including ADRs) compare at least two distinct options and state trade-offs across multiple axes, then explain why the chosen option was selected.
## Fix Focus Areas
- docs/ADRs/0001-use-adrs-for-decision-making.md[9-52]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Error counter exits early🐞 Bug ⛯ Reliability
Description
hack/lint-adr-status increments errors with ((errors++)) under set -e, which returns a
non-zero exit status on the first failure (when errors was 0) and can terminate the script
immediately. This prevents the linter from checking remaining ADR files and from reaching the
intended final summary/exit logic.
Code

hack/lint-adr-status[R49-55]

+    if status=$("$UTIL_DIR/get-adr-status.sh" "$file" 2>&1); then
+        echo "$status"
+    else
+        echo "FAILED"
+        error "  $status"
+        ((errors++))
+    fi
Evidence
The script enables set -euo pipefail, then on a per-file failure path runs ((errors++)). In
bash, the exit status of (( ... )) is 1 when the evaluated arithmetic result is 0; for
post-increment, the expression evaluates to the *previous* value, so the first increment evaluates
to 0 and triggers set -e to exit early.

hack/lint-adr-status[7-66]
hack/lint-adr-status[49-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hack/lint-adr-status` uses `set -euo pipefail` and increments the `errors` counter with `((errors++))`. On the first error, this arithmetic command evaluates to 0 and returns exit status 1, which can cause the script to exit immediately under `set -e`.
### Issue Context
The script intends to continue scanning all ADR files, count failures, and then exit non-zero at the end if any errors were found.
### Fix Focus Areas
- hack/lint-adr-status[49-55]
### Suggested change
Replace `((errors++))` with one of:
- `((++errors))` (preferred)
- `errors=$((errors + 1))`
- `((errors++)) || true` (least preferred)
Ensure the script continues looping and reports the final summary correctly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Missing workflow token scoping🐞 Bug ⛨ Security
Description
The new .github/workflows/lint.yml does not set explicit permissions for GITHUB_TOKEN, so the
job inherits repository defaults that may be broader than needed for a read-only lint job. This
increases blast radius if the repository default permissions are permissive.
Code

.github/workflows/lint.yml[R1-19]

+name: Lint ADRs
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+  merge_group:
+
+jobs:
+  lint:
+    name: Lint ADRs
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Run make lint
+        run: make lint
Evidence
The workflow checks out code and runs local linting but does not declare a permissions: block,
relying on whatever defaults are configured at the repo/org level; GitHub security guidance
recommends explicitly setting minimal required permissions for workflows.

.github/workflows/lint.yml[1-19]
Best Practice: GitHub Actions Security Hardening

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The lint workflow does not define explicit `permissions`, which can leave the workflow over-privileged depending on repository defaults.
### Issue Context
This job only needs to read repository contents to run `make lint`.
### Fix Focus Areas
- .github/workflows/lint.yml[1-19]
### Suggested change
Add a top-level or job-level permissions block, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread docs/ADRs/0001-use-adrs-for-decision-making.md
Comment thread hack/lint-adr-status
Add structured frontmatter to ADR files, adapted from the
konflux-ci/architecture repo. Instead of mapping ADRs to services
(applies_to), fullsend maps ADRs to problem documents (relates_to)
since problem docs are the primary organizational unit here.

Frontmatter fields:
- title (required) - ADR title
- status (required) - must match the ## Status section in the body
- relates_to - list of problem doc filenames (without .md) from
  docs/problems/, or "*" for cross-cutting ADRs
- topics - free-form tags for discoverability

The lint-adr-frontmatter script validates all of the above, including
cross-reference checking against existing problem docs.

Assisted-by: OpenCode claude-opus-4-6@default
@waynesun09

Copy link
Copy Markdown
Member

Additional Review Findings (not covered by Qodo)

Reviewed with Claude Code and Gemini. These are issues Qodo's review did not flag.


Critical

1. Bash 4.0+ required but not checked — hack/lint-adr-numbers:28

declare -A (associative arrays) requires Bash 4.0+. macOS ships Bash 3.2 due to GPLv3 licensing, so this script will fail on contributor machines with:

./hack/lint-adr-numbers: line 28: declare: -A: invalid option

Suggested fix — add a version check at the top of the script:

if ((BASH_VERSINFO[0] < 4)); then
    error "This script requires Bash 4.0 or higher (found ${BASH_VERSION})"
    error "On macOS, install via: brew install bash"
    exit 1
fi

Minor

2. PyYAML not installed in CI workflow

hack/lint-adr-frontmatter requires PyYAML but the workflow has no install step. CI will fail on the Python linter. Add a step before make lint:

- name: Install Python dependencies
  run: pip install pyyaml

3. Template frontmatter contains invalid YAML — docs/ADRs/0000-adr-template.md:3

status: Proposed | Undecided | Accepted | Deprecated | Superseded

This is a string literal, not a choice selector. Contributors copying the template verbatim will get lint errors. Consider:

status: Proposed  # Valid values: Proposed, Undecided, Accepted, Deprecated, Superseded

4. Makefile default target runs lint instead of help

Running make with no arguments executes lint (the first non-.PHONY target) rather than help. Add:

.DEFAULT_GOAL := help

Nit

5. Qodo's "ADR 0001 lacks Options section" finding is a false positive — the template states Options is "Required for Undecided ADRs" and ADR 0001 has status Accepted. The ADR correctly follows the template for its status.

OpenCode Agent and others added 3 commits March 20, 2026 15:39
Fixes from review by waynesun09 and Qodo:

- Fix ((errors++)) under set -e in lint-adr-status: post-increment
  evaluates to 0 on first error, causing set -e to exit early. Use
  ((++errors)) instead (pre-increment evaluates to 1).
- Remove Bash 4.0+ dependency from lint-adr-numbers: replace
  associative array with sort | uniq -d for duplicate detection.
  Simpler and works on macOS default Bash 3.2.
- Fix template frontmatter: status field had pipe-separated values
  which is a string literal, not valid YAML for a status. Use a
  comment to document valid values instead.
- Add .DEFAULT_GOAL := help to Makefile so bare 'make' shows help.

The Qodo finding about ADR 0001 lacking an Options section is a false
positive: Options are only required for Undecided ADRs per the template,
and ADR 0001 has status Accepted.

Workflow changes (permissions block, pip install pyyaml) require the
'workflow' PAT scope and are in a separate commit.

Assisted-by: OpenCode claude-opus-4-6@default
Runs 'make lint' on pushes to main and pull requests to validate ADR
statuses and number uniqueness.

Assisted-by: OpenCode claude-opus-4-6@default
waynesun09 added a commit that referenced this pull request Mar 20, 2026
Experiment code has heavy dependencies (torch, transformers, anthropic)
that aren't installed in CI. ty runs locally via pre-commit where
developers have their venv. The CI ty step can be added back when
hack/ scripts land (PR #39) as a lightweight target.

Signed-off-by: Wayne Sun <gsun@redhat.com>
Add pre-commit configuration with:
- ruff for linting and formatting
- ty for type checking (via uvx)
- bandit for Python security scanning
- gitleaks for secret detection
- actionlint for GitHub Actions workflow validation
- pre-commit-hooks for YAML/JSON/TOML validation and private key detection

Update CI workflow to use uv + pre-commit + ty check hack/ with
explicit permissions: contents: read.

Add PEP 723 inline script metadata to hack/lint-adr-frontmatter so
uv run auto-installs PyYAML without needing a pyproject.toml.

Update Makefile with check and fmt targets.

Pattern borrowed from redhat-community-ai-tools/cicaddy-action.

Signed-off-by: Wayne Sun <gsun@redhat.com>
Auto-fix import sorting (I001), unused imports (F401), line length
(E501), trailing whitespace, and missing trailing newlines across
experiment code and problem docs.

Signed-off-by: Wayne Sun <gsun@redhat.com>
Convert .format() calls to f-strings, remove unnecessary "r" mode
args from open(), and combine nested if statements.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@ralphbean

Copy link
Copy Markdown
Member Author

@waynesun09 thanks for the fixups!

This needs an approval from @konflux-ci/fullsend-sig in order to merge.

@ralphbean
ralphbean enabled auto-merge March 20, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants