Skip to content

Drop a Bare Sequence-Item Block Scalar's Body Too - #1005

Merged
ptr727 merged 2 commits into
developfrom
fix-949-followup-bare-sequence-scalar
Aug 25, 2026
Merged

Drop a Bare Sequence-Item Block Scalar's Body Too#1005
ptr727 merged 2 commits into
developfrom
fix-949-followup-bare-sequence-scalar

Conversation

@ptr727

@ptr727 ptr727 commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Follow-up to #1003 (issue #949), found by qodo-code-review on the develop -> main promotion PR #1004, which cannot carry the fix directly since its diff is develop against main.

What happened

_code_view()'s block-scalar recognition required a mapping key before the |/> indicator, so a keyed step (- run: |) was recognized but a bare sequence item with no key at all (- |, e.g. a matrix string) was not, leaving its body searchable and the requireTokensInJob/forbidTokensInJob mitigation bypassable the same way the original issue describes.

Fix

Recognizes a bare - |/- > too. Its boundary is the dash's own column rather than a key past it, since there is no key: verified against a real YAML parser (content one column past the dash parses, content at or before the dash's column is a parse error).

Verification

Ran locally from the repo root: python3 spec/audit.py --selftest (SELFTEST PASS, including the new case), uvx ruff@latest check/format --check, uvx mypy@latest, python3 scripts/prose_lint.py (default set), python3 scripts/repo_gate.py.

Summary by CodeRabbit

  • Bug Fixes

    • Improved code view filtering for bare block scalars in sequence items, including literal and folded formats.
    • Correctly removes multiline scalar content while preserving indentation behavior for keyed sequence items.
  • Tests

    • Added coverage for filtering matrix-style block scalars and validating indentation boundaries.

A step's `- run: |` is keyed, so _code_view() recognized it, but a
bare sequence item (`- |`, e.g. a matrix string) has no key at all
and was not recognized, leaving its body searchable and the
requireTokensInJob/forbidTokensInJob mitigation bypassable the same
way as the keyed case.

The boundary for a bare scalar is the dash's own column, not a key
past it, verified against a real YAML parser: content one column past
the dash parses, content at or before the dash's column does not.
Adds the matching regression case.

Found by qodo-code-review on PR #1004.
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Exclude Bare Sequence Block Scalars from Workflow Token Audits

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Excludes bare YAML sequence block-scalar bodies from workflow contract token searches.
• Preserves dash-column dedent boundaries and adds targeted regression self-test coverage.
Diagram

graph TD
  A["Workflow YAML"] -->|split jobs| B["Job Blocks"] -->|filter structure| C["Code View"] -->|search tokens| D["Contract Checks"]
Loading
High-Level Assessment

The targeted scanner extension is the best fit because it preserves the audit's text-oriented, standard-library-only design and existing keyed-scalar behavior. Replacing this path with a full YAML parser would add dependency and source-preservation complexity disproportionate to the narrowly defined syntax gap.

Files changed (1) +16 / -3

Bug fix (1) +16 / -3
audit.pyFilter bare sequence-item block-scalar bodies +16/-3

Filter bare sequence-item block-scalar bodies

• Adds recognition for bare '- |' and '- >' YAML sequence items, using the dash column as their outdent boundary so scalar contents cannot satisfy or trigger workflow token contracts. Extends '_code_view()' self-tests with a matrix-style regression case.

spec/audit.py

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 42db2d33-54cf-4290-a3df-4bc27d53bc6a

📥 Commits

Reviewing files that changed from the base of the PR and between 36cc418 and fa92105.

📒 Files selected for processing (1)
  • spec/audit.py

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

_code_view now filters bodies from bare - | and - > YAML sequence-item scalars while preserving keyed scalar behavior. A self-test verifies indicator retention and body removal.

Changes

YAML scalar filtering

Layer / File(s) Summary
Bare scalar detection and filtering
spec/audit.py
_code_view tracks dash and key columns separately. Bare sequence-item block scalars use the dash column as the dedentation boundary. Keyed sequence-item scalars retain their existing behavior. The self-test covers body filtering and indicator retention.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to fa921

This is a localized parser-recognition fix for bare sequence-item block scalars, with no actionable merge-blocking risk remaining after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: removing the body of a bare sequence-item block scalar. It is concise and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-949-followup-bare-sequence-scalar

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

qodo-code-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Comment wraps one sentence ✓ Resolved 📜 Skill insight ✧ Quality
Description
The new test comment wraps a single sentence across two comment lines. This violates the requirement
that each comment line contain a complete sentence rather than a mid-sentence continuation.
Code

spec/audit.py[R2973-2974]

+            # A bare sequence item (a matrix string) has no key at all, so its own boundary is the
+            # dash's column: content one past the dash parses, content at or before it does not.
Relevance

●●● Strong

Recent same-file precedents accepted one-sentence-per-line comment fixes, including an exact
analogous wrapped comment.

PR-#978
PR-#901

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2826725 prohibits comments wrapped in the middle of a sentence. The added comment at
spec/audit.py[2973-2974] continues the from the first line with dash's column on the second
line.

spec/audit.py[2973-2974]
Skill: comment-and-doc-style

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 comment wraps one sentence across two lines, ending the first line mid-sentence.

## Issue Context
PR Compliance ID 2826725 requires one complete sentence per comment line and prohibits wrapped sentences.

## Fix Focus Areas
- spec/audit.py[2973-2974]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 70 rules
✅ Skills: 5 invoked
  comment-and-doc-style
  dotnet-codestyle
  python-codestyle
  shell-codestyle
  workflow-ci-contract
Review mode: ⚖️ Balanced: This is a localized but security-sensitive behavioral change to token-mitigation code, so it warrants a complete careful review rather than a light pass.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread spec/audit.py Outdated
Found by qodo-code-review on PR #1005.
@ptr727
ptr727 merged commit 2b2025e into develop Aug 25, 2026
8 checks passed
@ptr727
ptr727 deleted the fix-949-followup-bare-sequence-scalar branch August 25, 2026 20:11
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.

1 participant