Skip to content

fix: surface actionable validation step messages in push detail view#246

Merged
coopernetes merged 4 commits into
mainfrom
fix/scan-diff-step-content-and-consistency
May 13, 2026
Merged

fix: surface actionable validation step messages in push detail view#246
coopernetes merged 4 commits into
mainfrom
fix/scan-diff-step-content-and-consistency

Conversation

@coopernetes
Copy link
Copy Markdown
Member

@coopernetes coopernetes commented May 13, 2026 •
edited
Loading

Summary

  • Fixes bug: validation step expand panel shows s.content instead of error/blocked message #244: scanDiff step content was showing the branch ref (ref: refs/heads/...) instead of the blocked pattern and matching file — BlockedContentDiffCheck was producing violations with null formattedDetail, and DiffScanningHook was hardcoding the ref name as content
  • scanSecrets: both hook and filter now append a remediation hint (rotate + scrub history) to each finding so the dashboard gives actionable next steps
  • identityVerification WARN mode: now emits per-violation sideband warnings to the git client terminal so developers see the mismatch even when the push isn't blocked
  • URL rule step consistency: RepositoryUrlRuleHook now uses step name checkUrlRules (not class name); fixes a duplicate step bug where both validationContext.addIssue and pushContext.addStep(FAIL) were called, producing two entries in the push detail view; wording aligned with UrlRuleAggregateFilter
  • URL rules page display fixed: Repos.tsx was using the old slug/owner/name field shape; updated to target/value/matchType from the 34aebb9 backend refactor — every rule was showing as "any: *" and new rules created via UI were saving with value=null (never matching)
  • Admin connectivity crash fixed: SpringWebConfig sets NON_NULL Jackson inclusion globally, so null tls/http fields are omitted from JSON rather than serialised as null; the frontend strict === null check fell through to tls.status and crashed — changed to == null throughout and updated types to | undefined
  • Provider display: all provider dropdowns (Add Rule, Add Permission, Add SCM identity) and provider labels throughout the UI were showing host (e.g. github.com) instead of name (e.g. github); fixed across Repos.tsx, UserDetail.tsx, Profile.tsx; clone URL construction switched to use provider.proxyPath from the API response; Repos.tsx provider state typed as Provider[] instead of an ad-hoc inline type
  • Terminology: createAccessRule/deleteAccessRule → createUrlRule/deleteUrlRule in api.ts and Repos.tsx; "Access type" label → "Rule type" in the add-rule modal

Test plan

  • Unit tests: BlockedContentDiffCheckTest — assertions for formattedDetail content and deduplication with first matching line
  • Unit tests: ScanDiffFilterTest — assertion that errorMessage contains the blocked pattern and content includes the matching line
  • Server e2e tests: all 9/9 pass (proxy push flows, URL rule enforcement, secret scanning)
  • Dashboard LDAP e2e test failure is pre-existing and unrelated (timing/container issue on corporate network; passes on CI)
  • Manual: URL Rules tab shows correct target/value/matchType for each rule; new rules created via UI save and evaluate correctly
  • Manual: Admin → Provider Connectivity no longer crashes when tls/http are absent (HTTP provider or TCP failure path)
  • Manual: All provider dropdowns show friendly name (e.g. "github") not hostname (e.g. "github.com"); clone URL in active repos view is correct

🤖 Generated with Claude Code

coopernetes and others added 4 commits May 12, 2026 22:54
@coopernetes @claude
fix: surface actionable validation step messages in push detail view
962edc3 
Fixes scanDiff, scanSecrets, identityVerification WARN, and URL rule
validation steps so the dashboard push detail panel and git client terminal
both display specific, actionable error information instead of placeholder
values (e.g. branch refs or missing context).

Key changes:
- BlockedContentDiffCheck: track violations as Map<summary, firstMatchingLine>
  so formattedDetail includes the matching content, not null
- DiffScanningHook: pass violation.formattedDetail() to addIssue instead of
  hardcoding the branch ref as the content
