fix(fix-agent): sync package registry policy with code agent - #766
Conversation
PR Summary by QodoClarify fix agent policy overlap with code agent policy
AI Description
High-Level Assessment
Files changed (1)
|
|
🤖 Finished Review · ✅ Success · Started 1:32 PM UTC · Completed 1:43 PM UTC Commit: |
This comment was marked as spam.
This comment was marked as spam.
ReviewFindingsMedium
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Labels: PR modifies fix agent sandbox policy file Previous run (3)ReviewFindingsMedium
Labels: PR updates only a documentation comment in the fix agent sandbox policy file. |
waynesun09
left a comment
There was a problem hiding this comment.
Automated review findings (review-only sweep).
|
🤖 Review · Commit: |
c4f23c5 to
985bc67
Compare
|
🤖 Finished Review · ✅ Success · Started 11:50 AM UTC · Completed 12:01 PM UTC Commit: |
waynesun09
left a comment
There was a problem hiding this comment.
Automated review findings (review-only sweep).
[MEDIUM] No automated regression test guards package_registries parity (triage's proposed test was never added) (policies/fix.yaml, around the package_registries: block, line 88 — outside this PR's diff hunks so it can't be attached inline)
This is the third time policies/fix.yaml and policies/code.yaml have drifted apart (issue #532 references #399 and #531 as prior instances). Issue #532's triage comment supplied a ready-to-use pytest test comparing package_registries host/binary sets between the two files specifically to prevent recurrence, but this PR (verified at head 985bc67) does not add it, and none of the repo's existing scripts (script-test.yml target scripts, e.g. process-fix-result-test.py, post-fix-test.sh) or workflows (lint.yml, functional-tests.yml, script-test.yml) perform any such comparison. The maintainer's Aug-12 comment on #532 addresses this only via docs (fullsend#6111) and per-repo harness-override guidance, not via an in-repo CI check. Since this PR's own header-comment change now explicitly permits future intentional divergence ("may diverge when fix-specific access is required"), there is no guardrail left to distinguish a future accidental drift from a sanctioned one.
Suggestion: Add the test proposed in the issue-532 triage comment (comparing endpoint hosts and binary paths per network_policies.package_registries block between policies/code.yaml and policies/fix.yaml) as a script-test or pytest case wired into script-test.yml or functional-tests.yml, so a future silent drift fails CI instead of shipping quietly.
|
🤖 Finished Review · ✅ Success · Started 1:35 AM UTC · Completed 1:48 AM UTC Commit: |
waynesun09
left a comment
There was a problem hiding this comment.
Automated review findings (review-only sweep).
Replace "Identical to the code agent policy" with wording that describes shared needs while allowing intentional divergence. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Fixes #532. Add missing yarn registry endpoint and npx/yarn/pnpm binaries; revise header comment to allow intentional divergence. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com>
Scoped yarn package paths need encoded slashes, same class of issue as Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> #531 on registry.npmjs.org. Apply to both code and fix agent policies.
d44ce69 to
76b7bf5
Compare
|
🤖 Finished Review · ✅ Success · Started 3:16 PM UTC · Completed 3:29 PM UTC Commit: |
waynesun09
left a comment
There was a problem hiding this comment.
Verified all review findings are addressed at head 76b7bf5:
- [HIGH] Missing
registry.yarnpkg.comendpoint andnpx/yarn/yarnpkg/pnpmbinaries inpolicies/fix.yaml— fixed in c4f23c5, confirmed in the diff. - [MEDIUM]
allow_encoded_slash: truemissing on the yarnpkg endpoint (same class as #531) — fixed in d44ce69 in bothfix.yamlandcode.yaml, confirmed in the diff. - [MEDIUM] ADR 0065 string coupling — resolved in fullsend-ai/fullsend#6111 (e5c996fa): original Context quote restored, divergence documented in a
## Notesannotation instead. - [MEDIUM] PR body omitting the
code.yamlbehavior change — body now documents it explicitly with rationale.
The parity-test suggestion from #532 triage was discussed and declined by the maintainer as intentional (code and fix harnesses may diverge by design) — noted, not blocking.
CI is green at head. Approving.
|
🤖 Finished Retro · ✅ Success · Started 3:37 PM UTC · Completed 3:51 PM UTC Commit: |
Retro: PR #766 — fix(fix-agent): sync package registry policy with code agentTimeline
Review quality
The manual review sweeps (waynesun09, labeled "Automated review findings (review-only sweep)") produced 5 findings, 4 of which led directly to code or documentation changes — including the HIGH finding that transformed the PR from a cosmetic comment change into the substantive policy fix that closed #532. The bot's Evidence for existing issues
Code agent infrastructure failureThe code agent's failure on #532 was a sandbox OCI runtime error ( Proposals filed |
Summary
Fixes #532 by syncing
policies/fix.yamlpackage_registrieswithpolicies/code.yaml, and updating the header comment to describe shared needs without implying the two files must stay identical.Also fixes a pre-existing gap on
registry.yarnpkg.comin both policy files (allow_encoded_slash: true, same class of issue as #531) — called out in review because adding the yarn endpoint to fix without the flag would have copied the latent bug from code.Companion docs: fullsend-ai/fullsend#6111.
Changes
policies/fix.yaml(#532)registry.yarnpkg.comendpoint (withallow_encoded_slash: true)npx,yarn,yarnpkg, andpnpmbinary allowlist entriespolicies/code.yaml(review follow-up, d44ce69)allow_encoded_slash: trueto the existingregistry.yarnpkg.comendpoint — same Code agent can't access npmjs #531 rationale for scoped yarn paths; was already on main without the flagTest plan
package_registriesendpoints and binaries match code (baseline:mainat time of Fix agent policy gaps with code agent #532 fix, plus the sharedallow_encoded_slashcorrection on yarnpkg in both files)