Skip to content

fix(#2245): add conflict-resolution guidance to intent-coherence sub-agent#2246

Closed
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/2245-intent-coherence-conflict-resolution
Closed

fix(#2245): add conflict-resolution guidance to intent-coherence sub-agent#2246
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/2245-intent-coherence-conflict-resolution

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

The intent-coherence sub-agent had no guidance for resolving conflicts between a linked issue and existing design specs. When a PR implemented issue-authorized behavior that deviated from a prior spec, the agent treated the spec as authoritative and raised false-positive scope-creep and architectural-violation findings (observed on PR #2020).

Add a "Resolving conflicts between linked issue and design specs" section after "Exploration budget" that establishes the linked issue as the current decision when it explicitly authorizes spec-deviating behavior. Issue-authorized deviations should produce info-level spec-drift findings (so the spec can be updated) rather than high-severity scope-creep findings that block the PR. Unauthorized deviations — changes beyond what the issue describes — remain scope creep.

Note: make lint could not run due to shellcheck installation failure in the sandbox (network restriction). The change is markdown-only and does not affect Go code or tests.


Closes #2245

Post-script verification

  • Branch is not main/master (agent/2245-intent-coherence-conflict-resolution)
  • Secret scan passed (gitleaks — a2652eaaf81409406b42337c2481651b18bd6665..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…agent

The intent-coherence sub-agent had no guidance for resolving conflicts
between a linked issue and existing design specs. When a PR implemented
issue-authorized behavior that deviated from a prior spec, the agent
treated the spec as authoritative and raised false-positive scope-creep
and architectural-violation findings (observed on PR #2020).

Add a "Resolving conflicts between linked issue and design specs"
section after "Exploration budget" that establishes the linked issue as
the current decision when it explicitly authorizes spec-deviating
behavior. Issue-authorized deviations should produce info-level
spec-drift findings (so the spec can be updated) rather than
high-severity scope-creep findings that block the PR. Unauthorized
deviations — changes beyond what the issue describes — remain
scope creep.

Note: make lint could not run due to shellcheck installation failure
in the sandbox (network restriction). The change is markdown-only
and does not affect Go code or tests.

Closes #2245
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

Site preview

Preview: https://1502d155-site.fullsend-ai.workers.dev

Commit: d546bcf76d729228bdfc85376c720ba138627d6d

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:51 PM UTC · Completed 5:01 PM UTC
Commit: 718439e · View workflow run →

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two questions:

  • Is spec-drift meant to be recognized outside this sub-agent? Does it need special handling in the post script? Unsure. Need to check.
  • There's some overlap or should be some interaction with "docs-currency" here. If the proposed change contradicts an ADR, then the change must include a new ADR superseding that ADR (or a tweak to the ADR's text, whichever is more appropriate), so that the in-repo intent is updated to match the issue's intent as a part of the change.

We don't want to merge something that will introduce spec-drift into the repo!

Should we change this term? Maybe it should be "spec-change" to indicate that the spec here is changing, which then implies that review should ensure that documented in-repo specs are updated to be consistent with the proposed change.

@ralphbean

Copy link
Copy Markdown
Member

/fs-fix

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [spec-change] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md — The implementation deviates from issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245's proposed specification in two ways: (1) the category name is spec-change rather than the proposed spec-drift, and (2) the implementation adds medium-severity findings (when the PR does not update the affected spec) alongside info-level findings, whereas the issue proposed info-level only. Both choices are reasonable refinements — spec-change is arguably more descriptive and the medium-severity tier for un-updated specs encourages keeping documentation current. However, since the category name is part of a routing table contract (SKILL.md step 3a), the deviation from the authorizing issue's naming should be a deliberate, documented choice.
    Remediation: Either align with issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245's naming (spec-drift) or document the rationale for the rename in the PR description or a comment on the issue.

Info

  • [edge-case] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md:63 — The guidance says to treat the linked issue as authoritative "for the behavior it describes" even when the issue references a spec indirectly, but does not provide a tie-breaking rule for cases where the issue's intent regarding spec override vs. bug fix is ambiguous. An issue saying "improve uninstall behavior" could mean either "change the spec-documented behavior" or "fix a bug where the implementation doesn't match the spec." This could lead to inconsistent classification between spec-change and scope-creep. Not actionable for this PR but useful for future iteration.

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md — The new conflict-resolution guidance establishes a priority order: linked issue > design spec when they conflict. This is a substantive behavioral change to the intent-coherence sub-agent, authorized by issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245.

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:199 — The routing table update adds spec-change to the intent-coherence category list, which is architecturally coherent with the new sub-agent guidance and resolves the prior review's consumer-completeness finding.

  • [doc-style] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md:45 — The new section uses consistent formatting with existing sub-agent files: sentence-case headers, bullet-list structure, and inline code formatting for technical terms.

Previous run

Review

Findings

Medium

  • [consumer-completeness] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:199 — The new spec-drift category introduced in intent-coherence.md is not listed in the orchestrator's category-to-dimension routing table (step 3a). The intent-coherence row lists 13 categories but spec-drift is absent. On re-reviews, when the orchestrator routes prior findings to sub-agents by category, a spec-drift finding will not be recognized as belonging to intent-coherence. The fallback rule sends unrecognized categories to "the nearest matching dimension by keyword, or to correctness as a fallback," meaning intent-coherence would never see its own prior spec-drift findings during re-reviews, breaking severity anchoring for that category.
    Remediation: Add spec-drift to the intent-coherence category list in the routing table at SKILL.md.

Info

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md:45 — The new conflict-resolution guidance establishes a priority order: linked issue > design spec when they conflict. This is a substantive change to review agent behavior, not just clarification. The prior behavior (treating specs as authoritative) was implicit. The new guidance is authorized by issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245. No action required — this finding is for transparency.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 12, 2026
@fullsend-ai-coder

fullsend-ai-coder Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 5:03 PM UTC · Completed 5:10 PM UTC
Commit: 4e21a60 · View workflow run →

… in PR

Rename the `spec-drift` category to `spec-change` to convey that the
spec is actively changing (not drifting), and update guidance to require
PRs to include spec/ADR updates when deviating from existing design
docs. Also register `spec-change` in the SKILL.md routing table so
the orchestrator routes prior findings to intent-coherence on re-reviews.

Addresses review feedback on #2246

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 3 findings: renamed spec-drift to spec-change, updated guidance to require spec updates within the PR (not deferred), and registered the new category in the SKILL.md routing table.

Fixed (3):

  1. rename spec-drift to spec-change (internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md): Renamed spec-drift to spec-change throughout the conflict-resolution section per reviewer feedback. The new name conveys that the spec is actively changing rather than passively drifting.
  2. require spec updates in PR instead of deferring (internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/intent-coherence.md): Updated guidance to require PRs to include spec/ADR updates when deviating from existing design docs. If the PR includes the update, spec-change is info-level; if not, it's medium-severity to prompt the author to add it. This prevents merging changes that introduce documentation inconsistency.
  3. spec-drift not in SKILL.md routing table (internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md): Added spec-change to the intent-coherence category list in the routing table so the orchestrator routes prior spec-change findings back to intent-coherence on re-reviews.

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:13 PM UTC · Completed 5:22 PM UTC
Commit: d546bcf · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 12, 2026
@ralphbean

Copy link
Copy Markdown
Member

I'm underconfident that we're doing the right thing here. I'm going to defer this until we have better evals and functional test cases for agents in place.

@ralphbean ralphbean closed this Jun 12, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 12, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:28 PM UTC · Completed 6:37 PM UTC
Commit: d546bcf · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2246 — conflict-resolution guidance for intent-coherence sub-agent

Outcome: Closed without merging. The human maintainer was underconfident in the approach and deferred until better evals/test cases exist.

Timeline:

  1. Issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245 created → triage agent applied ready-to-code (6 min)
  2. Code agent opened PR fix(#2245): add conflict-resolution guidance to intent-coherence sub-agent #2246 with spec-drift category naming (~8 min)
  3. First review correctly caught that spec-drift wasn't registered in the SKILL.md routing table (medium finding)
  4. Human reviewed with CHANGES_REQUESTED: rename spec-driftspec-change, ensure specs are updated in-PR
  5. Human triggered /fs-fix → fix agent addressed all 3 findings correctly
  6. Second review raised a false positive: flagged that spec-change deviated from issue Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245's proposed spec-drift naming — but the human had explicitly requested this rename
  7. Human closed PR, citing desire for better evals first

What went well: Triage-to-PR pipeline was fast (~14 min). First review caught a real gap (routing table). Fix agent correctly addressed all feedback including the human-requested rename.

Key issue identified: The review agent's intent-coherence sub-agent has no mechanism to incorporate human review feedback when evaluating spec compliance. It treats the linked issue as the sole source of truth for authorized scope, so when a human requests changes that deviate from the issue's original proposal, the review agent flags them as unauthorized deviations. This produced a false-positive medium finding on the post-fix review.

Related existing issues: #664 (incorporate human reviews into assessment) covers the broad problem. The proposal below addresses the specific intent-coherence gap. The human's decision to close was strategic (wanting better evals) and is not actionable as an agent improvement.

Skipped proposals:

  • The code agent's failure to run make lint (shellcheck install failure in sandbox) is an environment issue, not an agent logic issue. No proposal filed.
  • The human's strategic decision to defer is not an agent-addressable concern.

Proposals filed

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

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue

1 participant