OCI format ADR#115
Open
dmartinol wants to merge 1 commit into
Open
Conversation
Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
📝 WalkthroughWalkthroughTwo documentation files introduce an architecture decision record for OCI artifact format support in Lola modules, including design rationale, metadata specifications, secure verification strategies, and proposed CLI commands with implementation phases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
docs/adr/0007-oci-format.md (2)
445-445: Use “CLI compatibility” instead of “CLI interface compatibility.”“Interface” is redundant in this phrase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0007-oci-format.md` at line 445, Update the phrasing in the ADR text by replacing the phrase "CLI interface compatibility (Click → Cobra)" with the concise "CLI compatibility (Click → Cobra)"; locate the exact string "CLI interface compatibility (Click → Cobra)" in docs/adr/0007-oci-format.md and change it to "CLI compatibility (Click → Cobra)" so the word "interface" is removed.
365-365: Add language identifiers to fenced code blocks.Fences opened on Line 365, Line 392, and Line 410 are missing a language tag, which triggers
MD040. Usingtextfor tree/architecture diagrams keeps lint clean and readability high.Proposed doc-only diff
-``` +```text OCI Image: ... -``` +``` -``` +```text .lola/modules/module-name/ ... -``` +``` -``` +```text lola (Go binary) ... -``` +```Also applies to: 392-392, 410-410
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0007-oci-format.md` at line 365, Three fenced code blocks are missing language identifiers causing MD040; update the three backtick fences that surround the blocks starting with "OCI Image:", ".lola/modules/module-name/", and "lola (Go binary)" to use a language tag (use "text") so each opening fence becomes ```text and the corresponding closing fence remains ```, keeping the block contents unchanged; apply the same change for the other occurrences mentioned (the blocks around those three headings).docs/adr/0007-oci-format/oci-cli-exploration.md (1)
29-29: Add a language tag to the fenced block at Line 29.This currently triggers
MD040;textis appropriate for the tree diagram.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0007-oci-format/oci-cli-exploration.md` at line 29, The fenced code block that contains the tree diagram is missing a language tag (triggering MD040); update the opening backtick fence for that block (the fenced block beginning at the tree diagram around line 29) to include the language tag "text" (e.g., ```text) so the markdown linter recognizes the code block type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0007-oci-format/oci-cli-exploration.md`:
- Around line 1441-1443: The documented provenance flag is inconsistent: replace
the mention of `--skip-provenance` with the already defined
`--skip-verification` (or explicitly document both if provenance has a separate
flag) so the "Provenance verification" bullet uses the same flag names as the
earlier OCI command options; update the line that reads `Provenance
verification: **Enabled** for OCI modules (use --skip-provenance flag to
disable)` to reference `--skip-verification` (or add clarifying text listing
both `--skip-verification` and `--skip-provenance`) to keep
`--skip-verification` and `--skip-provenance` consistent across the doc.
- Around line 909-913: The docs show conflicting command flows for installing
OCI modules (sometimes using the two-step "lola mod add <source>" then "lola
install <module> -a <assistant>" flow, and other times invoking "lola install
oci://..." directly, and option names like "--verify-signature" don't match
earlier specs); pick one canonical contract (either the two-step registry flow
using "lola mod add" + "lola install <module> -a <assistant>" or the
direct-install flow "lola install oci://<...> -a <assistant>"), then update all
examples and error messages to that contract consistently (including normalizing
option names such as "--verify-signature" to the agreed flag name), and audit
the referenced sections (the current examples and errors around the OCI CLI
exploration doc, plus the other occurrences noted) to ensure every example,
error text, and option usage matches the chosen command format and flag names
(search for "lola mod add", "lola install", "oci://", and "--verify-signature"
to find instances to change).
- Around line 27-28: The doc currently declares Lola modules are packaged as
"single-layer OCI images" but the example outputs later show multiple content
layers; update the examples that describe image build/push output (the examples
currently showing multiple content layers) to reflect a single-layer model by
consolidating layers into one content layer in the output text, adjusting
sizes/digests/summary lines accordingly, and ensuring any CLI outputs or example
manifests referenced in the "single-layer OCI images" section are consistent
with that single-layer representation; search for the phrase "single-layer OCI
images" and the example blocks that show "content layers" and change those
outputs to a single-layer format.
- Around line 73-80: The Table of Contents contains anchor fragments that don't
match existing headings (e.g., entries like "#new-commands",
"#modified-commands", "#configuration", "#possible-implementation-phasing");
update each TOC link to the exact heading text used in the document (or
normalize the headings to match the TOC) so intra-doc navigation works—ensure
the TOC entries such as "New Commands", "Modified Commands", "Configuration",
and "Possible Implementation Phasing" map exactly to their corresponding
headings or change the headings to the slugified forms used in the TOC so
anchors like Module Metadata and SkillCard and Workflow Pattern: Add Then
Install resolve correctly.
---
Nitpick comments:
In `@docs/adr/0007-oci-format.md`:
- Line 445: Update the phrasing in the ADR text by replacing the phrase "CLI
interface compatibility (Click → Cobra)" with the concise "CLI compatibility
(Click → Cobra)"; locate the exact string "CLI interface compatibility (Click →
Cobra)" in docs/adr/0007-oci-format.md and change it to "CLI compatibility
(Click → Cobra)" so the word "interface" is removed.
- Line 365: Three fenced code blocks are missing language identifiers causing
MD040; update the three backtick fences that surround the blocks starting with
"OCI Image:", ".lola/modules/module-name/", and "lola (Go binary)" to use a
language tag (use "text") so each opening fence becomes ```text and the
corresponding closing fence remains ```, keeping the block contents unchanged;
apply the same change for the other occurrences mentioned (the blocks around
those three headings).
In `@docs/adr/0007-oci-format/oci-cli-exploration.md`:
- Line 29: The fenced code block that contains the tree diagram is missing a
language tag (triggering MD040); update the opening backtick fence for that
block (the fenced block beginning at the tree diagram around line 29) to include
the language tag "text" (e.g., ```text) so the markdown linter recognizes the
code block type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c44a68ff-1a38-4923-8a24-bf49119de34e
📒 Files selected for processing (2)
docs/adr/0007-oci-format.mddocs/adr/0007-oci-format/oci-cli-exploration.md
Comment on lines
+27
to
+28
| Following skillimage's minimalist approach, Lola modules are packaged as **single-layer OCI images**: | ||
|
|
There was a problem hiding this comment.
Align build/push output examples with the declared single-layer OCI model.
Line 27–28 defines single-layer images, but output examples on Line 123–124 and Line 499–503 describe multiple content layers. Keep one model throughout to avoid implementation drift.
Also applies to: 123-124, 499-503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/adr/0007-oci-format/oci-cli-exploration.md` around lines 27 - 28, The
doc currently declares Lola modules are packaged as "single-layer OCI images"
but the example outputs later show multiple content layers; update the examples
that describe image build/push output (the examples currently showing multiple
content layers) to reflect a single-layer model by consolidating layers into one
content layer in the output text, adjusting sizes/digests/summary lines
accordingly, and ensuring any CLI outputs or example manifests referenced in the
"single-layer OCI images" section are consistent with that single-layer
representation; search for the phrase "single-layer OCI images" and the example
blocks that show "content layers" and change those outputs to a single-layer
format.
Comment on lines
+73
to
+80
| 1. [New Commands](#new-commands) | ||
| - [Module Metadata and SkillCard](#module-metadata-and-skillcard) | ||
| 2. [Modified Commands](#modified-commands) | ||
| - [Workflow Pattern: Add Then Install](#workflow-pattern-add-then-install) | ||
| 3. [CLI Examples by Use Case](#cli-examples-by-use-case) | ||
| 4. [Error Handling](#error-handling) | ||
| 5. [Configuration](#configuration) | ||
| 6. [Possible Implementation Phasing](#possible-implementation-phasing) |
There was a problem hiding this comment.
Fix invalid Table of Contents anchor fragments.
Several links point to headings that don’t exist verbatim (e.g., #new-commands, #modified-commands, #configuration, #possible-implementation-phasing). This breaks intra-doc navigation.
Proposed doc-only diff
-1. [New Commands](`#new-commands`)
+1. [New Commands](`#possible-new-commands`)
- [Module Metadata and SkillCard](`#module-metadata-and-skillcard`)
-2. [Modified Commands](`#modified-commands`)
+2. [Modified Commands](`#possible-command-modifications`)
- [Workflow Pattern: Add Then Install](`#workflow-pattern-add-then-install`)
3. [CLI Examples by Use Case](`#cli-examples-by-use-case`)
4. [Error Handling](`#error-handling`)
-5. [Configuration](`#configuration`)
-6. [Possible Implementation Phasing](`#possible-implementation-phasing`)
+5. [Configuration](`#configuration-optional`)
+6. [Possible Implementation Phasing](`#implementation-phasing`)
7. [Future Enhancements](`#future-enhancements-not-for-initial-version`)
8. [Backward Compatibility](`#backward-compatibility`)🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 73-73: Link fragments should be valid
(MD051, link-fragments)
[warning] 75-75: Link fragments should be valid
(MD051, link-fragments)
[warning] 79-79: Link fragments should be valid
(MD051, link-fragments)
[warning] 80-80: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/adr/0007-oci-format/oci-cli-exploration.md` around lines 73 - 80, The
Table of Contents contains anchor fragments that don't match existing headings
(e.g., entries like "#new-commands", "#modified-commands", "#configuration",
"#possible-implementation-phasing"); update each TOC link to the exact heading
text used in the document (or normalize the headings to match the TOC) so
intra-doc navigation works—ensure the TOC entries such as "New Commands",
"Modified Commands", "Configuration", and "Possible Implementation Phasing" map
exactly to their corresponding headings or change the headings to the slugified
forms used in the TOC so anchors like Module Metadata and SkillCard and Workflow
Pattern: Add Then Install resolve correctly.
Comment on lines
+909
to
+913
| **Important Architectural Note:** To maintain consistency with Lola's current behavior, the workflow for OCI modules follows the existing two-step pattern: | ||
|
|
||
| 1. **`lola mod add <source>`** - Registers the module to `~/.lola/modules/` (local module registry) | ||
| 2. **`lola install <module> -a <assistant>`** - Installs from registry to the specified assistant | ||
|
|
There was a problem hiding this comment.
Resolve command-flow contradictions for lola install.
Line 911–913 defines mod add then install <module>, but later examples invoke lola install oci://... directly. This also introduces option mismatches (e.g., --verify-signature) relative to earlier command specs. Pick one contract and normalize all examples/errors to it.
Also applies to: 1115-1116, 1357-1357, 1374-1374, 1397-1407
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/adr/0007-oci-format/oci-cli-exploration.md` around lines 909 - 913, The
docs show conflicting command flows for installing OCI modules (sometimes using
the two-step "lola mod add <source>" then "lola install <module> -a <assistant>"
flow, and other times invoking "lola install oci://..." directly, and option
names like "--verify-signature" don't match earlier specs); pick one canonical
contract (either the two-step registry flow using "lola mod add" + "lola install
<module> -a <assistant>" or the direct-install flow "lola install oci://<...> -a
<assistant>"), then update all examples and error messages to that contract
consistently (including normalizing option names such as "--verify-signature" to
the agreed flag name), and audit the referenced sections (the current examples
and errors around the OCI CLI exploration doc, plus the other occurrences noted)
to ensure every example, error text, and option usage matches the chosen command
format and flag names (search for "lola mod add", "lola install", "oci://", and
"--verify-signature" to find instances to change).
Comment on lines
+1441
to
+1443
| - Signature verification: **Enabled** for OCI modules (use `--skip-verification` flag to disable) | ||
| - Provenance verification: **Enabled** for OCI modules (use `--skip-provenance` flag to disable) | ||
| - Registry authentication: Delegated to `podman login`, `docker login`, or `skopeo login` |
There was a problem hiding this comment.
Documented provenance flag doesn’t match earlier command options.
Line 1442 mentions --skip-provenance, but earlier OCI command options define --skip-verification instead. This should be reconciled to prevent user confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/adr/0007-oci-format/oci-cli-exploration.md` around lines 1441 - 1443,
The documented provenance flag is inconsistent: replace the mention of
`--skip-provenance` with the already defined `--skip-verification` (or
explicitly document both if provenance has a separate flag) so the "Provenance
verification" bullet uses the same flag names as the earlier OCI command
options; update the line that reads `Provenance verification: **Enabled** for
OCI modules (use --skip-provenance flag to disable)` to reference
`--skip-verification` (or add clarifying text listing both `--skip-verification`
and `--skip-provenance`) to keep `--skip-verification` and `--skip-provenance`
consistent across the doc.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
An ADR to integrate OCI format for lola modules.
Related Issues
NA
Test Plan
AI Disclosure
AI-assisted with Claude Code
Summary by CodeRabbit
Documentation
lola build,lola push,lola sign,lola verify,lola inspect) for module operations