Repository navigation
Standardize human-attention labels and agent attribution - #732
Conversation
📝 WalkthroughWalkthroughThe change adds a shared GitHub comment-attribution envelope, integrates envelope-aware address-review fetching and posting, adds Human Attention desk and transition commands, and updates comment producers, workflows, trust manifests, and validation tests. ChangesComment attribution and human attention
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to Operational workflows can lose required audit records, misroute comments, incorrectly advance source-review handling, or hide unaffected human-attention queue entries. These should be corrected before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 15 files. (16 skipped: 16 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
ReviewReviewed the new Overall: the parser/renderer design is solid for a security-sensitive gate — it uses an exact-match strict format (not regex-on-untrusted-content), and the Main finding (see inline comment): Minor: Design note (non-blocking, informational): No security issues, performance concerns, or correctness bugs found in |
ReviewReviewed the Overall: solid design for a security-sensitive gate. Two issues found (posted inline):
No security issues found in |
Review summaryReviewed this cross-cutting PR (human-attention labels + comment-attribution envelope, ~3k additions across 45 files). Posted 8 inline comments; overview below. Correctness
Robustness
Security-adjacent (fail-safe, not exploitable)
Scope / duplication (matches the "smallest change" framing from the PR description)
Minor performance
Otherwise: the envelope round-tripping (CRLF handling, HTML-comment injection guard on |
Review summaryThis is a large but well-scoped cross-cutting migration (label state machine + comment attribution boundary), and it's already been through many review-fix rounds — the core logic (label transition fail-closed/verify/cleanup, envelope render/parse, trust exclusion at every evidence call site) held up well under scrutiny and is backed by proportionate test coverage for a merge/human-authority-critical feature. Two concrete issues found, posted inline:
Nothing else stood out as a genuine bug — attribution envelope construction/parsing correctly rejects newline/ |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 110bcb1a67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f6148c32-3675-4cb9-ae5d-d44a83d15898) |
Review summaryI reviewed this in depth (core new libraries by hand, plus two parallel deep-dives into the envelope rollout across address-review/post-merge-audit and into the docs/tests/scope surface). Overall this is a carefully engineered, well-tested change, and the "authority hardening" goal — excluding agent-enveloped/prefixed comments from counting as human approval or merge authority — is applied consistently and correctly everywhere it needs to be:
One scope finding (left as an inline comment): I didn't find correctness bugs, injection issues, or gaps in the fail-closed behavior in the core logic. Given the PR's own justification for bundling 45 files (producer/consumer schema changes that must move together), that claim holds up for everything except the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab6f36d8b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5fe66147-88b6-4d9d-befb-efea13ee6014) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f2e1d2a0-3264-4b01-9555-af4e6a3b3e70) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc18c6557a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review summaryThis is a large (3.3k+ line), 45-file cross-cutting change, but the PR body's justification for not splitting it (label state, attribution envelope, review fetch/post, merge-authority evidence, and audit receipts all need to move together) is reasonable given the coupling between producers and consumers of the new contract. I verified the changes directly rather than only relying on described intent; findings below are all confirmed against the actual code, not speculative. Correctness (posted inline):
Duplication introduced by this PR (posted inline) — worth consolidating given the review's simplicity bar:
Lower-severity correctness note (posted inline):
Everything else I checked — the |
|
Approved this exact revision for merge after reviewing the listed risk and rollback plan.
Ordinary merge checks still apply. Approval receipt---
head_sha: 6a8a3c2eeed166e73b609721394cc4e5126c803c
triggered_gates:
- architectural-product-judgment
- autonomous-merge-policy-change
- changed-files-limit
- changed-lines-limit
- commit-count-limit
- public-compatibility
- security-auth-privacy
rollback_disposition: Code rollback and forward recovery were reviewed.
decision: approve
approved_by: justin808
source: direct-user-task
evidence: direct-user-task:01a08d88-d2e1-7761-bfa0-ba0d15d28490
... |
…naged-snapshot * origin/main: Standardize human-attention labels and agent attribution (#732)
…libration-utf8 * origin/main: Standardize human-attention labels and agent attribution (#732)
…rrent-main-rebuild * origin/main: Align coordination backend preset vocabulary (#843) Complete replicate-ci run-history recipe (#842) Standardize human-attention labels and agent attribution (#732) address-review: bind automation reviewers to exact check names (#836) Align coordination manifest target forms (#834) Extract seam-doctor shell safety helpers (#837) Keep current PR walkthroughs visible during closeout (#831) test: cover dispatch approval after refresh (#835) test: add scheduler headroom to batch status timeouts (#838) Add Cursor as a third Agent Workflows host (#832) Separate optional quality candidates from admitted maintenance (#827) fix: authenticate terminal configured-review exceptions (#825) Prevent temporary-pack hardlink races in stack fixture clones (#824) Enforce actor trust at the review-data ingestion boundary (#799)
…ust-role-shapes * origin/main: Follow-up: calibration GitHub reader needs UTF-8 and surrogate gates (#700) Make maintainer decision requests understandable (#849) Align coordination backend preset vocabulary (#843) Complete replicate-ci run-history recipe (#842) Standardize human-attention labels and agent attribution (#732) # Conflicts: # skills/pr-batch/bin/stale-assignment-sweep
…sk-titles * origin/main: Follow-up: calibration GitHub reader needs UTF-8 and surrogate gates (#700) Make maintainer decision requests understandable (#849) Align coordination backend preset vocabulary (#843) Complete replicate-ci run-history recipe (#842) Standardize human-attention labels and agent attribution (#732)
Why
GitHub is the canonical human-attention surface for Agent Workflows, but maintainers need portable walkthrough and merge states plus reliable agent attribution when an agent and human share one GitHub account.
Issue: #731
What changed
human-attention:walkthroughandhuman-attention:mergedefaults with consumer policy overrides, exact-head fail-closed transitions, and a configured cross-repository desk.main, re-triaged every review wave, and fixed the current-head validation failures.Scope and deferrals
This is a 49-file, cross-cutting contract migration. The label state, attribution envelope, review fetch and post paths, autonomous merge evidence, audit receipt, copied workflow documentation, and compatibility tests need to change together so producers and consumers agree about comment authority and human-attention state.
deferred_to_update_changelog.How to review and verify
Start with
.agents/agent-workflow.yml,skills/pr-batch/lib/human_attention.rb, andskills/pr-batch/lib/github_comment_envelope.rb. Then trace the address-review and closeout callers to confirm they use the shared boundaries. The regression suites cover exact-head label transitions, process cleanup, malformed policy, CRLF and legacy comments, and prevention of agent comments being accepted as human authority.Test plan
bin/validatepassed at6a8a3c2eeed166e73b609721394cc4e5126c803c(run 34748760849).ruby skills/pr-batch/bin/human-attention-test.rb: 38 runs, 238 assertions.ruby skills/pr-batch/bin/human-attention-contract-test.rb: 9 runs, 118 assertions.ruby skills/pr-batch/bin/github-comment-envelope-test.rb: 22 runs, 91 assertions.ruby skills/pr-batch/bin/integration-closeout-contract-test.rb: 19 runs, 749 assertions, 1 intentional skip.git diff --checkpassed.Agent details
01a08d88-d2e1-7761-bfa0-ba0d15d28490aw-sept7-closeout-01a08d8844c9292926bc670b663825e0614384c989751d056a8a3c2eeed166e73b609721394cc4e5126c803cSummary by CodeRabbit
New Features
Documentation
Bug Fixes
Note
High Risk
Changes how human merge/waiver/audit authority is inferred on GitHub and how PR attention labels are mutated, which are security- and workflow-critical paths.
Overview
Introduces a portable human-attention contract and a shared GitHub comment attribution boundary so towers and skills no longer apply raw queue labels or post comments directly.
Human attention.
.agents/agent-workflow.ymlgainshuman_attentiondefaults (walkthrough/mergelabels plus configured repos). The newhuman-attentionhelper exposesdesk(cross-repo queue) andtransition(semanticwalkthrough→merge→nonewith exact-head checks, fail-closed cleanup on conflicts, timeouts, and closed PRs). Seam doctor validates the policy; control-tower and closeout docs route through${PR_BATCH_SKILL_DIR}/bin/human-attention transitioninstead of literal label names.Agent attribution. New
github-comment-envelopeCLI/library wraps issue comments, edits, and review replies with a visible🤖 Codex|Claude|Cursorline and a hiddenagent-comment-attribution:v1marker (AGENT_COMMENT_RUNNER, host, task/run). Address-review, stale-assignment sweep, manual-testing, verify-fix, and completed-batch audit receipts post through this path; review fetch addspayload_bodyand--issue-comments-onlyso workflow markers still work inside envelopes.Trust / merge authority. Autonomous merge, hosted QA waivers, completed-batch publication preflight, and accepted-deferral logic treat agent-enveloped or prefixed comments as non-human so shared accounts cannot spoof maintainer decisions.
cursoris added to trusted GitHub actors; mutating stale-assignment--applynow requires the same attribution env vars.Extensive contract and regression tests are wired into
bin/validate.Reviewed by Cursor Bugbot for commit bc18c65. Bugbot is set up for automated code reviews on this repo. Configure here.