fix(agentic-engineering): validate Surveyor JSON fields at source - #177
Conversation
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git status --short
git show --no-ext-diff --format=fuller --stat 8a553428f7c0d04caf76e3de3de1fb7fede6f74b
printf '\n--- changed paths ---\n'
git diff-tree --no-commit-id --name-status -r 8a553428f7c0d04caf76e3de3de1fb7fede6f74b
printf '\n--- patch ---\n'
git show --no-ext-diff --format= 8a553428f7c0d04caf76e3de3de1fb7fede6f74b
printf '\n--- shell file outlines ---\n'
git diff-tree --no-commit-id --name-only -r 8a553428f7c0d04caf76e3de3de1fb7fede6f74b \
| while IFS= read -r path; do
case "$path" in
*.sh)
echo "### $path"
ast-grep outline "$path" --lang bash --items all || true
;;
esac
doneLength of output: 16440 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- validator function and resource loop ---'
sed -n '430,545p' scripts/validate-manifests.sh
sed -n '955,1065p' scripts/validate-manifests.sh
printf '%s\n' '--- validator test helpers and complete fixture setup ---'
sed -n '1,180p' scripts/validate-manifests.test.sh
sed -n '610,710p' scripts/validate-manifests.test.sh
sed -n '880,990p' scripts/validate-manifests.test.sh
printf '%s\n' '--- Surveyor structure and JSON-related instructions ---'
rg -n -i -C 4 --glob '*.md' 'gh\s+.*--json|--json|JSON field|JSON vocabulary|QUERY-UNKNOWN' \
plugins/agentic-engineering/agents/portfolio-surveyor.agent.md
printf '%s\n' '--- desired-state references to portfolio-surveyor ---'
rg -n -C 3 '"portfolio-surveyor"|definitionSha256|portfolio-surveyor\.agent\.md' \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json \
scripts/validate-manifests.sh scripts/validate-manifests.test.shLength of output: 63193 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Surveyor definition ---'
cat -n plugins/agentic-engineering/agents/portfolio-surveyor.agent.md
printf '%s\n' '--- all JSON and GitHub CLI guidance in the plugin ---'
rg -n -i -C 3 --glob '!*.lock' \
'(gh[[:space:]].*--json|--json|field list|JSON (field|vocabulary)|gh run list|gh search prs)' \
plugins/agentic-engineering
printf '%s\n' '--- changed JSON files parse and marketplace versions agree ---'
jq -e . .claude-plugin/marketplace.json >/dev/null
jq -e . .github/plugin/marketplace.json >/dev/null
jq -e . plugins/agentic-engineering/plugin.json >/dev/null
jq -e . plugins/agentic-engineering/.claude-plugin/plugin.json >/dev/null
jq -e . plugins/agentic-engineering/resources/provider-neutral.desired-state.json >/dev/null
jq -r '.plugins[] | select(.name == "agentic-engineering") | .version' \
.claude-plugin/marketplace.json .github/plugin/marketplace.json
jq -r '.version' \
plugins/agentic-engineering/plugin.json \
plugins/agentic-engineering/.claude-plugin/plugin.json
printf '%s\n' '--- changed Surveyor digest ---'
sha256sum plugins/agentic-engineering/agents/portfolio-surveyor.agent.md
jq -r '.spec.roles["portfolio-surveyor"].definitionSha256' \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
printf '%s\n' '--- shell parser checks ---'
bash -n scripts/validate-manifests.sh
bash -n scripts/validate-manifests.test.shLength of output: 94927
I verified the Surveyor contract, validator check, four negative fixtures, manifest versions, and desired-state digest. The changed JSON files parse. The changed shell files pass syntax checks. ✅ Action performedReview finished.
|
@coderabbitai review |
|
Warning Review limit reachedNext included review available in 45 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)The repo ships **two marketplace manifests that must stay byte-for-byte in sync** (modulo key order):📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (8)
📝 WalkthroughWalkthroughThe portfolio-surveyor agent now requires subcommand-local Merge Risk: ⚪ Minimal · up to The PR adds localized Surveyor field validation and updates the plugin contract/version metadata; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation 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 2 files. (6 skipped: 6 unsupported.) 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 |
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8da2a5078
ℹ️ 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".
Corroborating the open P1 from a second, independent direction — not a takeover; this branch is the The same guard blocked a different mandated operation for me this run. The surveyor is required to The script takes positional arguments rather than stdin, so there is no third shape to try. The It cost real delivery today. So there are now two measured manifestations of one root cause: a required diagnostic shape that the Verified rather than reasoned: both denials above are exact strings observed this run, and the exit-0 |
The surveyor definition added in this PR REQUIRES a bare `--json` vocabulary probe before any ad hoc JSON read, but the guard shipped alongside it classified `--json` as unconditionally value-taking and denied that exact command. The PR therefore prescribed a command its own guard rejected, so every ad hoc read would have fallen through to QUERY-UNKNOWN. Admit `--json` as the FINAL word only: gh prints the subcommand's field list and exits nonzero without contacting the forge. Every other value flag still needs its value, and a `--json` that HAS a value is unchanged. The desired state pins this script's bytes, so its sha256 moves in the same commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='8a553428f7c0d04caf76e3de3de1fb7fede6f74b'
head='fd1fb375ad3a08fef7751bdb8ed1899a91a889c1'
printf '%s\n' '--- commit ancestry and summary ---'
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=true' || echo 'base-is-ancestor=false'
git show --no-ext-diff --format=fuller --stat "$head"
printf '%s\n' '--- changed paths since prior review ---'
git diff --no-ext-diff --name-status "$base" "$head"
printf '%s\n' '--- patch since prior review ---'
git diff --no-ext-diff --find-renames "$base" "$head"
printf '%s\n' '--- shell outlines for changed shell files ---'
git diff --no-ext-diff --name-only "$base" "$head" |
while IFS= read -r path; do
case "$path" in
*.sh)
echo "### $path"
ast-grep outline "$path" --lang bash --items all || true
;;
esac
done
printf '%s\n' '--- locate guard and exemption contract references ---'
rg -n -i -C 4 \
'(read-only allowlist|forge command|bare[[:space:]]+--json|programmed-bot-review-exemption|QUERY-UNKNOWN|portfolio-surveyor)' \
. --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**'Length of output: 107160 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- gh flag parser and gh command dispatch ---'
sed -n '830,1000p' plugins/agentic-engineering/scripts/forge-readonly-guard.sh
printf '%s\n' '--- command segmentation and consumer classifier handling ---'
sed -n '1440,1565p' plugins/agentic-engineering/scripts/forge-readonly-guard.sh
printf '%s\n' '--- guard test harness and nearby test cases ---'
sed -n '1,125p' plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh
printf '%s\n' '--- Surveyor mandated gh commands ---'
rg -n -C 2 --glob 'portfolio-surveyor.agent.md' \
'(^|[[:space:]])gh[[:space:]]|--json' plugins/agentic-engineering/agents
printf '%s\n' '--- integrity and static syntax checks ---'
bash -n plugins/agentic-engineering/scripts/forge-readonly-guard.sh
bash -n plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh
bash -n scripts/validate-manifests.sh
bash -n scripts/validate-manifests.test.sh
jq -e . plugins/agentic-engineering/resources/provider-neutral.desired-state.json >/dev/null
guard_hash=$(sha256sum plugins/agentic-engineering/scripts/forge-readonly-guard.sh | awk '{print $1}')
surveyor_hash=$(sha256sum plugins/agentic-engineering/agents/portfolio-surveyor.agent.md | awk '{print $1}')
jq -r '.spec.resources[] | select(.path == "scripts/forge-readonly-guard.sh") | .sha256' \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
printf '%s\n' "$guard_hash"
jq -r '.spec.roles["portfolio-surveyor"].definitionSha256' \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
printf '%s\n' "$surveyor_hash"
printf '%s\n' '--- all call sites of the gh flag parser ---'
rg -n -C 2 '\bcheck_gh_verb_flags\b' plugins/agentic-engineering/scripts/forge-readonly-guard.shLength of output: 20362 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- desired-state resource schema and protected entries ---'
jq -r '
paths(scalars) as $p
| select(($p | map(tostring) | join(".")) | test("forge-readonly-guard|portfolio-surveyor"))
| "\($p | map(tostring) | join(".")) = \(getpath($p) | tostring)"
' plugins/agentic-engineering/resources/provider-neutral.desired-state.json
printf '%s\n' '--- full command parsing and segment loop ---'
sed -n '1560,1645p' plugins/agentic-engineering/scripts/forge-readonly-guard.sh
rg -n -C 3 '^(tokenize_segment|split_command|parse_command|classify_segment|main)\(\)' \
plugins/agentic-engineering/scripts/forge-readonly-guard.sh
printf '%s\n' '--- static syntax and digest parity ---'
bash -n plugins/agentic-engineering/scripts/forge-readonly-guard.sh
bash -n plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh
bash -n scripts/validate-manifests.sh
bash -n scripts/validate-manifests.test.sh
jq -e . plugins/agentic-engineering/resources/provider-neutral.desired-state.json >/dev/null
expected_guard_hash=$(jq -r '
.. | objects
| select(.path? == "scripts/forge-readonly-guard.sh")
| .sha256
' plugins/agentic-engineering/resources/provider-neutral.desired-state.json)
actual_guard_hash=$(sha256sum plugins/agentic-engineering/scripts/forge-readonly-guard.sh | awk '{print $1}')
expected_surveyor_hash=$(jq -r '.spec.roles["portfolio-surveyor"].definitionSha256' \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json)
actual_surveyor_hash=$(sha256sum plugins/agentic-engineering/agents/portfolio-surveyor.agent.md | awk '{print $1}')
printf 'guard: expected=%s actual=%s\n' "$expected_guard_hash" "$actual_guard_hash"
printf 'surveyor: expected=%s actual=%s\n' "$expected_surveyor_hash" "$actual_surveyor_hash"
printf '%s\n' '--- changed files and line statistics relative to prior reviewed commit ---'
git diff --check 8a553428f7c0d04caf76e3de3de1fb7fede6f74b fd1fb375ad3a08fef7751bdb8ed1899a91a889c1
git diff --stat 8a553428f7c0d04caf76e3de3de1fb7fede6f74b fd1fb375ad3a08fef7751bdb8ed1899a91a889c1Length of output: 4745
The guard permits bare Static syntax checks passed.
|
@coderabbitai review |
|
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Readiness at 1. Programmatically tested. RED at 2. Reviewed. Codex green at this head — CodeRabbit was tried first, per lane priority, and refused twice at this head ( 3. Tried and evaluated as a user — the complete mandated workflow, which was unreachable before this fix:
The two literal forms this repo's consumer contract prescribes — Scope note: the position conjunct ( |
1 participant
Fixes #176.
Upstream repair for devantler-tech/monorepo#3049.
Evidence
H59 moved in the wrong direction after its consumer-wide rule: the latest 24-hour corpus contains 11 genuine
Unknown JSON fieldfailures across 3 Claude sessions, versus the 1-per-46-session baseline. All 11 were Surveyor calls that supplied an unsupported field togh run list. This run independently reproduced the same cross-subcommand transfer ongh search prs, confirming the rule was too far from the caller.Safety and cross-lane coordination remain
QUERY-UNKNOWN; this PR makes no positive claim about either companion floor.Repair
--json.QUERY-UNKNOWN, never an empty result.Verification
scripts/validate-manifests.shrejected the unchanged entrypoint.scripts/validate-manifests.shpassed.