fix(#621)!: tighten head_sha schema to require 40 or 64 hex chars - #622
Conversation
The head_sha field in review-result.schema.json used a loose
minLength: 7 constraint while the CLI enforces a strict regex
requiring 40 or 64 hex characters. This divergence allowed
schema-valid values that failed at runtime, causing unnecessary
agent fix iterations (e.g. fullsend#5654).
Replace {"minLength": 7} with the CLI's regex pattern
{"pattern": "^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$"} to catch
invalid SHAs at schema validation time.
Update test fixtures in validate-output-schema-test.sh (8 fixtures
using 7-char "abc1234") and post-review-test.sh (23 fixtures using
6-char "abc123") to use valid 40-char hex strings. Add two negative
test cases: short SHA rejected, non-hex SHA rejected.
BREAKING CHANGE: The head_sha field now requires exactly 40 or 64
hex characters. Previously any string of 7+ characters was accepted
by the schema. The CLI already enforced this constraint at runtime,
so no working end-to-end workflows are affected.
Closes #621
|
🤖 Finished Review · ✅ Success · Started 12:16 PM UTC · Completed 12:30 PM UTC |
ReviewFindingsMedium
Low
Labels: PR modifies review agent schema validation and agent definition. Previous runReviewFindingsMedium
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 1:15 PM UTC · Completed 1:20 PM UTC |
Update the head_sha field description in agents/review.md from "Commit SHA (min 7 chars)" to "Commit SHA (40 or 64 hex chars)" to match the new pattern validation in review-result.schema.json. Addresses review feedback on #622
🔧 Fix agent — iteration 1 (human-triggered)Fixed 1 of 3 review findings. Updated stale documentation in agents/review.md to reflect the tightened head_sha schema constraint. Disagreed with 2 informational findings (out-of-scope external verification and protected-path note). Fixed (1):
Disagreed (2):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 1:21 PM UTC · Completed 1:33 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 1:41 PM UTC · Completed 1:55 PM UTC |
Retro: PR #622 — Tighten head_sha schema to require 40 or 64 hex charsTimeline
What went well
Evidence for existing open issuesAll significant improvement opportunities from this workflow are already tracked:
AssessmentOnce the issue was in the correct repo, the pipeline worked well. The primary quality gap — the code agent missing a documentation update — required one fix iteration but was caught by the review agent. The 6-day delay from cross-repo misfiling was the most significant waste, and is already tracked in #624. No new proposals are warranted. |
Update the head_sha field description in agents/review.md from "Commit SHA (min 7 chars)" to "Commit SHA (40 or 64 hex chars)" to match the new pattern validation in review-result.schema.json. Addresses review feedback on #622
Update the head_sha field description in agents/review.md from "Commit SHA (min 7 chars)" to "Commit SHA (40 or 64 hex chars)" to match the new pattern validation in review-result.schema.json. Addresses review feedback on fullsend-ai#622
1 participant
Summary
head_shavalidation inreview-result.schema.jsonfrom{"minLength": 7}to{"pattern": "^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$"}, aligning the schema with the CLI'shexSHAReregex ininternal/cli/postreview.govalidate-output-schema-test.shand 23 fixtures inpost-review-test.shthat used short SHAs ("abc1234"/"abc123") to use valid 40-character hex stringsContext
The schema and CLI enforced different constraints on the same
head_shafield. A code agent produced"0000000"which passed schema validation but failed at the CLI step (fullsend post-review), triggering an avoidable fix iteration on PR fullsend-ai/fullsend#5654. This change catches invalid SHAs at schema validation time, preventing that class of failures.Breaking change
The
head_shafield now requires exactly 40 or 64 hex characters. Previously any string of 7+ characters was accepted by the schema. The CLI already enforced this constraint at runtime, so no working end-to-end workflows are affected.Testing
bash scripts/validate-output-schema-test.sh— all tests pass (including 2 new negative cases)bash scripts/post-review-test.sh— all tests pass with updated fixturesCloses #621
Post-script verification
agent/621-tighten-head-sha-schema)4ea342abf067696961d0df7b922e0498e332a371..HEAD)