Repository navigation
feat(esign): signing UX decisions (2/2) - #92
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe e-sign service checks agent versions during job loading and signature preparation. It records delivery results only for the attempt that still owns the claim. It also sends request-outcome updates and signed-document emails with sender reply details. ChangesAgent version checks
E-sign email and delivery updates
Sequence Diagram(s)sequenceDiagram
participant EsignService
participant EsignStore
participant sendRequestUpdateEmail
participant RequestUpdateEmail
EsignService->>EsignStore: Record delivery result for claimed attempt
EsignStore-->>EsignService: Return whether attempt still owns claim
EsignService->>sendRequestUpdateEmail: Send outcome update when applicable
sendRequestUpdateEmail->>RequestUpdateEmail: Render request update
Priority: ➖ Normal Change: Feature Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 18 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title identifies the e-signing UX decision work and matches the pull request objectives. It is concise and sufficiently specific, although it does not list every supporting change. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 18 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
lorenzocorallo
force-pushed
the
feat/esign-ux-decisions
branch
from
October 22, 2026 01:23
55a5801 to
25c92fc
Compare
The dashboard's proxy now passes on the agent's `User-Agent: PoliNetwork-eSign/<version>`. Loading a job or preparing a signature with an agent older than MIN_AGENT_VERSION answers 426 agent_outdated and fails the job, so the signer learns it before typing the PIN and the dashboard can show why. A missing User-Agent, a development build or an unreadable version passes, so the dashboard, the backend and the agent can be deployed in any order; the protocol check at prepare stays as the safety net. The minimum is 0.2.0, the first agent with protocol 2, so no installed agent that works today is refused.
…ester
The "Signed:" email is read by offices and people outside PoliNetwork.
It becomes "Documento firmato: <title>", all in Italian: the
requester's message, then one fixed line saying the document was signed
with the CIE and by whom. The "attached" sentence appears only when the
requester wrote nothing, and the too-large variant links the OneDrive
copy whatever the message says.
The footer says who sent it ("Inviato da … tramite PoliNetwork") and,
instead of "do not reply", that replying writes to them: Reply-To is the
requester's address, set by both the Graph and the development sender.
A new "Request update" email tells a request's creator what they would otherwise only learn by opening the dashboard: - a signer declined: who, and the reason; - everyone signed, unless the creator signed last: where the signed PDF was saved and who it was emailed to; - saving or emailing the signed PDF failed: the step, where it was going and the error. It also goes to every manager who may retry. Nobody is emailed about what they just did (the decliner, the last signer, a manager retrying). Each outcome is reached by one call, so it is emailed once: a failed retry is seen by whoever retried, and a retry that works tells the creator. Sending never blocks the action and a failure is only logged.
The dashboard drops the official-app fallback from the request page. The endpoint stays so a dashboard deployed before that change keeps working; the comment says when it can go.
The agent did open the job before it was told to update. Marking it seen lets the dashboard tell an agent that must update from one that isn't running, instead of closing its window as unanswered.
The agent's build scripts stamp `git describe --tags --dirty` by default, so a build made after a tag reads like `v0.1.1-6-g0cb0f7b`. Counting it as its base tag refused builds that are newer than that tag and may already speak the current protocol: that build is the commit tagged v0.2.0. Once the minimum is raised, every source build made after the previous tag would have been locked out. A version with commits after its tag, or uncommitted changes, is now a development build like `dev` and passes; preparing a signature still refuses one without protocol 2. Releases and pre-releases are compared as before.
A delivery run that outlives its five-minute claim, for example on a hung OneDrive upload, lets the sweep claim the same delivery again. Both runs then wrote their result and each emailed the creator "Signed by everyone". The result is now written only where the claim is still the one this run took, and only then is the outcome emailed: the run that took over wins and the stale one is dropped with a warning. The stale run may already have saved or emailed a copy; that is unchanged.
The "Not delivered" email went to every manager who may see the request and told them they could retry it. Retrying goes through the dashboard, whose e-signing procedures also take esign:sign, so a manager without it was told to do something they couldn't. Who may retry now holds both permissions.
… last Saving or emailing the signed PDF runs in the background after the last signature, but the "Not delivered" email treated the last signer as the one who acted and skipped them. A creator who signed last, as on every request they sign alone, never learned the save failed; when they were also the only manager, nobody did. An attempt nobody asked for has no actor: the creator and every manager who may retry are told, the last signer included. "Signed by everyone" still skips a creator who saw it happen, by signing last or retrying, but now reaches one whose failed delivery another manager fixed. The email no longer opens with "Everyone signed", which read oddly for a request its creator signed alone: "The signed PDF of <title> couldn't be saved to <path>."
The template and the Graph sender were tested on their own, with a hand-written subject and Reply-To, so the glue between them could regress unnoticed. sendSignedDocumentEmail is now tested through both senders: the Italian subject, the attached PDF, Reply-To set from the requester, and left out when the requester has no address.
Agents released before the rename to PoliNetwork eSign (v0.1.0, v0.1.1) send `User-Agent: PoliNetwork-Sign-Agent/<version>`, so the version check read no version from them: their signers still got the PIN form and were refused only by the protocol check at prepare. Read the old prefix too, so they get agent_outdated before the PIN as the README says. Source builds and `dev` builds with that prefix still pass.
The agent hid "Requested by" when the requester's name matched the signer's, but the signer's name is the copy saved with the request and the requester's the current profile name: after a rename the row came back on someone's own request, and it was hidden for a different person with the same name. The job now sends requestedBy as null when the signer created the request, which the agent already hides.
lorenzocorallo
force-pushed
the
feat/esign-ux-decisions
branch
from
October 22, 2026 09:51
25c92fc to
61f6513
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second stacked PR, on top of #91. It builds the backend side of the decisions taken on the signing UX brief.
MIN_AGENT_VERSION = "0.2.0", read from the agent's User-Agent:PoliNetwork-eSign/<v>, orPoliNetwork-Sign-Agent/<v>for releases from before the rename.agent_outdated; its job is failed and counted as seen.v0.1.1-6-g…),dev, and requests with no User-Agent.requestedByis null on the signer's own request, so the agent hides "Requested by" by account, not by name.esign.requests.uploadSignedhas no caller in the dashboard; it is marked removable.bun run check,bun run typecheck,bun run testandbun run buildpass. I ran it end to end with the dashboard and the agent PRs, including a real v0.1.1 agent build, which is refused before the PIN.Merge order: #91 first. #91 is squash-merged, so this branch then needs a rebase onto
main(git rebase --onto main fix/esign-ux) before it can merge.