feat(client): tell the app when a credential's revocation status changes

[w-ensink]

What

For an SD-JWT VC, the wallet never told the app about a revocation. It wrote the
new status to the database and stopped there, so a credential revoked while the
app sat idle stayed rendered as valid until the user happened to do something that
re-read the list. For an idemix credential the signal did exist — Revoked(cred)
— but it repeated on every rediscovery.

This adds the missing signal, and takes the opportunity to give client its own
app-facing callback interface instead of borrowing irmaclient's.

• client.ClientHandler — client.New now takes this instead of
  irmaclient.ClientHandler. It carries only what the app acts on:
  CredentialsChanged, ReportError, and the enrollment and change-pin results.
  An unexported irmaHandler adapter implements the irmaclient surface and
  forwards, so IrmaClient's vocabulary stops leaking into the app.
• CredentialsChanged() — one signal for "the credentials you are showing
  are out of date", replacing UpdateAttributes and Revoked(cred). It now
  fires for revocations too, on both credential technologies.

Why one callback and not a revocation-specific one

The first cut had a separate RevocationStatusUpdated(). It was collapsed
because the app re-requests the whole credential list either way — a second
callback carried no information the app acted on differently. Revoked(cred)'s
credential identity is dropped for the same reason; IrmaClient still fires it
internally, so re-widening later is a one-line change in the adapter if a
per-credential notification is ever wanted.

The trade-off worth knowing: the app can no longer tell why its credentials
changed. Issuance, deletion, revocation, and a logo download are one signal.

Only a change fires it

A signal on every check would have the app re-reading its whole credential list
on a timer, forever. Both paths report transitions, not state:

• SD-JWT VC. ListInstancesWithStatusReference now also selects
  last_known_status, so the sweep can compare. RefreshStatuses returns
  (changed int, err error) — the number of batches whose status moved, since
  only one representative per batch is ever checked — and client.Client decides
  that > 0 is worth waking the app for. A re-confirmation, and a sweep whose
  fetches all failed, both count zero.
• Idemix. IrmaClient reports revocation as state: a revoked credential's
  witness never advances (gabi returns ErrorRevoked before touching it), so its
  staleness only grows and the periodic witness-update job rediscovers the same
  revocation every few tens of seconds. The adapter reports each credential once.

No irmaclient behaviour changes

irma/irmaclient is untouched apart from a counter on its test mock. The idemix
signal is derived entirely from the existing Revoked(cred) callback, which
already fires on exactly the right event — the adapter maps it and dedupes it.

Breaking — irmamobile must ship in lockstep

client.New takes a client.ClientHandler. The app must:

• rename UpdateAttributes → CredentialsChanged;
• fold whatever it did on Revoked(cred) into CredentialsChanged (it no longer
  receives which credential);
• drop UpdateConfiguration.

Client.RefreshStatuses(ctx) error keeps its signature.

Internal, no app impact: services.RevocationService.RefreshStatuses returns
(changed int, err error), and db.CredentialStatusInstance carries
LastKnownStatus.

Tests

• client/status_change_notification_test.go — the wiring, against a real
  Client with a seeded status-bearing batch and a real status server: a change
  wakes the app, a re-confirmation does not, a known revocation is not
  re-reported, and a caller who cancels gets no callback. Plus a unit test that
  the adapter reports each idemix revocation once.
• internal/sessiontest/revocation_test.go — TestRevocation*/NotifiesApp,
  a real idemix revocation at a revocation-enabled server, driven through
  client.Client. Needs the docker-compose postgres, like its 22 siblings.
• internal/sessiontest/openid4vci_statuslist_test.go — the SD-JWT counterpart,
  a real credential revoked at the status-list agent. In the opt-in status-list
  group (needs the full docker stack), verified by hand.
• eudi/services/credential_status_refresh_test.go — changed counts: zero on
  re-confirmation, zero on failure, one per batch rather than per row.

Every notification path was mutation-checked — each assertion fails when the
code under it is disabled.

Follow-up

Client.Close can still close storage while a background job is running against
it, because no wallet job can be cancelled. A jobCtx fixing this for the status
sweep alone was written and reverted — it buys an inconsistent contract for one of
five job families. Doing it properly means covering all five, and picking up two
existing TODOs (InitJobs, IrmaClient.Close) and the fact that
common.HTTPClient has no timeout along the way.

