Skip to content

fix(#2247): compare decoded text in shim drift detection#77

Open
guyoron1 wants to merge 28 commits into
mainfrom
mirror/2254-2247-fix-shim-stale-comparison
Open

fix(#2247): compare decoded text in shim drift detection#77
guyoron1 wants to merge 28 commits into
mainfrom
mirror/2254-2247-fix-shim-stale-comparison

Conversation

@guyoron1

@guyoron1 guyoron1 commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Mirror of upstream fullsend-ai#2254

Fixes shim drift detection to compare decoded text instead of raw bytes.

@fullsend-ai-coder
fix(fullsend-ai#2247): compare decoded text in shim drift detection
bff0e4b 
The stale-shim comparison in reconcile-repos.sh used
managed_content_b64() which decoded base64, extracted
managed content, then re-encoded to base64. Bash command
substitution strips trailing newlines, so the re-encoded
base64 could differ from the original even when the
decoded text was identical. This caused false-positive
drift detection, leading to bogus update PRs (e.g.
PR fullsend-ai#2101) that removed the sentinel lines.

Replace the base64-to-base64 comparison with decoded
text comparison: decode both sides, strip carriage
returns, extract managed content via extract_managed_
content, and compare the resulting strings directly.
For pre-sentinel shims (no sentinel found), fall back
to comparing full decoded content.

Add a regression test that verifies logically identical
content with different trailing newlines is not flagged
as stale.

Note: pre-commit could not run in sandbox (shellcheck
download blocked by network policy). The post-script
runs pre-commit authoritatively on the runner.

Closes fullsend-ai#2247
@guyoron1

guyoron1 commented Jun 21, 2026

Copy link
Copy Markdown
Owner Author

/fs-qf

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:02 PM UTC · Completed 3:12 PM UTC
Commit: 0535520 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Review

Reason: stale-head

The review agent reviewed commit 1607e80f7a2ec619356e5805568788b1eb3f15de but the PR HEAD is now cc5e08d5e2c67eba32a4f54864469d354cd00f78. This review was discarded to avoid approving unreviewed code.

Previous run

Review

Findings

High

  • [protected-path] CLAUDE.md — This PR deletes CLAUDE.md, which is a protected governance file. The PR title and body claim only to fix shim drift detection — the deletion is not mentioned or justified. Protected-path changes always require human approval, and this PR has no linked issue authorizing the deletion. If the deletion is intentional, it should be in a separate PR with explicit justification.
    Remediation: Restore CLAUDE.md or split the deletion into a separate, authorized PR with a linked issue explaining why the governance redirect is no longer needed.

  • [scope-creep] outputs/ — The PR adds 783 lines of QualityFlow STP artifacts across 4 new files (outputs/GH-77_test_plan.md, outputs/stp/GH-77/GH-77_stp_review.md, outputs/stp/GH-77/GH-77_test_plan.md, outputs/summary.yaml). These auto-generated test plan documents are unrelated to the 12-line production bug fix in reconcile-repos.sh. The outputs/ directory does not exist on main and appears to be ephemeral agent output that should not be committed.
    Remediation: Remove the outputs/ files from this PR. Consider adding outputs/ to .gitignore if it holds ephemeral agent artifacts.

Medium

  • [dead-code] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:150 — After this PR removes the two calls to managed_content_b64() (the only call sites), the function definition at lines 150–162 becomes dead code with zero remaining callers in the repository.
    Remediation: Remove the managed_content_b64 function in this PR or a follow-up cleanup PR.

  • [duplicate-content] outputs/GH-77_test_plan.md — The test plan file is duplicated identically at outputs/GH-77_test_plan.md and outputs/stp/GH-77/GH-77_test_plan.md (218 lines each, same blob hash).
    Remediation: Remove the duplicate (moot if the entire outputs/ directory is removed per the scope-creep finding above).

  • [stale-reference] docs/guides/user/customizing-with-agents-md.md:14 — This user guide recommends keeping CLAUDE.md lightweight and includes a code example showing CLAUDE.md content. With CLAUDE.md deleted from the fullsend repo, fullsend no longer dogfoods the recommended pattern, creating an inconsistency (though the guidance itself remains valid for enrolled repos).
    Remediation: Add a note that CLAUDE.md is optional, or restore CLAUDE.md in the fullsend repo.

Low

  • [logic-error] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:411 — The expected-side fallback [ -z "$EXPECTED_MANAGED" ] && EXPECTED_MANAGED="$EXPECTED_DECODED" is unreachable because the template always contains the sentinel line. This is a defensive guard, not a bug, but a clarifying comment would help.

  • [test-coverage-alignment] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh — Test 5 validates the no-drift case. Stale detection is covered by Test 1 (verified), but a cross-reference comment in Test 5 would aid maintainability.

  • [stale-reference] docs/architecture.md:577 — References CLAUDE.md in the pre-agent scan description. This describes product behavior on enrolled repos and remains accurate, but could note that CLAUDE.md is optional.

  • [test-isolation] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh:745 — Test 5 reuses the yq mock from initial setup (returning 4 repos) but only mocks gh responses for test-repo. Other repos fail silently via || true. The test passes correctly but could be more isolated.

  • [commit-message-convention] — PR title uses fix(#2247) but issue reconcile-repos.sh produces shim blob without sentinel, creating bogus update PR fullsend-ai/fullsend#2247 exists only in upstream fullsend-ai/fullsend, not in this fork.

Info

  • [trailing-newline-normalization] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:408 — The fix correctly leverages bash command substitution's trailing-newline stripping behavior (POSIX-defined) to normalize comparisons. The tr -d '\r' handles CR normalization separately. Both mechanisms are sound.

  • [architectural-context] — The fix reduces drift detection sensitivity: repos previously flagged as stale due to encoding differences will now be correctly recognized as up-to-date. This is the intended behavior per upstream issue reconcile-repos.sh produces shim blob without sentinel, creating bogus update PR fullsend-ai/fullsend#2247.

Previous run (2)

Review

Reason: stale-head

The review agent reviewed commit fad46472ad6d1870851f377a7367721a8829b6d6 but the PR HEAD is now 4c46925e07368aae9204f5235c8a073f56202fdd. This review was discarded to avoid approving unreviewed code.

Previous run (3)

Review

Reason: stale-head

The review agent reviewed commit 737030aed4d5ecf9f2aa3e2c1f9610aaac646f92 but the PR HEAD is now 4a11fb9a570a9afb149d6a3e180fb8cff4a9acee. This review was discarded to avoid approving unreviewed code.

Previous run (4)

Review

Reason: stale-head

The review agent reviewed commit 103585e8ac78fe7b2b67566d5119a1c37ca99340 but the PR HEAD is now 10095e9d8875c63b57fa9f0c47898173bff2a19e. This review was discarded to avoid approving unreviewed code.

Previous run (5)

Review

Reason: stale-head

The review agent reviewed commit 254d9af8ceea34b5190caf20ba6023871be2379e but the PR HEAD is now 103585e8ac78fe7b2b67566d5119a1c37ca99340. This review was discarded to avoid approving unreviewed code.

Previous run (6)

Review

Reason: stale-head

The review agent reviewed commit d83e032a9cbb5ad304ef8dfa3661188b22161284 but the PR HEAD is now 32fce47688a60d03c15163d17461c357da92cfe3. This review was discarded to avoid approving unreviewed code.

Previous run (7)

Review

Reason: stale-head

The review agent reviewed commit 040cb88565b472777d3d4744c42ccf95a8a43fea but the PR HEAD is now d83e032a9cbb5ad304ef8dfa3661188b22161284. This review was discarded to avoid approving unreviewed code.

Previous run (8)

Review

Reason: stale-head

The review agent reviewed commit 46421d77b49034564dfe39260afc8c092ae6bf27 but the PR HEAD is now 13f4a46dde85d2ab4b0377b86408b6dc76501092. This review was discarded to avoid approving unreviewed code.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

/fs-review

Add QualityFlow output for fullsend-aiGH-2247 [skip ci]
040cb88 
STD generation complete: 17 test scenarios (14 unit, 3 functional)
with Go/stdlib test stubs for design review.
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:23 PM UTC · Completed 3:33 PM UTC
Commit: 0535520 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:38 PM UTC · Completed 3:50 PM UTC
Commit: 0535520 · View workflow run →

QualityFlow and others added 2 commits June 21, 2026 15:45
Add test output for fullsend-aiGH-2247 [skip ci]
9ccc0ed 
Generated 17 working Go test implementations from STD YAML covering:
- Drift detection encoding normalization (scenarios 1-4)
- Sentinel preservation across code paths (scenarios 5-7)
- Pre-sentinel shim fallback logic (scenarios 8-10)
- Reconcile flow update PR lifecycle (scenarios 11-13)
- User-owned header preservation (scenarios 14-15)
- Base64 round-trip integrity (scenarios 16-17)

All tests pass: go test -v ./outputs/go-tests/fullsend-aiGH-2247/
@guyoron1
Add QualityFlow tests for fullsend-aiGH-2247
32fce47 
Replaces intermediate pipeline artifacts with organized test files.

Total: 7 test files → qf-tests/fullsend-aiGH-2247/
Jira: fullsend-aiGH-2247
[skip ci]
@github-actions

github-actions Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

QualityFlow Pipeline Summary

Stage Agent Status
1 STP Builder ✅
2 STP Reviewer ✅
3 STP Refiner ✅
4 STD Builder ✅
5 STD Reviewer ✅
6 STD Refiner ✅
7 Test Generator ✅

Test Output

Language Count Location
Go 7 files internal/scaffold/qf_*_test.go

Issue: GH-77

Generated by QualityFlow

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

/fs-review

@guyoron1

guyoron1 commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

/fs-qf

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:02 AM UTC · Completed 3:17 AM UTC
Commit: 73a8f7b · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:20 AM UTC · Completed 3:34 AM UTC
Commit: 73a8f7b · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:39 AM UTC · Completed 3:53 AM UTC
Commit: 73a8f7b · View workflow run →

QualityFlow and others added 2 commits June 22, 2026 03:43
Add QualityFlow tests for GH-77 [skip ci]
1469d16
@guyoron1
Clean QualityFlow artifacts for GH-77
4a11fb9 
Removes intermediate pipeline artifacts (STP, STD, reviews).
Test files (5) are co-located in source tree with qf_ prefix.
Jira: GH-77
[skip ci]
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

/fs-review

@guyoron1
chore: remove old qf-tests/ artifacts
fad4647 
Co-located tests (qf_* prefix) are now in source package directories.
The qf-tests/ directory contained non-compiling tests from the old pipeline.
@guyoron1

guyoron1 commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

/fs-qf

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:01 PM UTC · Completed 12:13 PM UTC
Commit: db171e5 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:15 PM UTC · Completed 12:28 PM UTC
Commit: db171e5 · View workflow run →

fullsend-ai-review[bot]
fullsend-ai-review Bot suggested changes Jun 22, 2026
View reviewed changes

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:33 PM UTC · Completed 12:45 PM UTC
Commit: db171e5 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

/fs-review

QualityFlow and others added 3 commits June 22, 2026 12:50
Refine STD for GH-77: resolve 5 findings (2 CRITICAL, 3 MAJOR)
b061b80 
- Add v2.1-enhanced required fields (tier, patterns, variables,
  test_structure, code_structure) to all 15 scenarios
- Remove related_prs from document_metadata (content policy)
- Replace duplicated common preconditions in Go stubs with
  STD reference

Verdict: NEEDS_REVISION -> APPROVED_WITH_FINDINGS (score 52->92)

[skip ci]
Add QualityFlow tests for GH-77 [skip ci]
9559260
@guyoron1
Clean QualityFlow artifacts for GH-77
0085a01 
Removes intermediate pipeline artifacts (STP, STD, reviews).
Test files (7) are co-located in source tree with qf_ prefix.
Jira: GH-77
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@fullsend-ai-review fullsend-ai-review[bot] fullsend-ai-review[bot] requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@guyoron1