Skip to content

fix(#272): remove signing from convention discovery lists - #296

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/272-remove-signing-discovery
Jul 22, 2026
Merged

fix(#272): remove signing from convention discovery lists#296
rh-hemartin merged 1 commit into
mainfrom
agent/272-remove-signing-discovery

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

The fix agent non-deterministically adds Signed-off-by trailers because two skill files contain contradictory instructions. The convention-discovery steps tell agents to learn "signing" conventions from target repos, while later prohibitions forbid git commit -s. When a target repo requires DCO signing, the agent sometimes follows the discovered convention instead of the prohibition, wasting the entire run.

This PR removes "signing" from the convention discovery lists in both affected skills:

  • skills/fix-review/SKILL.md: (message format, signing)(message format)
  • skills/code-implementation/SKILL.md: signing requirements, message formatmessage format

The existing "NEVER use git commit -s" prohibitions and post-script hard gates remain as defense-in-depth.

Testing

  • make test passes (one pre-existing failure in post-triage-test.sh unrelated to this change)
  • make check-bundle passes
  • Verified that no other references to "signing" remain in the skills directory
  • Verified that the "NEVER use git commit -s" prohibitions still exist in both files

Closes #272

Post-script verification

  • Branch is not main/master (agent/272-remove-signing-discovery)
  • Secret scan passed (gitleaks — fc7b83e1e64b3d786b2d500189c142759fa06451..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

The fix and code agents non-deterministically add Signed-off-by
trailers because both skills instruct agents to discover "signing"
conventions from target repos. When a target repo requires DCO
signing, the agent sometimes follows the discovered convention
instead of the later "NEVER use git commit -s" prohibition.

Remove "signing" from the convention discovery bullet in both
skills/fix-review/SKILL.md and skills/code-implementation/SKILL.md.
The existing prohibitions and post-script hard gates remain as
defense-in-depth.

Closes #272
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 20, 2026 07:15
@rh-hemartin

Copy link
Copy Markdown
Member

/fs-review

@rh-hemartin rh-hemartin self-assigned this Jul 22, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:39 AM UTC · Completed 6:47 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md, skills/fix-review/SKILL.md — Both changed files are under the skills/ protected path. The PR links to issue Fix agent repeatedly fails with Signed-off-by post-script block on PR #5273 #272 and clearly explains the rationale: removing "signing" from convention discovery lists to prevent non-deterministic Signed-off-by trailer failures. The change is minimal and precisely scoped to the issue's suggested fix. Human approval is required for protected-path changes regardless of context.

Labels: PR modifies skills used by code-agent and fix-agent

@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 22, 2026
Merged via the queue into main with commit 8002158 Jul 22, 2026
26 of 27 checks passed
@rh-hemartin
rh-hemartin deleted the agent/272-remove-signing-discovery branch July 22, 2026 07:07
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 7:10 AM UTC · Completed 7:20 AM UTC
Commit: 5652e3a · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #296 — remove signing from convention discovery lists

Timeline

Time (UTC) Event Outcome
Jul 18 01:24 Issue #272 filed (fix agent failing with Signed-off-by post-script block) Bug confirmed: conflicting skill instructions
Jul 18 01:24–01:28 Triage agent run 29625126807 ✅ Confirmed root cause, applied ready-to-code
Jul 18 01:28–01:34 Code agent run 29625259693 ✅ Correct 2-line fix, created PR #273. ⚠ ready-for-review label failed to apply
Jul 18–Jul 20 PR #273 sits unreviewed No auto-review dispatched (label failure)
Jul 20 06:09 Human closes PR #273: "This should be fixed over at fullsend-ai/agents" ❌ Erroneous — PR was already on fullsend-ai/agents
Jul 20 06:11–06:23 Retro agent run 29720872073 on PR #273 ✅ Identified closure as erroneous, filed #294
Jul 20 07:08–07:15 Code agent run 29723681242 (manual /fs-code) ✅ Identical fix, created PR #296. ⚠ Same label failure
Jul 22 06:38 Human triggers /fs-review, immediately approves Human approved before review agent finished
Jul 22 06:38–06:47 Review agent run 29897410124 ✅ Medium protected-path finding, comment verdict
Jul 22 07:07 PR #296 merged ✅ Issue #272 closed

Assessment

What went well:

What was wasted:

Root causes and existing coverage

Both root causes that led to the wasted work are already tracked by existing open issues:

  1. ready-for-review label failure — The code agent's post-script failed to apply the label on both runs, preventing automatic review dispatch. PR fix(#272): remove signing from convention discovery lists #273 sat unreviewed for 2 days. This specific failure on the agents repo itself is strong evidence that the label provisioning gap is a real problem.

    • agents#274: Post-code.sh should auto-create ready-for-review label before applying it
    • agents#199: Post-code script should create the label if it doesn't exist in the target repo
    • agents#351: Rename ready-to-review to ready-for-review to match dispatch routing
    • fullsend#5187: Enrollment scaffold should provision required workflow labels
  2. PR body confusion — PR fix(#272): remove signing from convention discovery lists #273's summary led with a cross-repo reference (fullsend-ai/fullsend#5273), which likely primed the human reviewer to think the fix was on the wrong repo. This was caught by the retro agent and fixed via agents#294 (now closed, merged alongside PR fix(#272): remove signing from convention discovery lists #296).

No new proposals

All identified improvement opportunities are covered by existing issues. The label failure (agents#274/#199/#351) is the highest-impact fix — resolving it would have prevented the entire cascade of erroneous closure, wasted code run, and multi-day delay on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix agent repeatedly fails with Signed-off-by post-script block on PR #5273

1 participant