NO-ISSUE: feat(workflows): adopt Lola v0.5.0 pip-style URL fragments - #41
Open
quay-devel wants to merge 2 commits into
Open
NO-ISSUE: feat(workflows): adopt Lola v0.5.0 pip-style URL fragments#41quay-devel wants to merge 2 commits into
quay-devel wants to merge 2 commits into
Conversation
…in .lola-req Lola v0.5.0 adds pip-style URL fragment support for .lola-req files. This replaces the custom --module-content flag with the standard #subdirectory= fragment syntax and adds -a claude-code to lola sync calls for explicit assistant targeting. For workflows with multiple subdirectories from the same repository (quay-bugfix, quay-cvefix, quay-ticket), session-setup.sh parses fragments and uses lola mod add + lola install to avoid a name collision in lola sync (tracked upstream). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR migrates Lola plugin dependency specifications across documentation and workflow automation from ChangesLola plugin dependency migration
🎯 2 (Simple) | ⏱️ ~12 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (1)
37-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the syntax migration in the “One-time setup” example.
Line 38-40 still use
--module-content=..., which conflicts with the new#subdirectory=...guidance and can cause mixed usage in copy/paste flows.Suggested diff
-uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git --module-content=plugins/dev -uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git --module-content=plugins/jira-planning -uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git --module-content=plugins/openshift-testing +uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git#subdirectory=plugins/dev +uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git#subdirectory=plugins/jira-planning +uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git#subdirectory=plugins/openshift-testing🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 37 - 40, The README's "One-time setup" uvx commands still use the deprecated flag --module-content=..., causing mixed syntax with the new `#subdirectory` guidance; update each uvx command shown (the three lines beginning with "uvx --python 3.13 --from lola-ai lola mod add https://github.com/quay/ai-helpers.git --module-content=plugins/...") by removing the --module-content argument and appending the subdirectory to the repo URL as a fragment (use `#subdirectory`=plugins/<name>), e.g. change the URL portion to ...ai-helpers.git#subdirectory=plugins/dev (and similarly for jira-planning and openshift-testing) so the commands uniformly use the new syntax.enhancements/001-workflow-architecture.md (1)
342-344:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix contradictory bootstrap instruction (
ln -s) in Phase 5 example.The example at Line 343 tells users to create a symlink, but this document already states symlinks are not viable for hydrated workflow subpaths (Line 202-205). This can produce a non-functional
session-setup.shat runtime.Suggested diff
-# Symlink the shared bootstrap script -ln -s ../../../scripts/session-setup.sh workflows/clair/.claude/scripts/session-setup.sh +# Copy the shared bootstrap script (symlinks don't survive hydrate subpath extraction) +cp scripts/session-setup.sh workflows/clair/.claude/scripts/session-setup.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@enhancements/001-workflow-architecture.md` around lines 342 - 344, The Phase 5 example currently uses a symlink instruction ("ln -s") to create workflows/clair/.claude/scripts/session-setup.sh which contradicts the earlier note that symlinks are not supported for hydrated workflow subpaths (see the session-setup.sh reference and the non-viability note at Lines 202–205); replace the symlink step with a copy or direct file placement instruction so the actual session-setup.sh file exists in workflows/clair/.claude/scripts/, ensuring the bootstrap script is present at runtime (update the "Symlink the shared bootstrap script" section to create or copy the real file rather than using ln -s).workflows/quay-ticket/.claude/scripts/session-setup.sh (1)
55-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPost-install validation is ineffective in this workflow.
This check can pass even if plugin installation failed, because
session-setup.shitself keeps.claude/scriptsnon-empty.🔧 Suggested fix
-if [ -z "$(ls -A "${CLAUDE_DIR}/scripts" 2>/dev/null)" ]; then +installed_count="$(find "${CLAUDE_DIR}/scripts" -maxdepth 1 -type f ! -name 'session-setup.sh' | wc -l | tr -d ' ')" +if [ "${installed_count}" = "0" ]; then echo "ERROR: .claude/scripts/ is empty after plugin install — check .lola-req" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workflows/quay-ticket/.claude/scripts/session-setup.sh` around lines 55 - 58, The current post-install validation in session-setup.sh uses a simple emptiness check on "${CLAUDE_DIR}/scripts" which can be fooled because session-setup.sh itself may seed that directory; modify the script to record the directory contents before running the plugin install (e.g., PRE_SCRIPTS="$(ls -A "${CLAUDE_DIR}/scripts" 2>/dev/null || true)"), run the install, then compute POST_SCRIPTS and fail only if no new entries were added (compare PRE_SCRIPTS vs POST_SCRIPTS with comm/diff or by checking for any file in POST_SCRIPTS not present in PRE_SCRIPTS); reference the CLAUDE_DIR/scripts path and the session-setup.sh install step to locate where to add PRE_SCRIPTS, the install invocation, and the post-install comparison logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 52-57: The fenced code block beginning with "# .lola-req — AI
context modules for this project" in README.md is missing a language tag and
triggers markdownlint MD040; update the opening fence to include a language
identifier (e.g., add "text" after the triple backticks) so the block becomes a
labeled code fence and resolves the lint warning while preserving the existing
lines with the three repository URLs.
In `@workflows/quay-bugfix/.claude/scripts/session-setup.sh`:
- Line 48: The command using the unsupported flag "--module-content" in the
invocation of $LOLA mod add should be changed to supply the subdirectory via the
URL fragment or supported config; remove the "--module-content=\"$subdir\""
argument and instead append "`#subdirectory`=$subdir" (or other supported
fragment) to the "$url" variable before calling "$LOLA mod add \"$url\" --name
\"$name\"" (alternatively update the repository to include a .lola-req with the
subdir), ensuring the call no longer passes the unsupported flag.
In `@workflows/quay-bugfix/README.md`:
- Around line 71-74: Update the README text to reflect the current workflow:
replace the statement that "session-setup.sh uses lola sync to install plugins
declared in .lola-req" with a description that session-setup.sh instead uses
fragment-parsing flow via "lola mod add" followed by "lola install" (the shim)
to add modules and avoid same-repo module-name collisions, and note that
plugins' post-install hooks still copy scripts/templates into .claude/scripts/
and .claude/templates/; mention the replaced commands (lola mod add + lola
install) and remove the obsolete reference to lola sync.
In `@workflows/quay-ticket/CLAUDE.md`:
- Line 24: Update the inaccurate bootstrap description to reflect the actual
install flow: replace the claim that `session-setup.sh` runs `lola sync` with a
statement that it parses `.lola-req` fragments and installs modules by invoking
`lola mod add` for each module followed by `lola install` with explicit names;
mention the exact script `session-setup.sh` and the file `.lola-req` as the
sources of truth and include the commands `lola mod add` and `lola install` as
the operations performed.
In `@workflows/quay-ticket/README.md`:
- Around line 48-51: The README's Bootstrap section incorrectly states that
session-setup.sh uses `lola sync`; update the doc to reflect the actual
implementation: describe that `session-setup.sh` (the SessionStart hook)
installs plugins by running `lola mod add` followed by `lola install` for
entries in `.lola-req`, and that plugin post-install hooks copy
scripts/templates into `.claude/scripts/` and `.claude/templates/`; mention
`session-setup.sh`, `.lola-req`, and the `.claude/*` directories so readers can
locate the related implementation.
---
Outside diff comments:
In `@enhancements/001-workflow-architecture.md`:
- Around line 342-344: The Phase 5 example currently uses a symlink instruction
("ln -s") to create workflows/clair/.claude/scripts/session-setup.sh which
contradicts the earlier note that symlinks are not supported for hydrated
workflow subpaths (see the session-setup.sh reference and the non-viability note
at Lines 202–205); replace the symlink step with a copy or direct file placement
instruction so the actual session-setup.sh file exists in
workflows/clair/.claude/scripts/, ensuring the bootstrap script is present at
runtime (update the "Symlink the shared bootstrap script" section to create or
copy the real file rather than using ln -s).
In `@README.md`:
- Around line 37-40: The README's "One-time setup" uvx commands still use the
deprecated flag --module-content=..., causing mixed syntax with the new
`#subdirectory` guidance; update each uvx command shown (the three lines beginning
with "uvx --python 3.13 --from lola-ai lola mod add
https://github.com/quay/ai-helpers.git --module-content=plugins/...") by
removing the --module-content argument and appending the subdirectory to the
repo URL as a fragment (use `#subdirectory`=plugins/<name>), e.g. change the URL
portion to ...ai-helpers.git#subdirectory=plugins/dev (and similarly for
jira-planning and openshift-testing) so the commands uniformly use the new
syntax.
In `@workflows/quay-ticket/.claude/scripts/session-setup.sh`:
- Around line 55-58: The current post-install validation in session-setup.sh
uses a simple emptiness check on "${CLAUDE_DIR}/scripts" which can be fooled
because session-setup.sh itself may seed that directory; modify the script to
record the directory contents before running the plugin install (e.g.,
PRE_SCRIPTS="$(ls -A "${CLAUDE_DIR}/scripts" 2>/dev/null || true)"), run the
install, then compute POST_SCRIPTS and fail only if no new entries were added
(compare PRE_SCRIPTS vs POST_SCRIPTS with comm/diff or by checking for any file
in POST_SCRIPTS not present in PRE_SCRIPTS); reference the CLAUDE_DIR/scripts
path and the session-setup.sh install step to locate where to add PRE_SCRIPTS,
the install invocation, and the post-install comparison logic.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 7c585c47-bbd1-4d7e-85bb-553493e7117f
📒 Files selected for processing (19)
README.mdenhancements/001-workflow-architecture.mdenhancements/002-quay-bugfix-workflow.mdscripts/session-setup.shworkflows/autofix-dispatcher/.claude/scripts/session-setup.shworkflows/konflux-build-debugger/.claude/scripts/session-setup.shworkflows/konflux-build-debugger/.lola-reqworkflows/konflux-build-triage/.claude/scripts/session-setup.shworkflows/quay-bugfix/.claude/scripts/session-setup.shworkflows/quay-bugfix/.lola-reqworkflows/quay-bugfix/README.mdworkflows/quay-cvefix/.claude/scripts/session-setup.shworkflows/quay-cvefix/.lola-reqworkflows/quay-deploy/.claude/scripts/session-setup.shworkflows/quay-deploy/.lola-reqworkflows/quay-ticket/.claude/scripts/session-setup.shworkflows/quay-ticket/.lola-reqworkflows/quay-ticket/CLAUDE.mdworkflows/quay-ticket/README.md
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (12)
scripts/session-setup.sh (1)
19-19: LGTM!workflows/autofix-dispatcher/.claude/scripts/session-setup.sh (1)
19-19: LGTM!workflows/quay-deploy/.claude/scripts/session-setup.sh (1)
20-20: LGTM!workflows/konflux-build-debugger/.claude/scripts/session-setup.sh (1)
147-148: LGTM!workflows/konflux-build-triage/.claude/scripts/session-setup.sh (1)
147-148: LGTM!enhancements/002-quay-bugfix-workflow.md (1)
160-161: LGTM!workflows/konflux-build-debugger/.lola-req (1)
2-2: LGTM!workflows/quay-bugfix/.lola-req (1)
1-3: LGTM!workflows/quay-cvefix/.lola-req (1)
1-3: LGTM!workflows/quay-deploy/.lola-req (1)
1-2: LGTM!workflows/quay-ticket/.lola-req (1)
1-3: LGTM!workflows/quay-cvefix/.claude/scripts/session-setup.sh (1)
48-48: Same Lola flag-compatibility concern applies here.This line matches the previously noted
--module-contentcompatibility check in the bugfix workflow script.
| if [ -n "$content_dir" ]; then | ||
| $LOLA mod add "$url" --module-content="$content_dir" --name "$name" 2>&1 | tail -1 | ||
| if [ -n "$subdir" ]; then | ||
| $LOLA mod add "$url" --module-content="$subdir" --name "$name" 2>&1 | tail -1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Lola v0.5.0, is lola mod add --module-content=still supported, or should subdirectory be provided only via URL fragments like#subdirectory=?
💡 Result:
In Lola v0.5.0, lola mod add --module-content=<path> is not supported (it is not present in the current CLI/module-management docs for Lola); for specifying a subdirectory of module content you should instead use URL fragments of the form #subdirectory=<path> (or equivalent config such as .lola-req), since ongoing work and issues focus on adding “content subdirectory path” via request/config handling rather than a --module-content flag. Evidence: The official Lola docs show lola mod add <source> with no --module-content option mentioned, and module registration is done from git/zip/tar/folder sources via lola mod add https://… style commands [1][2]. Additionally, there is an open/merged development thread specifically about “support content subdirectory path in.lola-req for monorepo git sources,” indicating the mechanism for subdirectory-scoped content is part of the request/config format rather than a CLI --module-content flag [3].
Citations:
- 1: https://github.com/LobsterTrap/lola/blob/main/AGENTS.md
- 2: https://lobstertrap.org/lola/
- 3: feat: support content subdirectory path in .lola-req for monorepo git sources LobsterTrap/lola#118
Remove/replace unsupported --module-content flag for Lola v0.5.0
In workflows/quay-bugfix/.claude/scripts/session-setup.sh (line 48), the command $LOLA mod add "$url" --module-content="$subdir" --name "$name" uses --module-content, which is not supported in Lola v0.5.0; provide the subdirectory via the supported #subdirectory=<path> URL fragment (or equivalent supported config such as .lola-req) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflows/quay-bugfix/.claude/scripts/session-setup.sh` at line 48, The
command using the unsupported flag "--module-content" in the invocation of $LOLA
mod add should be changed to supply the subdirectory via the URL fragment or
supported config; remove the "--module-content=\"$subdir\"" argument and instead
append "`#subdirectory`=$subdir" (or other supported fragment) to the "$url"
variable before calling "$LOLA mod add \"$url\" --name \"$name\"" (alternatively
update the repository to include a .lola-req with the subdir), ensuring the call
no longer passes the unsupported flag.
- Fix quay-ticket session-setup.sh validation to use find + exclude pattern instead of ls -A (matches quay-bugfix/quay-cvefix) - Update README/CLAUDE.md descriptions to accurately reflect the fragment-parsing shim approach (lola mod add + lola install) - Fix enhancement-001 Phase 5 example: ln -s → cp (symlinks don't survive hydrate subpath extraction) - Add text language tag to .lola-req code block in README Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Migrates all
.lola-reqfiles andsession-setup.shbootstrap scripts to use Lola v0.5.0's new pip-style URL fragment syntax (#subdirectory=) instead of the custom--module-contentflag..lola-reqfiles:--module-content=plugins/dev→#subdirectory=plugins/devsession-setup.shscripts: Added-a claude-codetolola synccalls; simplified scripts wherelola syncworks directlyName collision workaround
Lola v0.5.0's
lola syncderives module names from the git URL, which causes collisions when multiple.lola-reqentries use different#subdirectory=fragments from the same repository. Workflows needing bothplugins/devandplugins/jira-planning(quay-bugfix, quay-cvefix, quay-ticket) use a fragment-parsing shim that callslola mod add+lola installwith explicit names to avoid this. This will be removable once the upstream issue is resolved.Affected workflows
lola sync -a claude-codelola sync -a claude-codelola sync -a claude-codeTest plan
lola synctest with single subdirectory entry (quay-deploy) — installs correct plugin (2 skills)lola synctest with entries from different repos (konflux-build-debugger) — both plugins installlola syncwith two same-repo entries skips the second (confirms workaround is necessary)🤖 Generated with Claude Code