[gorlix]

Summary

When a Git provider plugin explains why it refused something ("The connected project token covers only acme/ui. Add a connection for acme/api..."), Paca now shows that explanation under its generic error in project Git settings. Error codes and behaviour without a provider message are unchanged.

Why

• resolveThroughConnections turned every not-found/forbidden answer into a bare ErrNotVisible / ErrAccessNotGranted, dropping the provider's Message.
• The webhook panel always showed the fixed "probably lacks the permission..." text, even when the provider knew the real reason.

Changes

• API: gitdom.DetailedError wraps a sentinel with the provider's text (errors.Is still matches; Error() unchanged). gitsvc.ProviderMessage redacts secrets, strips control characters, caps at 500 characters. The presenter exposes it as error_details.provider_message. The webhook retry endpoint adds it to the not-found/forbidden/conflict mappings. Agent-facing git operations are untouched (provider text still stays out of transcripts).
• Web: gitProviderMessage() reads the detail; GitSettings shows it as plain text (never HTML) in the link, reconnect and webhook-retry errors. New project.git.errors.providerDetail string in all 9 locales.
• Docs: docs/plugins/git-provider-spi.md notes that these messages are shown to the user.

Tests

• Go: provider message propagated and errors.Is(err, ErrNotVisible) still true; no message leaves the bare sentinel/no error_details; redaction and cap; presenter and handler mapping.
• Web (vitest): details shown as plain text for the not-visible and webhook cases, absent when the platform said nothing; gitProviderMessage unit tests; locale parity.
• go vet, go test ./..., tsc -b, biome check pass.

Refs #656

🤖 Generated with Claude Code

[pullfrog]

Important

The webhook-retry path still hides the provider's reason for the exact errors the linked issue calls out (validation / unsupported). One functional gap to close before merge.

Reviewed changes

• Linked/reconnect errors carry provider text — resolveThroughConnections wraps ErrNotVisible / ErrAccessNotGranted in a new gitdom.DetailedError, and presenter.Error emits its detail as error_details.provider_message while status/code and Error() stay unchanged.
• Redaction and cap — gitsvc.ProviderMessage masks credentials, drops control characters, trims, and caps at 500 runes; errors.As still matches the sentinel beneath the wrapper.
• Webhook retry details — RegisterWebhook now uses failWithProviderMessage, attaching the provider message for not_found / forbidden / conflict only.
• Web rendering — gitProviderMessage reads the detail; a new ProviderDetail renders it as plain text under the generic message in the link, reconnect and webhook-retry alerts; the new locale string exists in all 9 locales.
• Docs — git-provider-spi.md documents that these messages are user-visible (redacted, capped 500).

I ran go test for internal/service/git, internal/transport/http/presenter, and internal/transport/http/handler — all pass.

ℹ️ The webhook notice still asserts the likely cause

project.git.webhook.description still says the token "probably lacks the permission to manage webhooks" and is rendered unconditionally above the failure alert. Now that the real reason is shown underneath, a panel for a validation/unsupported failure contradicts itself: the static copy names a permission problem while the detail says, for example, that the instance has no public URL. Since the PR's stated "Why" is that this fixed string is misleading, consider softening it (or replacing it when a provider detail is present) so the notice does not lead with a cause the provider just refuted.

Technical details

# Soften the fixed webhook copy when a provider detail is shown

