chore: sync workflow templates - #130
Conversation
Automated sync from stranske/Workflows Template hash: 0ab0029407c1 Changes synced from sync-manifest.yml
stranske
added
sync
📝 WalkthroughWalkthroughTwo independent changes: a new ChangesShared Design System
Workflow SHA Bump
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
GitHub Actions
Failure
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/agents-guard.yml (1)
109-117: 📐 Maintainability & Code Quality | 🟡 MinorDocument the controlled reason for pinning this action SHA, and correct the misleading version comment.
Lines 114 and 183 pin
setup-api-clientto commit62ed0a86b5d57062ac3d04f4519e3998858e2d96(release 1.16.0) without explaining why this specific commit is required. The# v1comment is also misleading—this is actually version 1.16.0, not v1.The coding guideline requires that SHA pins have a documented reason in the code (e.g., "pinned for stability while local action installation is unavailable" or "pinned to v1.16.0 to ensure token contract compatibility"). Since
agents-guard.ymlis a synced workflow managed through.github/sync-manifest.ymlin thestranske/Workflowsrepository, this fix must be applied upstream, not locally.🤖 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 @.github/workflows/agents-guard.yml around lines 109 - 117, The setup-api-client action at lines 114 and 183 is pinned to commit SHA 62ed0a86b5d57062ac3d04f4519e3998858e2d96 without documenting the reason for the pin, and the comment incorrectly labels it as v1 when it is actually version 1.16.0. Add an explanatory comment before the uses statement (such as "pinned to v1.16.0 to ensure token contract compatibility" or another controlled reason that explains why this specific commit is required), and update the version comment from # v1 to # v1.16.0 to accurately reflect the pinned version. Since agents-guard.yml is a synced workflow managed upstream through the stranske/Workflows repository, these changes must be applied to the source workflow in that repository, not to this local copy.Source: Coding guidelines
🤖 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 `@design-system/components.css`:
- Around line 15-16: Add explicit `:focus-visible` pseudo-class styles to all
interactive controls that currently lack keyboard focus indication. For the `.ds
.appbar nav a` selector and the other interactive selectors mentioned at lines
39-40 and 43-48 (inputs/selects and buttons), add `:focus-visible` rules that
apply a visible outline or background change to ensure keyboard users can
clearly see which element has focus. Use CSS properties like outline or a
distinct background color that contrasts well with the surrounding elements and
aligns with the design system's visual hierarchy.
In `@design-system/ds_streamlit.py`:
- Around line 168-170: The humanize_id() function uses split(":")[0] to extract
a segment from a colon-delimited ID, but the comment indicates it should extract
the trailing segment, not the leading one. Change split(":")[0] to
split(":")[-1] to correctly retrieve the last segment of the colon-delimited
string, which aligns with the intended behavior described in the comment.
- Around line 91-95: The st.markdown calls at lines 91-95, 109-122, and 158-162
inject user-provided variables (title, desc, icon, body, action, label) directly
into HTML strings without escaping, creating a security vulnerability. Import
html module and apply html.escape() to each user-provided variable before
embedding them in the f-string HTML content. Additionally, fix the humanize_id()
function (line 165) where split(":")[0] extracts the first segment instead of
the trailing segment as documented; change split(":")[0] to split(":")[-1] to
correctly extract the last colon-delimited segment.
In `@design-system/PRESENTATION_PATTERNS.md`:
- Around line 83-85: The function diagnostics_expander() is listed in the
documented API in PRESENTATION_PATTERNS.md but does not exist in the
implementation file design-system/ds_streamlit.py. To fix this inconsistency,
either implement the diagnostics_expander() function in ds_streamlit.py with
appropriate functionality, or remove the reference to diagnostics_expander()
from the PRESENTATION_PATTERNS.md documentation file. Choose the approach based
on whether this function is intended to be part of the public API or was
accidentally documented.
---
Outside diff comments:
In @.github/workflows/agents-guard.yml:
- Around line 109-117: The setup-api-client action at lines 114 and 183 is
pinned to commit SHA 62ed0a86b5d57062ac3d04f4519e3998858e2d96 without
documenting the reason for the pin, and the comment incorrectly labels it as v1
when it is actually version 1.16.0. Add an explanatory comment before the uses
statement (such as "pinned to v1.16.0 to ensure token contract compatibility" or
another controlled reason that explains why this specific commit is required),
and update the version comment from # v1 to # v1.16.0 to accurately reflect the
pinned version. Since agents-guard.yml is a synced workflow managed upstream
through the stranske/Workflows repository, these changes must be applied to the
source workflow in that repository, not to this local copy.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: eadbb9a8-7192-4fd6-8c3f-ddb6683b24c9
📒 Files selected for processing (6)
.github/workflows/agents-guard.ymldesign-system/PRESENTATION_PATTERNS.mddesign-system/README.mddesign-system/components.cssdesign-system/ds_streamlit.pydesign-system/tokens.css
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Workflows(auto-detected)stranske/Template(auto-detected)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
startup_failurein GitHub Actions workflows with zero jobs indicates GitHub couldn't parse the workflow; check for invalid YAML syntax, conflictingpermissions:blocks onworkflow_callreusable workflows, invalid permission scopes, or circular workflow references
Files:
.github/workflows/agents-guard.yml
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Reference reusable workflows with
@mainby default unless intentionally pinning to an exact commit SHA for a documented reason
Files:
.github/workflows/agents-guard.yml
{.github/workflows/agents-*.yml,.github/workflows/autofix.yml,.github/codex/**,docs/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Synced workflows, prompts, scripts, and consumer docs are managed through .github/sync-manifest.yml in Workflows and should not be edited locally
Files:
.github/workflows/agents-guard.yml
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
Reference reusable workflows with
@mainunless intentionally pinning to an exact commit SHA for a controlled reason.
Files:
.github/workflows/agents-guard.yml
{**/.github/workflows/agents-*.yml,.github/workflows/autofix.yml,.github/codex/**,docs/ops/CONSUMER_REPO_MAINTENANCE.md,docs/keepalive/Agents.md}
📄 CodeRabbit inference engine (AGENTS.md)
Synced workflows, prompts, scripts, and consumer docs (including
agents-*.yml,autofix.yml,.github/codex/prompts, and synced scripts) are managed through.github/sync-manifest.ymlin Workflows and should not be edited locally; fix issues in Workflows instead.
Files:
.github/workflows/agents-guard.yml
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For Manager-Database repository: use Prefect 2.x and import schedules from
prefect.client.schemas.schedules
Files:
design-system/ds_streamlit.py
🪛 markdownlint-cli2 (0.22.1)
design-system/PRESENTATION_PATTERNS.md
[warning] 16-16: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 30-30: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 39-39: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 46-46: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 54-54: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 77-77: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 88-88: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Stylelint (17.13.0)
design-system/tokens.css
[error] 29-29: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
[error] 29-29: Expected "Roboto" to be "roboto" (value-keyword-case)
(value-keyword-case)
[error] 29-29: Expected "Helvetica" to be "helvetica" (value-keyword-case)
(value-keyword-case)
[error] 29-29: Expected "Arial" to be "arial" (value-keyword-case)
(value-keyword-case)
[error] 74-74: Expected "Georgia" to be "georgia" (value-keyword-case)
(value-keyword-case)
🔀 Multi-repo context stranske/Workflows, stranske/Template
I'll explore the linked repositories to understand the source of these changes and how they're consumed across the organization. This will help identify potential breaking changes and cross-boundary issues.
<function_calls>
#!/bin/bash
# Get overview of Workflows repo structure
echo "=== Workflows repo structure ==="
ls -la
echo ""
echo "=== .github/actions directory ==="
ls -la .github/actions/
echo ""
echo "=== design-system files ==="
ls -la design-system/ 2>/dev/null || echo "No design-system found"
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Get overview of Template repo structure
echo "=== Template repo structure ==="
ls -la
echo ""
echo "=== .github directory ==="
ls -la .github/
echo ""
echo "=== design-system files ==="
ls -la design-system/ 2>/dev/null || echo "No design-system found"
</parameter>
</function_calls>
Now let me check the specific action version referenced in the PR and verify the sync mechanism:
<function_calls>
#!/bin/bash
# Check the setup-api-client action
echo "=== setup-api-client action content ==="
cat .github/actions/setup-api-client/action.yml
echo ""
echo "=== Check agents-guard.yml in Workflows ==="
cat .github/workflows/agents-guard.yml | head -50
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check sync-manifest.yml configuration
echo "=== sync-manifest.yml ==="
cat .github/sync-manifest.yml 2>/dev/null || echo "File not found"
echo ""
echo "=== List all workflow files ==="
find . -name "*.yml" -path "*/.github/workflows/*" | head -20
</parameter>
</function_calls>
Let me also search for consumers of the design-system files and check for any breaking changes in the Python/JavaScript utilities:
<function_calls>
#!/bin/bash
# Check if there's documentation about the design-system
echo "=== Design system files in Workflows ==="
find . -path "*/design-system/*" -type f | head -20
echo ""
# Check git log for recent changes to setup-api-client
echo "=== Recent commits to setup-api-client ==="
cd .github/actions/setup-api-client && git log --oneline -5 2>/dev/null || echo "Cannot access git log"
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check what templates are in Template repo
echo "=== Template repo design-system ==="
find . -path "*/design-system/*" -type f | head -20
echo ""
# Check if there are any references to setup-api-client
echo "=== Search for setup-api-client references ==="
grep -r "setup-api-client" . 2>/dev/null | head -20 || true
</parameter>
</function_calls>
Now let me check the actual sync-manifest in the Workflows repo and verify the commit hash referenced:
<function_calls>
#!/bin/bash
# Find sync-manifest.yml
echo "=== Looking for sync-manifest.yml ==="
find . -name "sync-manifest.yml" -o -name "sync*.yml" 2>/dev/null
echo ""
# Check if there's a .sync file or README
ls -la | grep -i sync
echo ""
# Get the current commit hash
echo "=== Current commit ==="
git rev-parse HEAD 2>/dev/null || echo "Cannot get commit"
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check for sync-manifest and its contents
echo "=== Checking for sync configuration ==="
cat .github/sync-manifest.yml 2>/dev/null || find . -name "*sync*" -type f 2>/dev/null | head -10
echo ""
# Check if Template serves as a template repo
echo "=== Template metadata ==="
cat README.md 2>/dev/null | head -30
</parameter>
</function_calls>
Let me verify the setup-api-client action version and check for any API contract changes:
<function_calls>
#!/bin/bash
# Find all references to setup-api-client in workflows
echo "=== References to setup-api-client ==="
grep -r "setup-api-client" .github/workflows/ 2>/dev/null | head -20
echo ""
# Check the action.yml more thoroughly
echo "=== Full setup-api-client action.yml ==="
cat .github/actions/setup-api-client/action.yml 2>/dev/null || echo "File not found"
echo ""
# Check if there's a script behind the action
echo "=== setup-api-client script ==="
ls -la .github/actions/setup-api-client/
cat .github/actions/setup-api-client/setup.sh 2>/dev/null || cat .github/actions/setup-api-client/index.js 2>/dev/null || echo "No script found"
</parameter>
</function_calls>
```shell:stranske/Workflows
#!/bin/bash
# Check Python scripts for breaking changes
echo "=== Python utility files ==="
find . -name "*.py" -path "*/.github/*" | head -20
echo ""
# Look at a sample Python file for API changes
echo "=== Sample Python utility ==="
ls -la .github/scripts/ 2>/dev/null || ls -la .github/python/ 2>/dev/null || echo "Checking other locations"
</parameter>
</function_calls>
Let me check if there are any breaking changes in the design-system or if there are version constraints:
<function_calls>
#!/bin/bash
# Check for design-system files and their versions
echo "=== Checking design-system usage ==="
find . -type f \( -name "ds_streamlit.py" -o -name "tokens.css" -o -name "components.css" \) -exec echo "File: {}" \; -exec head -20 {} \;
echo ""
# Check for any version pinning or imports
echo "=== Checking imports/requirements ==="
grep -r "ds_streamlit\|tokens\|components" . 2>/dev/null | grep -v ".git" | head -20 || true
</parameter>
</function_calls>
Let me verify the commit hash and understand the sync direction:
<function_calls>
#!/bin/bash
# Get commit information
echo "=== Checking commit hash 64a8012fcf61ffb010aed2c2f3e0e97f8766826c ==="
git log --oneline | grep "64a8012" || echo "Hash not in recent history"
echo ""
git rev-parse HEAD
echo ""
# Check recent commits
echo "=== Recent commits ==="
git log --oneline -10
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check if this is a template/consumer repo
echo "=== Checking repo purpose ==="
cat README.md 2>/dev/null | head -50
echo ""
# Check for references to stranske/Workflows
echo "=== References to stranske/Workflows ==="
grep -r "stranske/Workflows" . 2>/dev/null || echo "No direct references"
</parameter>
</function_calls>
Now let me check the design-system implementation in both repos to identify potential compatibility issues:
<function_calls>
#!/bin/bash
# Check design-system completeness
echo "=== Design system files in Workflows ==="
find . -path "*/design-system/*" -type f 2>/dev/null | sort
echo ""
# Check if tokens.css and components.css are properly formatted
if [ -f "design-system/tokens.css" ]; then
echo "=== tokens.css stats ==="
wc -l design-system/tokens.css
echo ""
head -30 design-system/tokens.css
fi
echo ""
if [ -f "design-system/ds_streamlit.py" ]; then
echo "=== ds_streamlit.py function signatures ==="
grep -E "^def " design-system/ds_streamlit.py
fi
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check if Template already has design-system
echo "=== Design system in Template ==="
find . -path "*/design-system/*" -type f 2>/dev/null | sort || echo "No design-system found"
echo ""
# Check file structure
tree -L 3 -I 'node_modules|.git' 2>/dev/null || find . -maxdepth 3 -type f -name "*.yml" -o -name "*.md" -o -name "*.py" -o -name "*.css" | head -30
</parameter>
</function_calls>
Let me check for any consumers of the synced Python/JavaScript utilities:
<function_calls>
#!/bin/bash
# Search for imports/usage of synced Python utilities
echo "=== Python script usage ==="
grep -r "from .github.scripts\|from .github.python\|import.*sync\|import.*aggregate" . 2>/dev/null | grep -v ".git" | head -30 || true
echo ""
# Check for JavaScript script usage
echo "=== JavaScript script invocations ==="
grep -r "node .*\.js\|require.*github" . 2>/dev/null | grep -v ".git" | grep -v node_modules | head -20 || true
</parameter>
</function_calls>
Let me look at the actual agents-guard.yml to understand the setup-api-client update:
<function_calls>
#!/bin/bash
# Check agents-guard.yml workflow
echo "=== agents-guard.yml complete content ==="
cat .github/workflows/agents-guard.yml 2>/dev/null | head -100
echo ""
echo "=== setup-api-client action.yml ==="
cat .github/actions/setup-api-client/action.yml 2>/dev/null
</parameter>
</function_calls>
Let me search across both repos to understand how the design-system is intended to be consumed:
<function_calls>
```shell:stranske/Workflows,stranske/Template
#!/bin/bash
# Search for design-system integration instructions
echo "=== DESIGN_SYSTEM or PRESENTATION files ==="
find . -name "*DESIGN*" -o -name "*design*" -o -name "*PRESENTATION*" 2>/dev/null | grep -v ".git" | head -20
echo ""
# Check if there's a sync guide
echo "=== Sync or integration guides ==="
find . -name "*SYNC*" -o -name "*integration*" 2>/dev/null | grep -v ".git" | head -10
</parameter>
</function_calls>
Based on my exploration, let me check specifically if there are any issues with the design-system imports or compatibility:
<function_calls>
#!/bin/bash
# Verify design-system file quality and completeness
echo "=== Checking design-system/ds_streamlit.py imports ==="
head -30 design-system/ds_streamlit.py 2>/dev/null || echo "File not found"
echo ""
echo "=== Check for required dependencies in ds_streamlit.py ==="
grep -E "^import |^from " design-system/ds_streamlit.py 2>/dev/null || echo "No imports found"
echo ""
# Verify tokens.css structure
echo "=== Verify tokens.css has expected CSS vars ==="
grep -c "var(--" design-system/tokens.css 2>/dev/null || echo "File not found"
echo ""
echo "=== Verify components.css imports ==="
head -10 design-system/components.css 2>/dev/null || echo "File not found"
</parameter>
</function_calls>
Let me now look for any consumers in both repos to see who depends on these files:
<function_calls>
#!/bin/bash
# Check for Streamlit app files that might use ds_streamlit
echo "=== Streamlit app files (*.py with 'streamlit' imports) ==="
find . -name "app.py" -o -name "*streamlit*.py" 2>/dev/null | grep -v ".git" | head -20
echo ""
# Check for HTML/CSS files that might use tokens/components
echo "=== HTML files that might reference design-system ==="
find . -name "*.html" -o -name "index.html" 2>/dev/null | grep -v ".git" | head -20
</parameter>
</function_calls>
Let me search more specifically for the design-system references in workflow outputs or documentation:
<function_calls>
#!/bin/bash
# Check PRESENTATION_PATTERNS.md content to understand design system rollout
echo "=== PRESENTATION_PATTERNS.md content ==="
cat design-system/PRESENTATION_PATTERNS.md 2>/dev/null || echo "File not found"
echo ""
echo "=== design-system README.md ==="
cat design-system/README.md 2>/dev/null || head -50 design-system/README.md 2>/dev/null
</parameter>
</function_calls>
```shell:stranske/Template
#!/bin/bash
# Check if Template has any references to design-system or if it's the target
echo "=== Template repo design-system status ==="
ls -la design-system/ 2>/dev/null || echo "design-system directory not present"
echo ""
# Check the sync-manifest
echo "=== Check sync configuration ==="
cat .github/sync-manifest.yml 2>/dev/null || echo "sync-manifest not found"
echo ""
# Check if Template acts as consumer
echo "=== Check if synced files are present ==="
ls -la templates/consumer-repo/design-system/ 2>/dev/null || echo "consumer templates not present"
</parameter>
</function_calls>
Let me check what the sync mechanism looks like and verify the commit hash referenced in the PR:
<function_calls>
#!/bin/bash
# Check git history for the specific commit
echo "=== Looking for commit 64a8012 ==="
git log --all --oneline | grep "64a8012" || echo "Not in recent history, checking date"
echo ""
# Get current HEAD
echo "=== Current Workflows HEAD ==="
git log --oneline -1
echo ""
# Check if 64a8012 is an ancestor
git log --oneline -30
</parameter>
</function_calls>
```shell:stranske/Workflows
#!/bin/bash
# List all design-system files to confirm what's being synced
echo "=== All files in design-system directory ==="
find design-system -type f 2>/dev/null | sort
echo ""
# Check file sizes to ensure they're complete
echo "=== File details ==="
ls -lh design-system/ 2>/dev/null
</parameter>
</function_calls>
Based on my exploration of the linked repositories, I can now provide a comprehensive assessment:
## Linked repositories findings
### stranske/Workflows [::stranske/Workflows::]
**Design System Assets (New in this sync):**
The source repository contains comprehensive new design-system files being synced:
- `design-system/tokens.css` - Defines CSS custom properties with themes (`.theme-air`, `.theme-paper`) and density axis (`.density-compact`) [::stranske/Workflows::]
- `design-system/components.css` - Component styling layer with `.ds` scoped styles for UI primitives, including typography, panels, appbars, KPIs, grids, tables, forms, buttons, notices, badges, and loading skeletons [::stranske/Workflows::]
- `design-system/ds_streamlit.py` - New Streamlit helper module with public API including:
- `inject_theme()` - Injects CSS theme
- `empty_state()` - Renders empty states with optional CTAs
- `notice()` / `error()` - Render semantic notice containers
- `translate_error()` - Converts exceptions to human-readable messages
- `humanize_id()` - Converts internal IDs to user-safe labels
- `dev_note()` - Logs diagnostic messages [::stranske/Workflows::]
- `design-system/PRESENTATION_PATTERNS.md` - Documents 6 presentation patterns (P1–P6) for standardizing fleet findings [::stranske/Workflows::]
- `design-system/README.md` - Design system usage guide with token/component architecture and rollout guidance [::stranske/Workflows::]
**setup-api-client Action Update:**
The PR updates `.github/workflows/agents-guard.yml` to pin `stranske/Workflows/.github/actions/setup-api-client` to a newer commit SHA in the fallback workflow steps. This action is defined in `.github/actions/setup-api-client/action.yml` and provides API client setup functionality for workflows [::stranske/Workflows::]
### stranske/Template [::stranske/Template::]
**No conflicting design-system files found** — Template repo does not currently contain a `design-system/` directory. The design-system assets from Workflows are intended to be synced into consumer repos (like `Fine-Art-Archive`) via the sync mechanism, with guidance in the design-system README about graduating into `Workflows/templates/consumer-repo/design-system/` [::stranske/Template::]
**Sync mechanism confirmation** — The PR describes syncing from commit `64a8012fcf61ffb010aed2c2f3e0e97f8766826c` of `stranske/Workflows`. The sync respects a `create_only` mode to avoid overwriting local customizations (e.g., `pr-00-gate.yml`, `ci.yml`, `renovate.json` are skipped if they exist locally) [::stranske/Template::]
### Cross-Repo Impact Assessment
**Breaking Changes:** None identified. The design-system is purely additive:
- New CSS files are scoped under `.ds` to avoid leaking to host app styles [::stranske/Workflows::]
- New Python module (`ds_streamlit.py`) is imported by consumers, not injected globally
- setup-api-client action maintains backward compatibility (fallback mechanism)
**Compatibility Considerations:**
- Consumers must explicitly import and call `ds_streamlit` functions; no breaking changes to existing workflows
- CSS tokens are opt-in (require explicit theme/density class application on `<body>`) [::stranske/Workflows::]
- The sync is tracked via `.github/sync-manifest.yml` to allow repos to version-control their synced state [::stranske/Template::]
</details>
</details>
<details>
<summary>🔇 Additional comments (3)</summary><blockquote>
<details>
<summary>design-system/tokens.css (1)</summary><blockquote>
`1-77`: LGTM!
<!-- cr-comment:v1:840bf2ef753dd7ae5f02b1d7 -->
</blockquote></details>
<details>
<summary>design-system/README.md (1)</summary><blockquote>
`1-52`: LGTM!
<!-- cr-comment:v1:a8e659220eccf4ccc0d83f6d -->
</blockquote></details>
<details>
<summary>.github/workflows/agents-guard.yml (1)</summary><blockquote>
`178-186`: LGTM!
<!-- cr-comment:v1:a948a4c06e8b8c48744bbd38 -->
</blockquote></details>
</blockquote></details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
| .ds .appbar nav a { font-size:var(--fs-sm); text-decoration:none; color:var(--muted); padding:5px 10px; border-radius:var(--radius-sm); cursor:pointer; } | ||
| .ds .appbar nav a.active { color:var(--accent); background:var(--accent-weak); font-weight:600; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add visible keyboard focus styles for interactive controls.
Links, inputs/selects, and buttons currently have no explicit :focus-visible styling, so keyboard users can lose location in navigation/forms.
Suggested patch
.ds .appbar nav a { font-size:var(--fs-sm); text-decoration:none; color:var(--muted); padding:5px 10px; border-radius:var(--radius-sm); cursor:pointer; }
.ds .appbar nav a.active { color:var(--accent); background:var(--accent-weak); font-weight:600; }
+.ds .appbar nav a:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}
@@
.ds input[type=text], .ds select { width:100%; font:inherit; font-size:var(--fs-base); padding:var(--control-pad-y) var(--control-pad-x); color:var(--text); background:var(--input-bg); border:1px solid var(--border); border-radius:var(--input-radius); }
.ds input[type=range] { width:100%; accent-color:var(--accent); }
+.ds input[type=text]:focus-visible,
+.ds select:focus-visible,
+.ds input[type=range]:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}
@@
.ds button { font:inherit; font-size:var(--fs-sm); font-weight:600; padding:var(--control-pad-y) 15px; border-radius:var(--btn-radius); cursor:pointer; border:1px solid transparent; }
+.ds button:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}Also applies to: 39-40, 43-48
🤖 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 `@design-system/components.css` around lines 15 - 16, Add explicit
`:focus-visible` pseudo-class styles to all interactive controls that currently
lack keyboard focus indication. For the `.ds .appbar nav a` selector and the
other interactive selectors mentioned at lines 39-40 and 43-48 (inputs/selects
and buttons), add `:focus-visible` rules that apply a visible outline or
background change to ensure keyboard users can clearly see which element has
focus. Use CSS properties like outline or a distinct background color that
contrasts well with the surrounding elements and aligns with the design system's
visual hierarchy.
| st.markdown( | ||
| f"<div class='ds-empty'><div style='font-size:22px;opacity:.6'>{icon}</div>" | ||
| f"<div class='t'>{title}</div><div class='d'>{desc}</div></div>", | ||
| unsafe_allow_html=True, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical
🧩 Analysis chain
🌐 Web query:
Does Streamlit's st.markdown with unsafe_allow_html=True sanitize HTML, and what are the official recommendations for escaping untrusted strings before rendering?
💡 Result:
Streamlit's st.markdown(..., unsafe_allow_html=True) does not perform sanitization on the provided HTML string [1][2]. The parameter is explicitly named "unsafe" to warn developers that they are responsible for ensuring the content is secure [1]. By enabling this flag, you allow arbitrary HTML to be rendered in the browser, which exposes the application to Cross-Site Scripting (XSS) attacks if the input string contains untrusted user data [1][3][2]. Official recommendations and best practices: 1. Avoid Untrusted Input: The primary recommendation is to never pass untrusted or user-generated input into st.markdown with unsafe_allow_html=True [2]. If you must render dynamic content, you are responsible for sanitizing it yourself before passing it to Streamlit [3]. 2. Prefer Stricter Alternatives: For cases where you need to render HTML, Streamlit now provides the st.html function [4][5]. Unlike st.markdown, st.html sanitizes the input using DOMPurify by default [4][5][6]. However, even with sanitization, inserting HTML remains a developer risk; it should still be used sparingly and never with untrusted input [4][6]. 3. Manual Escaping: If you are working with user-generated text that should not be interpreted as HTML or Markdown, do not use HTML-rendering features at all. If you need to escape Markdown-specific characters (such as *, _, or #) to prevent them from being rendered as formatting, you must handle this manually in Python before the string reaches the Streamlit component [7]. 4. Security Context: Treat any feature that allows custom HTML injection as an "escape hatch" rather than a standard tool for content display [2]. Because it is difficult to write and maintain secure HTML, the best defense is to minimize its usage entirely [1][2].
Citations:
- 1: https://discuss.streamlit.io/t/why-is-using-html-unsafe/4863
- 2: Allow HTML (again!) in st.write/st.markdown — but with kwarg toggle streamlit/streamlit#152
- 3: https://discuss.streamlit.io/t/every-security-aspects-of-allow-unsafe-html/66498
- 4: https://docs.streamlit.io/develop/api-reference/text/st.html
- 5: Feature:
st.htmlstreamlit/streamlit#8366 - 6: https://tutorial-3400-python.streamlit.app/text_elements
- 7: https://discuss.streamlit.io/t/how-to-sanitize-user-input-for-markdown/828
🏁 Script executed:
#!/bin/bash
# Check if the file exists and inspect the vulnerable functions
echo "=== Check design-system/ds_streamlit.py existence ==="
if [ -f "design-system/ds_streamlit.py" ]; then
echo "File found. Inspecting content..."
echo ""
echo "=== Full file content ==="
cat design-system/ds_streamlit.py
else
echo "File not found. Listing design-system directory:"
ls -la design-system/ 2>/dev/null || echo "design-system directory not found"
fi
</parameter>
</function_calls>Repository: stranske/Fine-Art-Archive
Length of output: 6821
Escape user-provided text before rendering with unsafe_allow_html=True.
title, desc, icon, body, action, and label are injected directly into HTML without escaping. Streamlit's st.markdown(..., unsafe_allow_html=True) does not sanitize HTML—the parameter name reflects this intentionally. User-provided input must be manually escaped using html.escape() to prevent markup injection attacks.
Suggested patch
import logging
+import html
from collections.abc import Callable, Mapping
from typing import Any
@@
st.markdown(
- f"<div class='ds-empty'><div style='font-size:22px;opacity:.6'>{icon}</div>"
- f"<div class='t'>{title}</div><div class='d'>{desc}</div></div>",
+ f"<div class='ds-empty'><div style='font-size:22px;opacity:.6'>{html.escape(icon)}</div>"
+ f"<div class='t'>{html.escape(title)}</div><div class='d'>{html.escape(desc)}</div></div>",
unsafe_allow_html=True,
)
@@
- head = f"<strong>{title}</strong><br>" if title else ""
- act = f"<div style='margin-top:6px'>{action}</div>" if action else ""
+ head = f"<strong>{html.escape(title)}</strong><br>" if title else ""
+ act = f"<div style='margin-top:6px'>{html.escape(action)}</div>" if action else ""
st.markdown(
f"<div class='ds-notice' style='background:{bg};border-color:{color}33'>"
f"<span class='ic' style='color:{color}'>{ic}</span>"
- f"<div>{head}{body}{act}</div></div>",
+ f"<div>{head}{html.escape(body)}{act}</div></div>",
unsafe_allow_html=True,
)
@@
def availability_badge(label: str) -> str:
@@
- return f"<span class='ds-badge'>{label}</span>"
+ return f"<span class='ds-badge'>{html.escape(label)}</span>"Also applies to: 109–122, 158–162
Fix humanize_id() to extract the trailing segment, not the first.
Line 165 uses split(":")[0] which extracts the first segment before the colon, contradicting the docstring (line 160) that documents extracting the "trailing human-ish segment". For colon-delimited IDs (e.g., namespace:id:hash), this will return the wrong label.
def humanize_id(raw: str, mapping: Mapping[str, str] | None = None) -> str:
"""P6 — decode an internal id to a human label; never show raw keys."""
if mapping and raw in mapping:
return mapping[raw]
- tail = str(raw).replace("_", " ").split(":")[0].strip()
+ tail = str(raw).replace("_", " ").split(":")[-1].strip()
return tail or "item"🤖 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 `@design-system/ds_streamlit.py` around lines 91 - 95, The st.markdown calls at
lines 91-95, 109-122, and 158-162 inject user-provided variables (title, desc,
icon, body, action, label) directly into HTML strings without escaping, creating
a security vulnerability. Import html module and apply html.escape() to each
user-provided variable before embedding them in the f-string HTML content.
Additionally, fix the humanize_id() function (line 165) where split(":")[0]
extracts the first segment instead of the trailing segment as documented; change
split(":")[0] to split(":")[-1] to correctly extract the last colon-delimited
segment.
| # Best-effort: take a trailing human-ish segment, strip hashes. | ||
| tail = str(raw).replace("_", " ").split(":")[0].strip() | ||
| return tail or "item" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
humanize_id() picks the wrong segment for colon-delimited IDs.
The comment says “take a trailing human-ish segment,” but split(":")[0] returns the leading segment.
Suggested patch
- tail = str(raw).replace("_", " ").split(":")[0].strip()
+ tail = str(raw).replace("_", " ").split(":")[-1].strip()🤖 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 `@design-system/ds_streamlit.py` around lines 168 - 170, The humanize_id()
function uses split(":")[0] to extract a segment from a colon-delimited ID, but
the comment indicates it should extract the trailing segment, not the leading
one. Change split(":")[0] to split(":")[-1] to correctly retrieve the last
segment of the colon-delimited string, which aligns with the intended behavior
described in the comment.
| - `dev_note(msg)` → logging; `diagnostics_expander()` (P4) | ||
| - `availability_badge(label)` (P5) | ||
| - `humanize_id(raw, mapping)` (P6) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Documented Streamlit API includes a non-existent function.
diagnostics_expander() is documented here, but it is not implemented in design-system/ds_streamlit.py. Please either add the function or remove it from the published API list to avoid broken adoption paths.
🤖 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 `@design-system/PRESENTATION_PATTERNS.md` around lines 83 - 85, The function
diagnostics_expander() is listed in the documented API in
PRESENTATION_PATTERNS.md but does not exist in the implementation file
design-system/ds_streamlit.py. To fix this inconsistency, either implement the
diagnostics_expander() function in ds_streamlit.py with appropriate
functionality, or remove the reference to diagnostics_expander() from the
PRESENTATION_PATTERNS.md documentation file. Choose the approach based on
whether this function is intended to be part of the public API or was
accidentally documented.
This branch was previously deployed
1 participant
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
64a8012fcf61ffb010aed2c2f3e0e97f8766826cTemplate hash:
0ab0029407c1Sync branch:
sync/workflows-0ab0029407c1Consumer repo:
stranske/Fine-Art-ArchiveManifest:
.github/sync-manifest.ymlSummary by CodeRabbit
New Features
Documentation
Chores