Skip to content

refactor(remediation): demote remediate_secret_incidents to a data tool - #166

Open
mathieubellon wants to merge 2 commits into
mainfrom
refactor/demote-remediate-tool-to-data
Open

refactor(remediation): demote remediate_secret_incidents to a data tool#166
mathieubellon wants to merge 2 commits into
mainfrom
refactor/demote-remediate-tool-to-data

Conversation

@mathieubellon

@mathieubellon mathieubellon commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Why

We ship a high-quality triage + remediation doctrine skill in agent-skills, which bundles this MCP server. But the server also exposed remediate_secret_incidents — a tool that fetched occurrences and rendered a static remediation prompt. Two remediation-shaped things in the same runtime, peers by name, giving different advice. Agents reached for the MCP tool instead of the skill. Its prompt also contradicted GitGuardian's own IR guidance: it never mentioned rotation and led with git-history rewriting.

On closer inspection the tool also had a data-model problem: it returned a flat list of occurrences across all incidents in the repo, globally trimmed to the first 10 — incident-blind. One noisy incident could consume the whole budget and silently hide the others; it was neither "all incidents" nor "the occurrences needed to fix one incident."

What

Two commits:

1. Demote to a data tool (kill the collision)

  • Rename remediate_secret_incidentslist_remediation_targets (module, tool, models). No back-compat alias — that would just re-seed the collision, and agents discover tools dynamically.
  • Honest docstring/description: returns occurrence data only; does not rotate or modify files; defer to the remediation skill/workflow.
  • Rewrite remediation_prompt.md into a minimal rotation-first fallback (for standalone MCP users): leads with rotate/revoke, gates git-history rewriting behind unpushed-only, and explicitly defers to a skill/workflow when present.

2. Make it incident-centric

  • Overview (no incident_id): group the underlying occurrences query by incident, most-recent-first (ordering=-date). One RemediationTarget per incident with a per-incident occurrence sample (max_occurrences_per_incident, default 10), the true total_occurrence_count, and an occurrences_truncated flag. Incidents capped at max_incidents (default 20); total_incident_count + truncated surface anything dropped.
  • Drill-down (incident_id): return that incident's complete occurrence set via get_incident(with_occurrences=100) so an agent can fully remediate it.
  • Replaces the incident-blind occurrences[:10] global trim.
  • Also corrects a stale description claim (the tool defaulted to mine=False / all incidents, but claimed the opposite).

Notes

  • Breaking: tool renamed, and the result shape changed from {occurrences, occurrences_count, ...} to {incidents: [{incident_id, detector, occurrences, total_occurrence_count, ...}], incident_count, total_incident_count, truncated}.
  • The "current branch" scoping stays at the occurrence level (DEFAULT_BRANCH tag), so the overview remains a single cheap paginated call.

Test

635 passed, 4 skipped; ruff + format clean on changed files.

Rename the doctrine-emitting remediate_secret_incidents tool to
list_remediation_targets and reframe it as a pure occurrence finder.
This removes the semantic collision with the agent-skills remediation
skill, which agents were reaching past in favor of the MCP tool.

- Rename tool + module + models (RemediateSecretIncidents* ->
  ListRemediationTargets*); result field remediation_instructions ->
  guidance, suggested_occurrences_for_remediation_count ->
  suggested_occurrences_count.
- Honest docstring/description: returns occurrence data only, does NOT
  rotate or modify files; defer to the remediation skill/workflow.
- Rewrite remediation_prompt.md as a rotation-first fallback that leads
  with rotate/revoke, gates git-history rewriting behind unpushed-only,
  and explicitly defers to a skill/workflow when present.
- Drop the now-unused git_commands/create_env_example/add_to_env params.
- Propagate rename through register_tools instructions, conftest,
  run_tool script, and tests; add a rotation-first guidance assertion.
mathieubellon added a commit to GitGuardian/agent-skills that referenced this pull request Jul 6, 2026
The ggmcp remediate_secret_incidents tool is a read tool (returns repo
occurrence data) but its name reads as an action and, in current released
ggmcp, it also returns a static remediation_instructions template that omits
rotation and leads with git-history rewriting — the doctrine's anti-pattern.
Agents were calling it and following its output instead of entering this
skill's rotation-first doctrine.

Add an interception note in SKILL.md "Start Here" and tighten the step-3
reference in triage-workflow.md: name the tool (and its newer
list_remediation_targets rename), mark it read-only occurrence data, and tell
the agent to ignore its returned instructions as the plan — the doctrine
drives. Covers both ggmcp versions while GitGuardian/ggmcp#166 (which renames
the tool and rewrites the template rotation-first) is in flight.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reshape the tool from a flat, globally-trimmed occurrence feed into an
incident-centric view, so an agent can reason about and fully remediate
one incident at a time.

- Overview (no incident_id): group the underlying occurrences query by
  incident, most-recent-first (ordering=-date). Return one
  RemediationTarget per incident with a per-incident occurrence sample
  (max_occurrences_per_incident, default 10) plus the true
  total_occurrence_count and an occurrences_truncated flag. Cap incidents
  at max_incidents (default 20) and surface total_incident_count +
  truncated so nothing is dropped silently.