- ScanDiffFilter: loop per violation (consistent with other filters) instead
  of collapsing all findings into a single step
- SecretScanningHook/SecretScanningFilter: append remediation hint to each
  finding's formattedDetail so the dashboard shows actionable next steps
- IdentityVerificationHook: emit per-violation sideband warnings in WARN mode
  so developers see the mismatch in their terminal even when push is not blocked
- RepositoryUrlRuleHook: fix duplicate step bug — when validationContext is set,
  PushStorePersistenceHook already creates the FAIL step from the issue, so the
  explicit pushContext.addStep() was creating a second FAIL entry; also align
  step name to "checkUrlRules" (was class name "RepositoryUrlRuleHook")
- UrlRuleAggregateFilter: align "not in allow list" wording and add NO_ENTRY
  symbol to NotAllowed title for parity with Denied case
- PushStorePersistenceHook: add "checkUrlRules" (order 100) to HOOK_STEP_ORDER;
  strip ANSI codes from issue.detail() at storage time
- GitProxyFilter.recordStep: strip ANSI from content before persisting; change
  null message args to empty strings throughout
- GitClientUtils.stripColors: make public for use in persistence hooks
- Rename createAccessRule/deleteAccessRule → createUrlRule/deleteUrlRule in
  api.ts and Repos.tsx to align with Java internals; update "Access type" label
  to "Rule type"

closes #244

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coopernetes @claude
fix: align URL rules frontend with target/value/matchType backend model
a34c56f 
The 34aebb9 refactor replaced slug/owner/name fields on AccessRule with
target (SLUG|OWNER|NAME), value, and matchType (LITERAL|GLOB|REGEX) but
the frontend was not updated, causing every rule to display as "any: *"
and new rules created via the UI to save with a null value (never matching).

- Rule interface: replace slug/owner/name with target/value/matchType
- Rule display: render target.toLowerCase() + value; add matchType to
  the secondary info line so users can see glob/regex/literal at a glance
- AddRuleModal: send target/value/matchType in the POST payload instead of
  slug/owner/name; drop the stale regex: prefix encoding (matchType carries
  the match semantics now)
- createUrlRule: update payload type to match AccessRule fields

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coopernetes @claude
fix: handle absent tls/http fields in connectivity response (NON_NULL…
03b7a66 
… Jackson)

SpringWebConfig configures Jackson with NON_NULL inclusion, so null values
are omitted from the JSON response. ConnectivityController sets tls=null for
HTTP providers and when TCP fails, but those null values are stripped before
reaching the browser. The frontend received tls=undefined, and the strict
null check (=== null) fell through to tls.status, crashing with TypeError.

- ProviderConnectivity: mark tls and http as optional (| undefined) in api.ts
- ConnectivityRow: use == null (covers both null and undefined) for tlsOk
- TlsBadge / HttpBadge: accept undefined in prop type, use == null guard

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coopernetes @claude
fix: display provider friendly name instead of hostname throughout UI
8cba7b5 
All provider dropdowns (Add Rule, Add Permission, Add SCM identity) and
provider labels (active repos list, rules list) were showing p.host
(e.g. "github.com") instead of p.name (e.g. "github"). Since 39f6887
enforced consistent provider IDs, name is the correct human-readable
identifier; host is an implementation detail of the upstream URI.

Also switch clone URL construction in the active repos view to use
provider.proxyPath from the API response rather than manually
concatenating /proxy/ + host — cleaner and less error-prone.

Repos.tsx: import Provider type and use it for the providers state
(previously typed as an inline partial) so proxyPath is available.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coopernetes coopernetes enabled auto-merge May 13, 2026 03:55
@coopernetes coopernetes merged commit 429339e into main May 13, 2026
16 checks passed
@coopernetes coopernetes deleted the fix/scan-diff-step-content-and-consistency branch May 13, 2026 03:56
@coopernetes coopernetes mentioned this pull request May 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: validation step expand panel shows s.content instead of error/blocked message

1 participant

@coopernetes