[dobby-coder]

Thanks for the PR! I'm on it — reviewing the new client.ClientHandler interface, the irmaHandler adapter, and the consolidated CredentialsChanged() signal (including that it now fires correctly for revocations on both credential technologies). I'll post my review shortly.

[dobby-coder]

Reviewed the diff, ran the repo's lint gates locally, and reproduced the one blocking finding. Requesting changes on that one; the rest are small.

The design reads right, and the write-up is honest about its trade-offs. This closes a real gap: the status sweep used to be storage-only, so an SD-JWT VC revocation never reached a foregrounded app at all. Coalescing to a single CredentialsChanged and deriving the idemix signal from the existing Revoked callback — rather than touching irmaclient — both look like the right calls, and the "only a change fires it" invariant is pinned from three angles.

Blocking — a status change can be committed and then never signalled. Inline at client/client.go:310. Reproduced deterministically at the service layer (cancellation landing after a writeback commits, which is what a slow later group or a mid-sweep shutdown does): changed=1, err=<nil>, ctx.Err()=context canceled, persisted=StatusInvalid, and the next sweep on a clean ctx returns changed=0. The revocation is in the database, the app is never told, and no later sweep will tell it. Worth noting err comes back nil, so the caller cannot see that the sweep was cut short either.

Two findings that have no line inside a diff hunk to anchor to:

• client/client.go:274 — SetLocale's doc comment still says the backfill "signals ClientHandler.UpdateAttributes on completion". That method is no longer on ClientHandler; the backfill now fires CredentialsChanged (client.go:259).
• The follow-up link in the PR description, docs/plans/cancellable-background-jobs.md, 404s: docs/ on this branch holds only keyshareFlow.puml, and there is no docs/plans/ directory. Either the file was left out of the commit or the link should point somewhere else.

Checked and clean: gofmt -d, go vet over the changed packages, go fix -diff (the modernizer gate), and misspell -error over the changed prose — all four steps of the lint job pass. Unit tests pass for client and eudi/..., including under -race. internal/sessiontest was not run here (no docker daemon available), so the two new integration tests are unverified from my side.

[dobby-coder]

Reviewed this and posted a Changes requested review with 5 inline comments plus 2 body notes: #653 (review)

The design is sound and this closes a real gap, but there's one blocking bug I reproduced deterministically rather than just flagging on read: in client/client.go:310, a status change persisted while ctx is cancelled never gets signalled, and no later sweep catches it — so a revoked credential can render as valid indefinitely, and err comes back nil so the caller can't even tell the sweep was cut short.

Smaller findings:

• CredentialsChanged's doc comment says it fires on issue-or-delete, but deletion and EUDI issuance paths stay silent
• client.New doesn't validate the now-required handler, so a nil handler crashes a background goroutine instead of no-oping
• CHANGELOG.md still has a sibling entry documenting the method this PR renames (sits outside the diff, so easy to miss)
• a nit on the revocation sweep possibly double-firing against a UI-initiated one
• plus two body-only notes: a stale UpdateAttributes doc reference and a follow-up doc link that 404s

Verification-wise: gofmt/go vet/go fix/misspell are all clean, and the client/eudi unit suites pass including under -race. I couldn't run the sessiontest integration suite (no docker in my environment), so the two new integration tests are unverified on my end.

[w-ensink]

/dobby fix your comments and check again until you consider it good enough. Then approve the PR.

[dobby-coder]

On it — I'll go through and address the outstanding review comments on this PR, then re-check my changes and iterate until things look solid.

[dobby-coder]

Approving. This clears my own changes-requested from the last cycle. I fixed the five findings on the branch rather than handing them back, in two commits, and replied on each thread.

91129fb is the blocking one. RefreshStatuses now signals on any change it recorded, whatever ctx ended up as. The writeback is already committed by the time the old guard dropped the callback, so the next sweep read the new status back as a re-confirmation and the app never heard about the revocation. TestRefreshStatusesNotifiesOnChangeCommittedBeforeCancellation drives that through a real Client, with a store that cancels the sweep's own ctx the moment a writeback commits; under the old guard it fails, expected 1, got 0. Its counterpart keeps the cancel-before-entry case, which stays silent because nothing was written back.