## Affected sites
- apps/web/src/i18n/locales/*/projects.json — `project.git.webhook.description` still states the token likely lacks permission.
- apps/web/src/components/projects/settings/GitSettings.tsx:700-715 — description is rendered before the `{failed ? ...}` block, so it is always visible.

## Required outcome
- The webhook notice must not assert a permission cause when a provider detail explaining a different cause is displayed.

## Suggested approach (optional)
- Reword the description to a neutral "Paca couldn't register a webhook on this repository" and keep the actionable permission hint as secondary text, or hide the hint once `detail` is non-null.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[gorlix]

Addressed the review in 0656c7b and 035cdf3:

• A webhook retry now attaches provider_message for every provider code except internal, so validation and unsupported reasons (unreachable URL, no public URL) reach the settings page. Handler tests cover both.
• The webhook notice no longer asserts a permission cause: the description is neutral, and the Maintainer/Webhooks hint shows only while the platform hasn't given its own reason (all 9 locales).

[pullfrog]

✅ No new issues found.

Reviewed changes

Both commits since the prior review close the two comments from that pass: the webhook-retry path now carries the provider's message for every code, and the notice no longer asserts a permission cause once the platform has said otherwise. Nothing new to flag.

• Every provider code surfaces its message — failWithProviderMessage now attaches error_details.provider_message for all provider codes except internal, instead of only not_found/forbidden/conflict, so validation and unsupported webhook failures (ensureWebhook's "no public URL…", an unreachable URL) reach the UI; handler tests cover both.
• Neutral webhook copy with a conditional hint — project.git.webhook.description became the neutral "Paca couldn't register a webhook…", and the actionable permission guidance moved to a new webhook.permissionHint string rendered only when no provider detail is present (failed && detail ? null : hint); locale parity holds across all 9 files.
• Docs — git-provider-spi.md now states any failed EnsureWebhook retry (any code except internal) is shown to the user.

I ran go test for internal/service/git, internal/transport/http/presenter, and internal/transport/http/handler — all pass. biome check is clean on the changed web files (web vitest isn't runnable in this environment — no node_modules).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[gorlix]

Added 2ff41a7 and 443b551. Repositories carried over by the plugin migrations (Paca-AI/paca-plugin-gitlab#6) arrive without a webhook that Paca has ever tried to register, and the notice blamed token permissions for them. ListRepositories now reports webhook_attempted, read from the existing webhook.register audit rows (one query per listing, no migration). A repository never attempted shows "Paca hasn't registered a webhook on this repository yet… Register webhook" without the permission hint; after a failed attempt the notice is as before. All 9 locales.

[pullfrog]

✅ No new issues found.

Reviewed changes

The two commits since the prior review make the webhook notice tell "never tried" apart from "tried and failed":

• webhook_attempted flows end to end — ListRepositories populates gitdom.Repository.WebhookAttempted from a new WebhookAttempts store method that reads git_action_log (tool = 'webhook.register'), surfaced as webhook_attempted on GitRepositoryResponse and consumed by the web GitRepository type. repository_id is ON DELETE SET NULL, so an unlinked repository drops out of the set.
• The notice no longer guesses at a cause it never observed — WebhookNotice computes neverTried = repository.webhook_attempted === false && !failed, switching the description to webhook.descriptionNotTried and the button to webhook.register, and hiding the permission hint until an attempt has actually been made. New descriptionNotTried / register keys exist in all 9 locales; an unset field (older API) falls back to the failure wording, which is the safe default.
• Tests — a postgres store test for the audit-log query, service tests for the listing distinction, and web tests for both the never-attempted and attempted-and-failed wordings.

go build ./... and go test for internal/service/git, internal/transport/http/handler, internal/transport/http/presenter, and internal/repository/postgres all pass. Web vitest is not runnable here (no node_modules).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pikann]

LGTM! Please resolve the merge conflicts before merging.
Thank you for your contribution! 🚀

[gorlix]

Merge conflicts resolved in b78bbeb (merged master: both sides had added to the same places for #657 and #658). Go tests, lint, tsc, biome and the web tests pass locally.

[pullfrog]

✅ No new issues found.

Reviewed changes

The only commit since the prior review is the merge of upstream/master (b78bbeb7), which resolves the conflicts the human reviewer flagged. It folds four upstream PRs into the branch without disturbing the provider-message feature:

• Repository descriptions now coexist with provider messages — GitSettings.tsx carries both the upstream link-form description field / RepositoryDescription block and this PR's ProviderDetail / WebhookNotice; GitRepositoryResponse maps both Description and WebhookAttempted; the Store port keeps WebhookAttempts beside the new SetRepositoryDescription.
• Other upstream commits merged in — the task-ref parser fix (#657), the change-request state fix for edited/synchronized events (#659), and the ACP cancel change (#655) land in service.go alongside the untouched resolveThroughConnections wrapping (ProviderMessage → WithDetail).
• Tests and locales merged without loss — service_test.go keeps TestLinkRepository_ProviderMessageRidesAlongNotVisible / TestProviderMessage next to upstream's TestFindTaskRefs; the en locale carries both the provider-detail strings and the description strings.

go build ./... and go test for internal/service/git, internal/transport/http/{dto,handler,presenter,router,middleware}, and internal/repository/postgres all pass; go vet compiles the changed packages' tests cleanly. Web vitest is not runnable here (no node_modules).

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}