feat: 设计文档 surface audit + DuckPR docs-sync（#28） - #32
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds deterministic design-doc auditing for backend, frontend, migration, event, and middleware surfaces. It adds mappings, snapshots, classifiers, tests, local commands, CI workflows, a docs-sync skill, and restricted PR documentation synchronization. ChangesDesign Doc Audit and Sync
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant AuditWorkflow
participant AuditCLI
participant DocsSyncWorkflow
participant PullfrogAction
PullRequest->>AuditWorkflow: trigger PR audit
AuditWorkflow->>AuditCLI: audit and classify changed files
AuditCLI-->>AuditWorkflow: return findings and verdict
AuditWorkflow->>PullRequest: upsert audit comment
PullRequest->>DocsSyncWorkflow: dispatch or trigger docs sync
DocsSyncWorkflow->>AuditCLI: prepare audit and prompt inputs
DocsSyncWorkflow->>PullfrogAction: run restricted docs-sync action
PullfrogAction->>PullRequest: update documentation branch and report audit results
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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. Comment |
![duckpr[bot]](https://avatars.githubusercontent.com/in/4166808?s=60&v=4){kind=small}
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
ℹ️ 没有阻塞性问题，仅有一处 minor 鲁棒性建议。
Reviewed changes — 新增设计文档 surface audit 工具链与手动 DuckPR docs-sync 工作流。
- 新增
scripts/docs-audit/audit_design_docs.py，从internal/api/server.go、internal/*包、internal/db/migrations/*.sql和web/src/app/router.tsx提取代码 surface。 - 新增
surface_map.md与surface_snapshot.json作为设计文档覆盖映射与基线。 - 新增
.github/workflows/design-doc-audit.yml：对 coverage finding 软提醒，对 extraction/integrity 失败硬失败。 - 新增
.github/workflows/duckpr-docs-sync.yml与.agents/skills/docs-sync/SKILL.md，支持手动触发对 PR 进行设计文档同步。 - 在
justfile中提供docs-audit*快捷命令。
anthropic/glm-5.2 ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/docs-audit/audit_design_docs.py (2)
440-468: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
result_to_jsonduplicates the finding serialization logic.The dict comprehension for
result.findings(lines 441-450) and the one fordiff_findings(lines 453-461) are identical. Extract a helper to reduce duplication.♻️ Proposed refactor
def result_to_json(result: AuditResult, diff_findings: list[Finding] | None = None) -> dict[str, Any]: - findings = [ - { - "surface_type": f.surface_type, - "surface_id": f.surface_id, - "kind": f.kind, - "severity": f.severity, - "message": f.message, - } - for f in result.findings - ] - if diff_findings: - findings.extend( - { - "surface_type": f.surface_type, - "surface_id": f.surface_id, - "kind": f.kind, - "severity": f.severity, - "message": f.message, - } - for f in diff_findings - ) + def _finding_to_dict(f: Finding) -> dict[str, Any]: + return { + "surface_type": f.surface_type, + "surface_id": f.surface_id, + "kind": f.kind, + "severity": f.severity, + "message": f.message, + } + + findings = [_finding_to_dict(f) for f in result.findings] + if diff_findings: + findings.extend(_finding_to_dict(f) for f in diff_findings) return { "extracted": result.extracted, "accounting": result.accounting,🤖 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 `@scripts/docs-audit/audit_design_docs.py` around lines 440 - 468, The finding serialization logic in result_to_json is duplicated for result.findings and diff_findings. Extract the repeated dict construction into a small helper (for example, a local serializer for Finding) and use it in both the main findings list comprehension and the diff_findings extension so the structure is defined in one place and stays consistent.
365-402: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
diff_snapshotsshould validateschema_versionbefore diffing.The function reads
old.get("surfaces", {})without checkingold.get("schema_version")againstSNAPSHOT_SCHEMA_VERSION. If the snapshot schema changes in a breaking way, the diff would silently produce incorrect results.🛡️ Proposed fix
def diff_snapshots(old: dict[str, Any], new: dict[str, Any]) -> list[Finding]: findings: list[Finding] = [] + old_version = old.get("schema_version") + if old_version is not None and old_version != new.get("schema_version"): + findings.append( + Finding( + "diff", + "schema", + "schema_mismatch", + f"snapshot schema version mismatch: old={old_version}, new={new.get('schema_version')} — re-run --update-snapshot", + ) + ) + return findings old_surfaces = old.get("surfaces", {}) new_surfaces = new.get("surfaces", {})🤖 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 `@scripts/docs-audit/audit_design_docs.py` around lines 365 - 402, diff_snapshots currently diffs snapshot contents without first verifying the snapshot format, so add a schema_version guard before reading surfaces. Update diff_snapshots to compare old.get("schema_version") and new.get("schema_version") against SNAPSHOT_SCHEMA_VERSION and handle mismatches explicitly (for example by failing fast or emitting a clear finding) before any set/diff logic runs. Keep the existing surface_type loop and Finding creation unchanged once the schema check passes.scripts/docs-audit/test_audit_design_docs.py (1)
162-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
unittest.mock.patch.dictinstead of direct module mutation.The tests directly mutate
mod.EXTRACTION_FLOORSand restore viatry/finally. This works in sequential unittest but is fragile if tests ever run concurrently.unittest.mock.patch.dictprovides automatic restoration and is more idiomatic.♻️ Proposed refactor
- import audit_design_docs as mod - - old = dict(mod.EXTRACTION_FLOORS) - try: - mod.EXTRACTION_FLOORS.update({"api_mounts": 1, "packages": 1, "migrations": 1, "fe_routes": 1}) - result = audit_coverage(root, map_path) - finally: - mod.EXTRACTION_FLOORS.clear() - mod.EXTRACTION_FLOORS.update(old) + from unittest.mock import patch + + with patch.dict( + "audit_design_docs.EXTRACTION_FLOORS", + {"api_mounts": 1, "packages": 1, "migrations": 1, "fe_routes": 1}, + clear=True, + ): + result = audit_coverage(root, map_path)Also applies to: 195-203
🤖 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 `@scripts/docs-audit/test_audit_design_docs.py` around lines 162 - 170, The tests are directly mutating audit_design_docs.EXTRACTION_FLOORS and restoring it manually, which is fragile and should be replaced with a scoped patch. Update the affected test cases in test_audit_design_docs.py to use unittest.mock.patch.dict around EXTRACTION_FLOORS instead of the current old/try/finally clear/update pattern, so the original mapping is automatically restored after each test while keeping the audit_coverage assertions unchanged..github/workflows/design-doc-audit.yml (1)
26-26: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSet
persist-credentials: falsefor consistency with the sibling workflow.The
duckpr-docs-sync.ymlworkflow setspersist-credentials: falseon both checkout steps, but this one doesn't. While this workflow is read-only (no push, no artifact upload), aligning the practice avoids leaving the GITHUB_TOKEN in.git/configunnecessarily.🔒 Suggested change
- uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/design-doc-audit.yml at line 26, The checkout step in the design-doc audit workflow is missing the same credential handling used in the sibling workflow. Update the actions/checkout usage in this workflow to set persist-credentials to false, matching the pattern already used in duckpr-docs-sync.yml and keeping GITHUB_TOKEN out of .git/config unnecessarily.Source: Linters/SAST tools
🤖 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 @.github/workflows/duckpr-docs-sync.yml:
- Around line 149-166: The workflow’s private Pullfrog docs-sync step has an
incomplete API key fallback: `ANTHROPIC_API_KEY` only reads
`secrets.ANTHROPIC_API_KEY`, unlike `LLM_API_KEY` and `ANTHROPIC_AUTH_TOKEN`
which already mirror the full secret chain. Update the `env` block in the `Run
private Pullfrog docs-sync` step so `ANTHROPIC_API_KEY` uses the same fallback
sequence as the other auth variables, keeping the runtime key populated
regardless of which secret name is configured.
---
Nitpick comments:
In @.github/workflows/design-doc-audit.yml:
- Line 26: The checkout step in the design-doc audit workflow is missing the
same credential handling used in the sibling workflow. Update the
actions/checkout usage in this workflow to set persist-credentials to false,
matching the pattern already used in duckpr-docs-sync.yml and keeping
GITHUB_TOKEN out of .git/config unnecessarily.
In `@scripts/docs-audit/audit_design_docs.py`:
- Around line 440-468: The finding serialization logic in result_to_json is
duplicated for result.findings and diff_findings. Extract the repeated dict
construction into a small helper (for example, a local serializer for Finding)
and use it in both the main findings list comprehension and the diff_findings
extension so the structure is defined in one place and stays consistent.
- Around line 365-402: diff_snapshots currently diffs snapshot contents without
first verifying the snapshot format, so add a schema_version guard before
reading surfaces. Update diff_snapshots to compare old.get("schema_version") and
new.get("schema_version") against SNAPSHOT_SCHEMA_VERSION and handle mismatches
explicitly (for example by failing fast or emitting a clear finding) before any
set/diff logic runs. Keep the existing surface_type loop and Finding creation
unchanged once the schema check passes.
In `@scripts/docs-audit/test_audit_design_docs.py`:
- Around line 162-170: The tests are directly mutating
audit_design_docs.EXTRACTION_FLOORS and restoring it manually, which is fragile
and should be replaced with a scoped patch. Update the affected test cases in
test_audit_design_docs.py to use unittest.mock.patch.dict around
EXTRACTION_FLOORS instead of the current old/try/finally clear/update pattern,
so the original mapping is automatically restored after each test while keeping
the audit_coverage assertions unchanged.
🪄 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 Plus
Run ID: 4678144b-d1d9-48a6-907e-0393bcf81402
📒 Files selected for processing (9)
.agents/skills/docs-sync/SKILL.md.github/workflows/design-doc-audit.yml.github/workflows/duckpr-docs-sync.ymljustfilescripts/docs-audit/README.mdscripts/docs-audit/audit_design_docs.pyscripts/docs-audit/surface_map.mdscripts/docs-audit/surface_snapshot.jsonscripts/docs-audit/test_audit_design_docs.py
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/duckpr-docs-sync.yml:
- Around line 119-123: Update the actions/checkout step in the PR-head checkout
to set persist-credentials: false, ensuring the GITHUB_TOKEN is not stored in
the local git configuration before subsequent PR code execution.
- Around line 43-48: Move permissions from the workflow-level block to job-level
blocks in the relevant jobs, especially `audit-comment` and `docs-sync`. Grant
`audit-comment` only `pull-requests: read` and `issues: write`, and remove
unnecessary `contents: write`; configure `docs-sync` with only the permissions
it actually requires, since its push uses the DuckPR App token. Retain `actions:
read` and `checks: read` only where needed.
- Around line 60-70: Restrict the issue_comment trigger condition in the
workflow’s top-level if expression to trusted commenters by requiring
github.event.comment.author_association to be OWNER, MEMBER, or COLLABORATOR
alongside the existing checks; keep workflow_dispatch behavior unchanged. Do not
allow untrusted PR comments to reach audit-comment, which checks out
PR-controlled code with write-scoped access.
🪄 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 Plus
Run ID: cf755c9a-af46-40e0-a74b-f423481c265d
📒 Files selected for processing (3)
.agents/skills/docs-sync/SKILL.md.github/workflows/duckpr-docs-sync.ymlscripts/docs-audit/README.md
✅ Files skipped from review due to trivial changes (1)
- scripts/docs-audit/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/skills/docs-sync/SKILL.md
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
Important
本次增量 commit 让 docs-sync 工作流可以独立运行 deterministic audit，但 issue_comment 触发与 contents: write 的组合会引入未授权代码执行风险；另外 deterministic job 的检出方式对 fork PR 不够可靠。
Reviewed changes — 本 run 只审阅了新 commit 45e4ead，聚焦 docs-sync 工作流与对应 skill 文档调整。
- 将
DuckPR Docs Sync拆分为 always-on 的 deterministicaudit-commentjob 与可选的 LLMdocs-syncjob。 - 新增
@duckpr docs/@pullfrog docs的 PR comment 触发。 docs-sync改用push: restricted并检出 branch name（而非 detached SHA）以支持推送。- 在
README.md与SKILL.md中同步说明触发方式、验证状态与输出范围。
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
Important
本次增量 commit 调整了 duckpr-docs-sync.yml 的模型传参与权限声明，但新增的 workflow-level id-token: write 找不到对应 OIDC 消费者，会进一步放大默认 token 的权限范围。
Reviewed changes — 本 run 只审阅了新 commit e335f7d，聚焦 docs-sync 工作流与 README 的模型/权限微调。
- 新增 workflow-level
id-token: write权限。 - 新增
Require model步骤，移除默认的 Claude 模型回退，要求显式传入与 DuckPR Review 一致的模型。 Run private Pullfrog docs-sync步骤的model与PULLFROG_MODEL改为使用steps.model.outputs.model。- 将
LOG_LEVEL从info调整为debug。 - 在 README 中同步说明模型参数必须与 DuckPR Review 保持一致。
⚠️ workflow-level id-token: write 缺乏必要依据
docs-sync job 使用 actions/create-github-app-token 创建 DuckPR App token，该 action 只需要 app-id 与 private-key，不需要 OIDC id-token。在 workflow 顶层声明 id-token: write 会让 audit-comment 这个检出并执行 PR 分支代码的 job 也持有 OIDC 写权限，扩大了攻击面。
Technical details
# workflow-level `id-token: write` 权限过宽
## Affected sites
- `.github/workflows/duckpr-docs-sync.yml:49` — 新增的 `id-token: write` 没有对应 OIDC 消费者
- `.github/workflows/duckpr-docs-sync.yml:120-123` — `audit-comment` job 检出 PR head 后执行代码，继承 workflow-level 权限
## Required outcome
- 如果私有 Pullfrog action 确实需要 `id-token: write`，应将其限制在 `docs-sync` job 级别，并记录具体用途。
- 否则应移除该权限声明。
## Suggested approach
移除 workflow-level `id-token: write`；若 `docs-sync` 需要，在 job-level 单独声明并记录具体用途。
## Open questions for the human
- 新增 `id-token: write` 是为了解决哪一次失败的运行？是否有对应的 OIDC 身份验证步骤？anthropic/glm-5.2 ｜ 𝕏
Register surface audit + docs-sync on the fork default branch so workflow_dispatch / @duckpr docs can be exercised end-to-end. Co-authored-by: Cursor <cursoragent@cursor.com>
Spell out when to write, no padding, truth-first, exemplar selection, and required compatibility/test notes so agent output matches project contract. Co-authored-by: Cursor <cursoragent@cursor.com>
Three changes that close the gap with warp docubot's mature prompt engineering, while keeping our deterministic audit advantage: 1. SKILL.md rewrite — truth-first promoted to the top hard rule; add a decision tree so the agent prefers "-> internal" / 「无需更新」 over padding docs for smoke stubs (the failure mode seen in the demo_widgets write E2E); document the injected <pr_context>/<changed_files>/ <audit_findings> blocks; make post-sync verification (step 4) mandatory with before→after finding counts. 2. duckpr-docs-sync.yml — inject <pr_context>, <changed_files>, and <audit_findings> into the prompt (warp's envsubst-style context feeding, scoped to what matters); add a pre-sync audit step for the baseline and a post-sync verify step that reports high-finding delta in the job summary. 3. Add docs/design/be/docs-sync-design-doc-audit.md and register it in surface_map.md — the sync flow eats its own dog food.
…y layer
Three-layer strengthening of the design-doc sync system, borrowing the
decision-quality core from warp's missing_docs / classify-changelog-pr while
keeping our single-repo model.
Layer 1 — audit coverage expansion (close real blind spots):
- 3 new surface extractors in audit_design_docs.py:
- api_subroutes: Register*Routes entry points per package (the real unit of
"this package contributes an HTTP resource"); defining package is the
source of truth so variable-name call prefixes don't leak in.
- event_contracts: event-type literals in managedagentsevents/events.go
(41 managed-agent event strings now tracked).
- auth_middleware: middleware defs + .Use() call sites in server.go.
- staleness reverse-check: scan docs/design/** prose for internal/<pkg>
references and flag packages that no longer exist (catches renames the
forward audit cannot see).
- surface_map.md: initial mappings for all 3 new types; snapshot refreshed.
- 4 new unit tests (subroutes, events, auth, staleness) + floor fix.
Layer 2 — deterministic classify layer (anti-drop guardrail):
- classify_changes.py: triages a PR's changed files into
exclude/must_document/should_document/needs_review BEFORE the LLM runs.
Rules cover only high-confidence cases; everything else is needs_review
with a reason — never silently bucketed as exclude. Unknown internal
packages route to should_document (verify) not exclude. Keyword bumps
(permission/state machine/outbox) raise should -> must.
- 21 unit tests including the no-silent-drop invariant.
- SKILL.md step 2 now consumes the verdict as binding input; final comment
reports classify verdict + per-file needs_review resolutions.
Layer 3 — agent/workflow capability borrowing from warp:
- design-doc-audit.yml: when audit=exit1 or classify=must/needs_review, post
a PR nudge to trigger @duckpr docs (the audit -> agent bridge).
- duckpr-docs-sync.yml: inject <classify> JSON + PR author into prompt;
classify runs as a dedicated step.
- SKILL.md: separate content vs bookkeeping commits; reviewer routing
(cc @author); comment template now carries classify verdict.
- Design doc + README + justfile updated to reflect 7 surface types,
classify layer, staleness, and the audit->agent bridge.
Verified: 35 unit tests pass; audit clean (No findings); both workflows
YAML-valid; classify E2E-checked on demo_widgets / refactor / real-feature
PR shapes.
The Classify changed files step referenced $PR_NUMBER but never defined it in env, so gh pr view "" failed silently and classify always fell back to verdict=unknown. The nudge then relied solely on audit exit_code == 1 and never on the classifier. Define PR_NUMBER from the pull_request event payload so the classify verdict flows into the audit->agent bridge.
Pullfrog/opencode routes providers by the model id prefix. A bare id like kimi-k2.5 has no provider signal, so Pullfrog falls back to bedrock and fails on missing AWS_* secrets even though LLM_BASE_URL/ANTHROPIC_BASE_URL point at the Moonshot Anthropic-compatible gateway and ANTHROPIC_AUTH_TOKEN is set. When the resolved base URL is an Anthropic-compatible gateway (moonshot/kimi/any /anthropic suffix) and the model id has no provider/ prefix, namespace it as anthropic/<id> so provider routing lands on the Anthropic provider. Provider-namespaced ids pass through unchanged.
The /sessions entry under API mounts mapped to a frontend design doc
(docs/design/fe/sessions/session-detail-lane-timeline-design.md), but
this surface is the backend HTTP resource mounted at internal/api/server.go
(Mount("/sessions", s.sessions)) — the same bucket as /agents, /files,
/v1/code/sessions. Pointing it at an FE doc misrepresents coverage and
matches no backend behavior doc (none exists for the sessions HTTP API).
Gate it as needs-design-doc for consistency with sibling backend mounts
(/agents, /files, /memory_stores are all gated). The FE sessions route
under FE routes still correctly maps to the lane-timeline doc; the
sessions package still maps to permission-policies.md. Audit still passes
with no findings (gated count 11->12, mapped 2->1).
Upstream main gained 2 packages (agentsnapshot, skillprewarm) and 3 migrations (00010-00012, from PR superduck-ai#29/superduck-ai#30 builtin skills + display title) since PR superduck-ai#32's base. The audit correctly flagged them as unmapped. - agentsnapshot -> internal (snapshot serialization helper, no contract) - skillprewarm -> gated:needs-design-doc (PR superduck-ai#30 prewarming behavior) - 00010_builtin_skills.sql -> gated:needs-design-doc (PR superduck-ai#29 data model) - 00011_unique_skill_display_title.sql -> internal (index change) - 00012_require_skill_display_title.sql -> internal (NOT NULL constraint) Audit now passes with 0 findings on upstream main.
e335f7d to
827e78f
Compare
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
Caution
本次增量提交引入了 actions/checkout@v6，该版本不是有效的公共 action 版本，会导致 DuckPR Docs Sync 工作流直接失败。
Reviewed changes — 本次增量提交在确定性 triage、模型命名空间与 surface map 维护上继续迭代。
- 新增
scripts/docs-audit/classify_changes.py与单元测试，为 docs-sync 提供exclude/must_document/needs_review的确定性 triage。 - 在
duckpr-docs-sync.yml中新增 classify 步骤，将<classify>JSON 注入 agent prompt。 - 新增
Require model步骤，对 Kimi/Moonshot Anthropic-compatible gateway 将模型 id 命名空间到anthropic/。 - 将
/sessions从 FE doc 映射改为gated:needs-design-doc。 - 在
surface_map.md与surface_snapshot.json中补齐skillprewarm与 00010-00012 migrations。
anthropic/glm-5.2 ｜ 𝕏
Adds .github/PULL_REQUEST_TEMPLATE.md with: - change-type checkboxes (backend/frontend/CI/docs/test) - self-check list (issue link, go test, bun build, migration, chi routes, multi-tenant scope) - design-doc sync section: authors self-check whether docs/design/ needs updating, with @duckpr docs / --audit-only trigger instructions The checklist complements design-doc-audit.yml's automatic nudge: the CI posts a reminder when it detects coverage signals, while the template lets authors self-assess before pushing.
The post-sync verify step labeled HIGH_BEFORE (a high-finding count) as 'exit', producing confusing output like 'pre-sync: exit 3 high findings'. HIGH_BEFORE is the baseline high-finding count from audit-before, not an exit code. Rewrite the line to show before→after high-finding counts with the audit exit code labeled separately.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
.github/workflows/design-doc-audit.yml (3)
34-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout.Runs on
pull_request(including forks); leaving the token persisted widens exposure to any later step even though the workflow-level permission is alreadycontents: read.🔒 Proposed fix
- uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/design-doc-audit.yml at line 34, Set persist-credentials: false on the actions/checkout@v4 step in the workflow so the GitHub token is not retained after checkout, while preserving the existing read-only permissions.Source: Linters/SAST tools
59-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSpace-splitting
gh pr viewoutput breaks on filenames with spaces.
tr '\n' ' 'collapses newline-delimited paths into a single space-separated string; any filename containing a space will be split into two bogus--filesentries, silently mis-triaging that PR.classify_changes.pyalready supports newline-delimited stdin input, which avoids this entirely.♻️ Proposed fix
- python3 scripts/docs-audit/classify_changes.py \ - --files $(tr '\n' ' ' < /tmp/paths.txt) \ - --output /tmp/classify.json > /tmp/classify.txt 2>&1 + python3 scripts/docs-audit/classify_changes.py \ + --output /tmp/classify.json < /tmp/paths.txt > /tmp/classify.txt 2>&1🤖 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/design-doc-audit.yml around lines 59 - 64, Replace the space-splitting command substitution in the classify_changes.py invocation with newline-delimited stdin input, passing /tmp/paths.txt directly via standard input while retaining the existing output and error redirection. This preserves filenames containing spaces and uses classify_changes.py’s supported input format.
90-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrefer
env:passthrough over direct template expansion ingithub-script.
auditCode/verdictare interpolated straight into the JS body. Current values are constrained enums (exit code / classifier verdict), so this isn't exploitable today, but it's the pattern zizmor's template-injection check exists to prevent — pass them viaenv:and readprocess.env.*instead so any future change to these outputs' vocabulary can't break out of the string literal.Proposed fix
uses: actions/github-script@v7 with: github-token: ${{ github.token }} + env: + AUDIT_EXIT_CODE: ${{ steps.audit.outputs.exit_code }} + CLASSIFY_VERDICT: ${{ steps.classify.outputs.verdict }} script: | - const auditCode = "${{ steps.audit.outputs.exit_code }}"; - const verdict = "${{ steps.classify.outputs.verdict }}"; + const auditCode = process.env.AUDIT_EXIT_CODE; + const verdict = process.env.CLASSIFY_VERDICT;🤖 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/design-doc-audit.yml around lines 90 - 95, Replace direct GitHub Actions expression interpolation in the github-script step with an env: mapping for the audit and verdict outputs, then read them via process.env in the script when initializing auditCode and verdict. Update the relevant github-script step while preserving existing behavior.Source: Linters/SAST tools
.github/workflows/duckpr-docs-sync.yml (1)
273-283: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame space-splitting risk as
design-doc-audit.yml.
--files $(cat /tmp/changed-paths.txt | tr '\n' ' ')mis-splits filenames containing spaces.classify_changes.pyaccepts newline-delimited stdin, which avoids the issue.♻️ Proposed fix
- python3 scripts/docs-audit/classify_changes.py \ - --files $(cat /tmp/changed-paths.txt | tr '\n' ' ') \ - --output /tmp/classify.json > /tmp/classify.txt 2>&1 || true + python3 scripts/docs-audit/classify_changes.py \ + --output /tmp/classify.json < /tmp/changed-paths.txt > /tmp/classify.txt 2>&1 || true🤖 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/duckpr-docs-sync.yml around lines 273 - 283, The classify_changes invocation in the “Run classify_changes” workflow step incorrectly splits filenames on whitespace. Pass the contents of /tmp/changed-paths.txt through classify_changes.py’s newline-delimited stdin interface instead of using the --files command substitution, while preserving the existing output, error handling, and verdict extraction.docs/design/be/docs-sync-design-doc-audit.md (1)
41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a language hint to the fenced code block.
markdownlint (MD040) flags this block; use a neutral language like
text.Proposed fix
-``` +```text <surface> -> docs/design/<area>.md # 已有设计文档 <surface> -> internal # 基础设施/无设计关切，无需文档 <surface> -> gated:<reason> # 明确推迟（如 gated:needs-design-doc）🤖 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 `@docs/design/be/docs-sync-design-doc-audit.md` around lines 41 - 45, Update the fenced code block in the design audit documentation to specify the neutral `text` language hint, changing the opening fence while preserving all existing content.Source: Linters/SAST tools
🤖 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 @.agents/skills/docs-sync/SKILL.md:
- Around line 23-30: Update the documented <pr_context> schema to include the PR
author, using the field name consumed by the final @<pr author> template. Ensure
the context-building instructions populate this field from the PR metadata, or
remove the mandatory author placeholder from the template if author data cannot
be provided; update the related sections at the referenced locations
consistently.
- Around line 25-27: The classifier verdict terminology is inconsistent: the
contract defines exclude, must_document, and needs_review, while later guidance
uses should_document. Update the SKILL.md instructions and final audit template
to use one consistent verdict set, preferably the classifier’s existing values;
if should_document is required, also update
scripts/docs-audit/classify_changes.py and its tests accordingly.
- Line 84: Add the text language identifier to the fenced decision-tree code
block in the documentation, changing its opening fence to ```text while
preserving the existing contents and closing fence.
In `@justfile`:
- Around line 55-58: Update the justfile recipe parameter for docs-classify so
paths is declared variadic, allowing multiple space-separated file paths to be
passed through to scripts/docs-audit/classify_changes.py as documented.
In `@scripts/docs-audit/audit_design_docs.py`:
- Around line 672-676: Handle JSONDecodeError around the snapshot parsing in the
args.diff branch, reporting a concise corruption error to stderr and returning
the script’s controlled failure exit code instead of allowing a traceback;
update the logic near snapshot validation and json.loads.
In `@scripts/docs-audit/classify_changes.py`:
- Line 77: Update the repository metadata regex in the change-classification
rules to match `.github/CODEOWNERS` with its actual capitalization, while
preserving matches for the existing dependabot and mergify paths.
In `@scripts/docs-audit/README.md`:
- Line 63: Add the text language identifier to the mapping code fence in the
README by changing the opening fence to ```text, leaving the fenced content
unchanged.
- Around line 33-43: Clarify the timing statement in the Docs agent section to
match the commands: state that the LLM agent runs against an open,
same-repository PR when secrets are available, or revise the commands to reflect
a true post-merge workflow. Keep the model and DuckPR Review compatibility
guidance unchanged.
---
Nitpick comments:
In @.github/workflows/design-doc-audit.yml:
- Line 34: Set persist-credentials: false on the actions/checkout@v4 step in the
workflow so the GitHub token is not retained after checkout, while preserving
the existing read-only permissions.
- Around line 59-64: Replace the space-splitting command substitution in the
classify_changes.py invocation with newline-delimited stdin input, passing
/tmp/paths.txt directly via standard input while retaining the existing output
and error redirection. This preserves filenames containing spaces and uses
classify_changes.py’s supported input format.
- Around line 90-95: Replace direct GitHub Actions expression interpolation in
the github-script step with an env: mapping for the audit and verdict outputs,
then read them via process.env in the script when initializing auditCode and
verdict. Update the relevant github-script step while preserving existing
behavior.
In @.github/workflows/duckpr-docs-sync.yml:
- Around line 273-283: The classify_changes invocation in the “Run
classify_changes” workflow step incorrectly splits filenames on whitespace. Pass
the contents of /tmp/changed-paths.txt through classify_changes.py’s
newline-delimited stdin interface instead of using the --files command
substitution, while preserving the existing output, error handling, and verdict
extraction.
In `@docs/design/be/docs-sync-design-doc-audit.md`:
- Around line 41-45: Update the fenced code block in the design audit
documentation to specify the neutral `text` language hint, changing the opening
fence while preserving all existing content.
🪄 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 Plus
Run ID: 6623a6d0-b677-4e79-af25-9e71ba52ca3c
📒 Files selected for processing (12)
.agents/skills/docs-sync/SKILL.md.github/workflows/design-doc-audit.yml.github/workflows/duckpr-docs-sync.ymldocs/design/be/docs-sync-design-doc-audit.mdjustfilescripts/docs-audit/README.mdscripts/docs-audit/audit_design_docs.pyscripts/docs-audit/classify_changes.pyscripts/docs-audit/surface_map.mdscripts/docs-audit/surface_snapshot.jsonscripts/docs-audit/test_audit_design_docs.pyscripts/docs-audit/test_classify_changes.py
✅ Files skipped from review due to trivial changes (2)
- scripts/docs-audit/surface_snapshot.json
- scripts/docs-audit/surface_map.md
The workflow listens to issue_comment directly (for repos without DuckPR installed) AND DuckPR dispatches via workflow_dispatch. On the issue_comment path, inputs.skip_agent is undefined, so --audit-only in the comment body was ignored and the LLM agent ran anyway — diverging from the DuckPR dispatch path where skip_agent is correctly forwarded. Parse --audit-only from github.event.comment.body on the issue_comment path so both trigger paths honor the flag. Concurrency dedup keeps the double-dispatch (when DuckPR is installed) to a single cancelled run. Also update docs-sync-design-doc-audit.md to document the --audit-only flag, the three-tier model resolution (repo docs_model > DUCKPR_DOCS_MODEL > DUCKPR_MODEL), and the no-review-model-fallback design.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
Important
本次增量改动（PR 模板 + --audit-only issue_comment 路径支持 + post-sync 文案修正）没有引入新问题，但此前 review 已标记的 actions/checkout@v6 仍在 .github/workflows/duckpr-docs-sync.yml 中未修复，会导致 workflow 解析 action 失败。
Reviewed changes — 新增 PR 模板，并在 docs-sync 工作流中统一 --audit-only 在两种触发路径下的语义。
- 新增
.github/PULL_REQUEST_TEMPLATE.md，包含变更类型、自检清单以及设计文档同步触发说明。 - 在
.github/workflows/duckpr-docs-sync.yml的Resolve PR context步骤中注入COMMENT_BODY，并对issue_comment路径解析--audit-only，使@duckpr docs --audit-only在评论触发时也能跳过 LLM agent。 - 修正 post-sync summary 的文案，将
exit与high findings的对应关系表达清楚。 - 在
docs/design/be/docs-sync-design-doc-audit.md中补充--audit-only与 model 解析三层优先级的说明。
anthropic/glm-5.2 ｜ 𝕏
The docs-sync job had no PR-facing failure feedback. When the pullfrog agent step failed or timed out, the workflow went red but the PR only showed DuckPR's earlier 'queued' message — the author had no signal to retry and might wait indefinitely. Add a 'Notify on docs-sync failure' step (if: failure()) that upserts a PR comment with a run-log link and retry instructions (including --audit-only as a fallback). Uses the same upsert marker pattern as the audit/nudge comments to avoid spamming on retries. Note: GitHub Actions does not run post-steps after a job-level timeout-minutes cancel, so this covers step-level failures (including pullfrog's internal 1h+5min agent safety-net kill) but not the extreme case where the safety-net itself hangs and the 45-min job cap fires.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ No new issues found.
Reviewed changes — 本次增量提交在 DuckPR Docs Sync 工作流中新增了 docs-sync agent 失败或超时的 PR 评论通知。
- 在
docs-syncjob 末尾新增Notify on docs-sync failure步骤，当上游Run private Pullfrog docs-sync步骤失败时通过actions/github-script@v7向 PR 发布失败通知。 - 使用
<!-- design-doc-audit-failure -->marker 对通知评论做 upsert，避免重试时重复刷屏。 - 通知文案指引评论者通过
@duckpr docs重新触发，或通过@duckpr docs --audit-only仅获取审计结果。
anthropic/glm-5.2 ｜ 𝕏
Security (C1/H3/M1): - duckpr-docs-sync.yml: wrap pr_body/trigger_comment/extra_instructions in unique per-run delimiters (crypto.randomBytes), place SKILL instructions first with an adversarial guard, sanitize delimiter-forging lines, cap each untrusted field at 20 KB; split prompt into trusted (XML-tagged) vs untrusted blocks - persist-credentials: false so the DuckPR App token is not written to .git/config (Pullfrog uses ASKPASS for per-call auth) - classify_changes.py invoked via stdin pipe in both workflows to avoid command substitution on attacker-controlled filenames Contract (P1-P8): - SKILL.md: clarify pr_context/changed_files format, degradation semantics for missing classify/audit blocks, exclude-priority, step 2a ambiguity, complete finding-kind table, commit edge cases, high-count semantics - design doc: add 安全边界 section, rewrite prompt-injection section for permission gating and trusted/untrusted split
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ No new issues found.
Reviewed changes — 本次增量提交聚焦 prompt injection 防御与日志/命令注入硬ening。
- 在
.agents/skills/docs-sync/SKILL.md中显式区分 trusted/untrusted 输入块，并将 SKILL 指令置于 prompt 最前，确保 agent 先建立指令框架再看到用户可控内容。 - 在
.github/workflows/duckpr-docs-sync.yml的 prompt 构造步骤引入随机唯一分隔符=== UNTRUSTED_DATA_OPEN_<token> ===包裹pr_body、trigger_comment、extra_instructions，并 sanitize 掉任何伪造分隔符的行。 - 将
classify_changes.py的文件路径输入改为 stdin 管道，避免 PR 文件名中的 shell 命令替换。 - 将
LOG_LEVEL从debug调回info，并避免在日志中打印完整base_url。 - 在
docs/design/be/docs-sync-design-doc-audit.md中同步记录安全边界、注入防御与权限门控。
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/skills/docs-sync/SKILL.md (1)
294-297: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReconcile the companion-branch flow with the push restriction.
The hard rules permit pushes only to the current PR branch and prohibit branch creation, but this edge case instructs the agent to create and push
docs/sync-<slug>. Choose one policy and make the authorization explicit; otherwise the agent has contradictory branch instructions.🤖 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 @.agents/skills/docs-sync/SKILL.md around lines 294 - 297, Update the companion-branch guidance in the docs-sync skill to resolve its conflict with the restricted-push and no-branch-creation rules. Choose a single policy and explicitly state whether creating and pushing docs/sync-<slug> is authorized for this edge case, while preserving the required original-PR comment behavior.
🤖 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 @.agents/skills/docs-sync/SKILL.md:
- Around line 27-32: The changed-file handling in the docs-sync workflow must
not silently omit files beyond the 300-file cap. Update the <changed_files>
processing and classification flow to process the complete list, or fail closed
with an explicit manual-follow-up requirement when the limit is exceeded; ensure
omitted files cannot bypass classification or the anti-drop guardrail.
- Around line 33-41: Update the `<classify>` and `<audit_findings>` handling in
the documentation sync workflow to parse each block as JSON and validate its
required schema fields before treating it as binding. If either block is absent,
malformed, non-object, or missing required fields, trigger its existing
classifier or audit fallback path instead of using incomplete data; preserve the
current valid-input behavior and field semantics.
- Around line 259-275: Update the before/after verification instructions around
the JSON audit comparison to track stable identities for high-severity findings
owned by this PR, rather than relying only on global high-finding totals.
Require owned findings to be resolved or explicitly deferred, assess regressions
and completion using the owned sets, and retain the global counts only as
supplemental final-comment data.
---
Outside diff comments:
In @.agents/skills/docs-sync/SKILL.md:
- Around line 294-297: Update the companion-branch guidance in the docs-sync
skill to resolve its conflict with the restricted-push and no-branch-creation
rules. Choose a single policy and explicitly state whether creating and pushing
docs/sync-<slug> is authorized for this edge case, while preserving the required
original-PR comment behavior.
🪄 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 Plus
Run ID: 166cecd2-a2ef-4f35-8334-dba0fa9f4eb6
📒 Files selected for processing (5)
.agents/skills/docs-sync/SKILL.md.github/PULL_REQUEST_TEMPLATE.md.github/workflows/design-doc-audit.yml.github/workflows/duckpr-docs-sync.ymldocs/design/be/docs-sync-design-doc-audit.md
✅ Files skipped from review due to trivial changes (1)
- .github/PULL_REQUEST_TEMPLATE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/design-doc-audit.yml
- docs/design/be/docs-sync-design-doc-audit.md
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/duckpr-docs-sync.yml (1)
328-458: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftTreat PR-controlled prompt inputs as untrusted.
Lines 328-458 load
SKILL.mdfrom the PR checkout and label changed paths, classifier JSON, and audit JSON as trusted. The PR can modify these files and outputs. The PR title and filenames are also user-controlled, but they are inserted outsidewrapUntrusted.This bypasses the prompt-injection boundary. A PR can provide instructions to the credentialed docs agent as if they were workflow policy.
Load the skill from an immutable base or default-branch revision. Wrap the PR title, changed paths, audit output, and classifier output with
wrapUntrusted. Treat only workflow-owned policy as binding instructions. Add size limits for these artifacts before setting the prompt output.🤖 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/duckpr-docs-sync.yml around lines 328 - 458, Treat all PR-controlled prompt content as untrusted: load SKILL.md from an immutable base/default-branch revision, and wrap the PR title, changed file paths, classifier JSON, and audit JSON with wrapUntrusted rather than placing them in trustedParts. Keep only workflow-owned policy in the trusted instruction context, and enforce bounded sizes for these artifacts before constructing the prompt..agents/skills/docs-sync/SKILL.md (1)
116-126: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMap every owned surface before completing the decision.
The
Nobranch permits leaving a surface unmapped, but step 3 requires every owned added surface to map to a design document,-> internal, or-> gated:<reason>. Leaving an owned surface unmapped preserves the audit finding and weakens the anti-drop guardrail.Reserve “leave unmapped” for unrelated findings. Map every owned surface.
🤖 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 @.agents/skills/docs-sync/SKILL.md around lines 116 - 126, Update the decision flow in the docs-sync skill so every owned added surface is mapped before completion: use a design document, “-> internal”, or “-> gated:<reason>”. Remove the option to leave owned surfaces unmapped; reserve unmapped status only for unrelated findings, while preserving the existing handling for documented changes.
🤖 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 @.agents/skills/docs-sync/SKILL.md:
- Around line 27-30: Update the changed-file handling in the docs-sync workflow
to preserve filenames containing spaces; pass the classifier’s structured
filename field instead of extracting the last whitespace-delimited token from
<changed_files>, or fail closed when exact paths cannot be represented by the
display format. Keep the complete changed-file list covered by the anti-drop
guardrail.
- Around line 259-276: Update the verification instructions around the
--update-snapshot and --diff commands to capture both exit codes and fail closed
when either returns 2. Require the agent to stop without committing or pushing
before comparing findings or reporting success if extraction or integrity
validation fails; preserve the existing before/after identity comparison for
successful verification.
- Around line 31-41: Update the workflow guidance for <classify> and
<audit_findings> in SKILL.md to explicitly treat all workflow-produced text and
string fields as untrusted data, including PR metadata, filenames, repository
guidance, and design documents. Restrict behavior to validated enum and numeric
fields, and instruct agents to ignore embedded instructions before reading files
or pushing changes while preserving the existing JSON schema validation and
fallback behavior.
---
Outside diff comments:
In @.agents/skills/docs-sync/SKILL.md:
- Around line 116-126: Update the decision flow in the docs-sync skill so every
owned added surface is mapped before completion: use a design document, “->
internal”, or “-> gated:<reason>”. Remove the option to leave owned surfaces
unmapped; reserve unmapped status only for unrelated findings, while preserving
the existing handling for documented changes.
In @.github/workflows/duckpr-docs-sync.yml:
- Around line 328-458: Treat all PR-controlled prompt content as untrusted: load
SKILL.md from an immutable base/default-branch revision, and wrap the PR title,
changed file paths, classifier JSON, and audit JSON with wrapUntrusted rather
than placing them in trustedParts. Keep only workflow-owned policy in the
trusted instruction context, and enforce bounded sizes for these artifacts
before constructing the prompt.
🪄 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 Plus
Run ID: 19c2ade3-ffaa-43f7-9311-37d3f244378c
📒 Files selected for processing (5)
.agents/skills/docs-sync/SKILL.md.github/workflows/design-doc-audit.yml.github/workflows/duckpr-docs-sync.ymldocs/design/be/docs-sync-design-doc-audit.mddocs/design/be/runtime-configuration.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/design/be/docs-sync-design-doc-audit.md
| - `<changed_files>` — the PR's complete, paginated changed-file list. Format per line: | ||
| `<status> +<additions>/-<deletions> <filename>` (e.g. | ||
| `modified +10/-3 internal/api/server.go`). To extract the filename for | ||
| `classify_changes.py --files`, take the last whitespace-delimited token. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve changed filenames without whitespace parsing.
<changed_files> can contain filenames with spaces. Taking the last whitespace-delimited token changes such a path. The classifier can then omit or misclassify the file, so the anti-drop guardrail does not cover the complete PR.
Use the structured filename field from the workflow, or fail closed when this display format cannot represent the exact path.
🧰 Tools
🪛 SkillSpector (2.4.4)
[error] 59: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 58: [P1] Instruction Override: This pattern attempts to override system instructions or ignore safety constraints. Without LLM analysis, manual review is recommended.
Remediation: Remove or rewrite any text that instructs the agent to ignore prompts, override safety rules, or trust unverified content. Ensure skill content cannot be injected to alter agent behavior.
(Prompt Injection (P1))
[error] 50: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 118: [YR4] YARA rule 'agent_skill_prompt_injection_hidden_instructions': Prompt injection or hidden instructions embedded in AI agent skill text [agent_skills]: YARA rule matched a hack tool or exploit indicator (offensive tools, reconnaissance, privilege escalation, or exploit frameworks).
Remediation: Remove offensive tool references and exploit code. Legitimate agent skills should not contain penetration testing tools, exploit frameworks, or reconnaissance utilities.
(YARA Match (YR4))
🤖 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 @.agents/skills/docs-sync/SKILL.md around lines 27 - 30, Update the
changed-file handling in the docs-sync workflow to preserve filenames containing
spaces; pass the classifier’s structured filename field instead of extracting
the last whitespace-delimited token from <changed_files>, or fail closed when
exact paths cannot be represented by the display format. Keep the complete
changed-file list covered by the anti-drop guardrail.
| - `<classify>` — JSON from `scripts/docs-audit/classify_changes.py`: the binding | ||
| per-file doc-need verdict (`exclude` / `must_document` / `needs_review`). | ||
| **Degradation**: parse it as JSON and require an object with a `files` array | ||
| and a `verdict.action` string. If absent, malformed, or incomplete, run the | ||
| classifier yourself (step 2a). | ||
| - `<audit_findings>` — JSON from `scripts/docs-audit/audit_design_docs.py --diff`. | ||
| Contains `exit_code` at the top level and a `findings` array. Each finding has | ||
| `severity` (`high`/`medium`/`low`), `kind`, `surface_type`, and `surface_id`. | ||
| **Degradation**: parse it as JSON and require an object with an integer | ||
| `exit_code` and a `findings` array. If absent, malformed, or incomplete, run | ||
| the audit yourself (step 1). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Treat workflow-produced text as data, not trusted instructions.
PR metadata, filenames, classifier strings, audit strings, AGENTS.md, and existing design documents can contain attacker-controlled text. JSON parsing and schema checks do not remove prompt instructions from string values.
Restrict binding behavior to validated enum and numeric fields. Treat all other text as data, and ignore instructions found in it before reading files or pushing changes.
🧰 Tools
🪛 SkillSpector (2.4.4)
[error] 59: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 58: [P1] Instruction Override: This pattern attempts to override system instructions or ignore safety constraints. Without LLM analysis, manual review is recommended.
Remediation: Remove or rewrite any text that instructs the agent to ignore prompts, override safety rules, or trust unverified content. Ensure skill content cannot be injected to alter agent behavior.
(Prompt Injection (P1))
[error] 50: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 118: [YR4] YARA rule 'agent_skill_prompt_injection_hidden_instructions': Prompt injection or hidden instructions embedded in AI agent skill text [agent_skills]: YARA rule matched a hack tool or exploit indicator (offensive tools, reconnaissance, privilege escalation, or exploit frameworks).
Remediation: Remove offensive tool references and exploit code. Legitimate agent skills should not contain penetration testing tools, exploit frameworks, or reconnaissance utilities.
(YARA Match (YR4))
🤖 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 @.agents/skills/docs-sync/SKILL.md around lines 31 - 41, Update the workflow
guidance for <classify> and <audit_findings> in SKILL.md to explicitly treat all
workflow-produced text and string fields as untrusted data, including PR
metadata, filenames, repository guidance, and design documents. Restrict
behavior to validated enum and numeric fields, and instruct agents to ignore
embedded instructions before reading files or pushing changes while preserving
the existing JSON schema validation and fallback behavior.
| Compare `/tmp/docs-audit-after.json` to the baseline from step 1. Build the | ||
| before/after sets of owned high finding identities using | ||
| `surface_type/surface_id/kind`. Global high counts are supplemental context, | ||
| not the completion criterion: | ||
|
|
||
| ```bash | ||
| python3 -c "import json;d=json.load(open('/tmp/docs-audit-after.json'));print(sum(1 for f in d.get('findings',[]) if f.get('severity')=='high'))" | ||
| ``` | ||
|
|
||
| - Baseline high findings **owned by this PR** (see step 2b) must be | ||
| resolved: mapped, doc created, or explicitly deferred to `gated:`. | ||
| - Pre-existing high findings for surfaces this PR did not touch are **not** your | ||
| responsibility — they should not block completion. | ||
| - If new high findings appeared **because of your own edits**, fix them before | ||
| pushing. | ||
| - No new owned high identity may appear after the edit. Record owned identities | ||
| resolved/deferred plus the supplemental global `before=<N> after=<M>` counts | ||
| in the final comment. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail closed when verification returns exit code 2.
The verification runs --update-snapshot and then --diff, but does not require checking either exit code before comparing findings. If extraction or integrity validation fails, the agent can compare incomplete data and report success.
Capture both exit codes. Stop without committing or pushing when either command returns 2.
🧰 Tools
🪛 LanguageTool
[grammar] ~260-~260: Use a hyphen to join words.
Context: ...uild the before/after sets of owned high finding identities using `surface_type/s...
(QB_NEW_EN_HYPHEN)
[style] ~276-~276: Try using a synonym here to strengthen your wording.
Context: ...re= after=` counts in the final comment. ### 5. Commit + push (separate conten...
(COMMENT_REMARK)
🪛 SkillSpector (2.4.4)
[error] 59: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 58: [P1] Instruction Override: This pattern attempts to override system instructions or ignore safety constraints. Without LLM analysis, manual review is recommended.
Remediation: Remove or rewrite any text that instructs the agent to ignore prompts, override safety rules, or trust unverified content. Ensure skill content cannot be injected to alter agent behavior.
(Prompt Injection (P1))
[error] 50: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 118: [YR4] YARA rule 'agent_skill_prompt_injection_hidden_instructions': Prompt injection or hidden instructions embedded in AI agent skill text [agent_skills]: YARA rule matched a hack tool or exploit indicator (offensive tools, reconnaissance, privilege escalation, or exploit frameworks).
Remediation: Remove offensive tool references and exploit code. Legitimate agent skills should not contain penetration testing tools, exploit frameworks, or reconnaissance utilities.
(YARA Match (YR4))
🤖 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 @.agents/skills/docs-sync/SKILL.md around lines 259 - 276, Update the
verification instructions around the --update-snapshot and --diff commands to
capture both exit codes and fail closed when either returns 2. Require the agent
to stop without committing or pushing before comparing findings or reporting
success if extraction or integrity validation fails; preserve the existing
before/after identity comparison for successful verification.
1 participant
Summary
scripts/docs-audit/：从 chi Mount、internal/*包、migrations、FE routes 提取 surface，对照surface_map.md；CI 软提醒（exit 1），extraction/integrity 硬失败（exit 2）。.github/workflows/duckpr-docs-sync.yml+.agents/skills/docs-sync/：可对指定 PR 手动触发，只允许改docs/design/**与 map/snapshot。范围说明
gated:，先手动workflow_dispatch）。Test plan
python3 scripts/docs-audit/test_audit_design_docs.pypython3 scripts/docs-audit/audit_design_docs.py→ exit 0Design Doc Surface Auditworkflow 在本 PR 上跑通gh workflow run "DuckPR Docs Sync" -f pr_number=<N>验证 DuckPR 能读 skill 并回评Close #28
Made with Cursor
Summary by CodeRabbit