The service side went with it: a fetch aborted by cancellation is no longer a fail-soft per-URI skip, so cancellation landing in the last group's fetch returns the cancellation instead of nil. The same commit adds the nil-handler check in New and fixes SetLocale's comment, which still named UpdateAttributes.

a55d5b1 is documentation only. CredentialsChanged now lists what fires it, and says deletion and OpenID4VCI issuance are silent. The logo-backfill CHANGELOG entry names CredentialsChanged(), so the two ## Unreleased entries agree on the method that ships. The wiring is unchanged: an app does not need waking for a change it made itself.

Two things I did not change, both stated on their threads. The overlapping-sweep nit stays: a mutex would make a UI refresh wait behind a scheduled sweep on a client with no HTTP timeout, and moving change detection into the store would make this PR's LastKnownStatus column redundant. Both cost more than the symptom, which is one extra list re-read. And docs/plans/cancellable-background-jobs.md is still missing, so the link in the description 404s. That one is yours: the file is in no commit, and the write-up is your reading of five job families, not something for me to invent. Either add it or drop the link.

Verification at a55d5b1: all 18 checks green. The test job ran the two integration tests I could not run last cycle and both pass, the idemix one on all three databases (TestRevocation{Postgres,MySQL,SQLServer}/NotifiesApp) and the SD-JWT VC one against the real status-list agent. Locally, client and eudi/services pass under -race, and each of the three new tests fails when I disable the code it covers. gofmt, go vet, go fix -diff, ineffassign and misspell are clean. The keyshare and sessiontest failures I see locally are pre-existing: no postgres and no docker daemon in my environment, and keyshare/tasks fails the same way on master.

Merge timing is yours, and irmamobile still has to ship in lockstep for the client.ClientHandler change.

[dobby-coder]

Approving. Every finding from my last cycle is closed on this HEAD, and all 16 checks are green.

I re-verified the four fixes against 85021d34 rather than taking the replies at face value:

• RefreshStatuses (client/client.go:319) signals on changed > 0 with no ctx guard left. Re-ran TestRefreshStatusesNotifiesOnChangeCommittedBeforeCancellation: the cancellingStore really does cancel from inside a committed writeback, so the test drives the state the old code got wrong.
• New rejects a nil handler before it touches storage, with TestNewRejectsNilHandler on it.
• ClientHandler.CredentialsChanged's doc now lists idemix issuance, a status move and a finished logo, and names deletion and OpenID4VCI issuance as silent. That matches what actually fires in-tree.
• The ## Unreleased logo-backfill line names CredentialsChanged(), so the two entries agree. SetLocale's doc comment came along with it, and the docs/plans/... link is gone from the description.

The overlapping-sweep nit stays where the last round left it — declined, for a reason I agree with: a mutex around a sweep that fetches over a timeout-less common.HTTPClient trades one redundant re-read for a bridge goroutine that can block for as long as a socket stays open.

Checks I ran here: gofmt -d, go vet, go fix -diff ./... and misspell -error (all four lint steps) are clean, and client, eudi/... and internal/testhelpers unit tests pass. go test -c builds internal/sessiontest under its default !local_tests tag, so both new integration tests compile; the docker-backed runs are CI's, and CI's test plus both integration-test-* jobs are green on this sha.

Not a finding for this PR: go test -race ./irma/irmaclient/ fails on TestKeyshareChangePin — a package-level write in irma.NewHTTPTransport (irma/transport.go:79) racing the scheme auto-updater. It reproduces on master (d5a83aa5) in two runs out of three, CI does not pass -race, and nothing in this diff sits on either side of it. Pre-existing.

One non-blocking note inline.

[dobby-coder]

Done — I re-verified all four fixes and the one declined item against HEAD 85021d34 (didn't just trust the previous reply), re-ran the four lint gates and unit tests locally, and confirmed all 16 CI checks are green. I've posted an APPROVE review with one non-blocking inline note on representative-row stability in the status sweep (eudi/services/revocation_service.go:157).

Two things to flag:

• The repo's required-review setting still needs a human maintainer's approval to unblock the merge — my approval alone doesn't satisfy it.
• This is a breaking change for irmamobile (client.New now takes client.ClientHandler), and there's no companion PR there yet.

PR: #653