- Drill-down (incident_id): return that incident's COMPLETE occurrence
  set via get_incident(with_occurrences=100) for full remediation.
- Replaces the old incident-blind occurrences[:10] global trim, which
  could exhaust the budget on one noisy incident and hide others.
- Fix the tool description (also corrects the stale 'defaults to mine'
  claim; default is mine=False) and rewrite tests around the new shape.
@linear

linear Bot commented Jul 6, 2026

Copy link
Copy Markdown

SI-3767

mathieubellon added a commit to GitGuardian/agent-skills that referenced this pull request Jul 6, 2026
…93)

* feat: overlay customer custom remediation workflow onto the doctrine

When a workspace has a configured remediation workflow (fetched via the
get_remediation_workflow MCP tool, id present), follow it as the spine of the
remediation deliverable and use the doctrine to fill in per-secret mechanics and
verification under each customer step. Falls back to doctrine-drives when the
workspace is on the default workflow or the tool is unavailable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat: cover pre-leak touchpoints of the custom remediation workflow

The custom remediation workflow is configured per touchpoint (Incident page,
pre-commit, pre-push, pre-receive). The pre-leak touchpoints reach the agent
through ggshield's own CLI output (>= 1.30.0), not the MCP endpoint, so the
overlay applies to scan-secrets and install-hooks too.

- triage doctrine: correct the §13 framing (it covers the Incident-page
  touchpoint; the earlier "no counterpart in scan-secrets" claim was wrong) and
  revert the header divergence note (§13 is now parallel in both doctrines).
- scan-secrets doctrine: add a parallel §13 (two delivery channels, same
  customer-wins precedence) plus a §5 pointer.
- scan-secrets SKILL.md + interpreting-results.md: surface a custom remediation
  message in ggshield output verbatim; fill blanks with doctrine mechanics.
- install-hooks SKILL.md: same guidance for a blocked commit/action.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: gate custom-vs-default remediation workflow on id, never on steps

A live triage session mislabeled the default remediation workflow as a
workspace-configured custom one — it inferred "custom" from the presence of
steps rather than from the id gate, then blended in generic advice. The default
workflow always ships a full steps[], so steps prove nothing; only workflow.id
(with created_at/updated_at) marks a configured custom workflow.

- §13 + triage-workflow step 4: make the gate explicit and mandatory to announce
  ("returned id -> custom" vs "no id -> default"); hard rule to gate on
  workflow.id presence, never on steps; never call a no-id result "custom" or
  "configured by your workspace"; add the Incident-page touchpoint caveat
  (custom Pre-commit/Pre-push/Pre-receive messages surface via ggshield, not
  here); align the doc field list with the nested workflow.* payload.
- Add three §13 evals (payloads embedded inline so they grade headlessly, no
  OAuth): custom (id present -> verbatim spine), default (rich steps but no id
  -> doctrine drives), and the exact pathological session payload (no
  mislabel-as-custom). Eval 6 catches the reported bug directly: without the
  skill the model calls the default "custom" every time (0%), with it 100%.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: on the default remediation workflow, drive purely from the doctrine

The Incident-page overlay already gated custom-vs-default on workflow.id,
but on the default branch it still told the agent to render the returned
steps as "useful scaffolding". Those default steps are GitGuardian's generic
placeholder, not workspace-authored guidance, and adding them alongside the
doctrine only muddies the deliverable.

Set the default steps aside entirely: on no-id (or an absent tool), drive the
remediation end-to-end from the doctrine exactly as if the tool had returned
nothing. Use the fetched workflow only when it is custom (id present). Updates
SKILL.md, remediation-doctrine.md §13, triage-workflow.md, and the two
default-branch evals to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: stop remediate_secret_incidents from hijacking the triage doctrine

The ggmcp remediate_secret_incidents tool is a read tool (returns repo
occurrence data) but its name reads as an action and, in current released
ggmcp, it also returns a static remediation_instructions template that omits
rotation and leads with git-history rewriting — the doctrine's anti-pattern.
Agents were calling it and following its output instead of entering this
skill's rotation-first doctrine.

Add an interception note in SKILL.md "Start Here" and tighten the step-3
reference in triage-workflow.md: name the tool (and its newer
list_remediation_targets rename), mark it read-only occurrence data, and tell
the agent to ignore its returned instructions as the plan — the doctrine
drives. Covers both ggmcp versions while GitGuardian/ggmcp#166 (which renames
the tool and rewrites the template rotation-first) is in flight.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: drop upstream-rename reference from the remediate_secret_incidents note

The interception note is a present-tense local fix; it should not reference an
unshipped ggmcp change. Remove the list_remediation_targets rename mention from
both SKILL.md and triage-workflow.md and state it plainly: this tool is
read-only occurrence data, ignore its remediation_instructions, do not take its
remediation guidance into account — the doctrine drives.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: cover the remediate_secret_incidents interception in evals

The default-workflow fix was already guarded (evals 6, 7), but nothing tested
that the agent ignores remediate_secret_incidents' returned remediation
guidance. Add eval 8: hand the agent a realistic tool payload (occurrence data
plus the rotation-absent template) and assert it uses only the occurrence
locations, does not adopt the tool's instructions as the plan, and drives
rotation-first from the doctrine for a valid pushed credential.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant