Reflow and wrap long string literals across multiple SaaS modules (logging messages, SQL/JPQL queries, email/content headers, and agreement text) and relocate tools.jackson imports for consistent grouping. These are formatting-only changes to improve line lengths and readability; no functional logic was altered.
Normalize indentation/whitespace for multi-line string concatenations in app/core/.../FormFillController.java and app/proprietary/.../AccountLinkClientTest.java. Pure formatting changes only; no behavior or logic modified.
# Description of Changes
## The problem
The portal's query client was created per mount:
```ts
const [queryClient] = useState(createPortalQueryClient);
```
The portal is a route (`/processor/*`, a lazy element), and the switch
to the editor is a client-side `navigate()`. So leaving the processor
unmounts `PortalApp`, the client goes with the component, and the cache
goes with the client. Coming back refetches everything, whether or not
anything changed: four requests for the Users page alone (roster,
grants, teams, auth config), and 21 `useQuery` sites across the portal.
The editor's client sits above the router in `AppProviders` and survives
the same trip. The round trip only ever cost in one direction.
## The fix
The module already kept the instance in a module-level slot so
`tryGetPortalQueryClient()` could find it. It just replaced it on every
mount instead of reusing it, so the change is to create it lazily and
hand out the same one:
```ts
export function getPortalQueryClient(): QueryClient {
current ??= new QueryClient({ defaultOptions: { queries: baseQueryOptions } });
return current;
}
```
Still a separate instance from the editor's. The two namespace their
keys apart (`["portal", ...]` against `["editor", ...]`) and invalidate
independently, which this does not change.
## What this does not do
`gcTime` is 5 minutes, from the shared `baseQueryOptions`. An entry with
no observer is still collected on that timer, so this warms a quick trip
to the editor and back, not a return after a long editing session.
Raising the portal's `gcTime` is a separate decision and is not made
here.
## Why it is safe
**Signing out.** A cache that outlives a mount must not outlive a
session, because the portal's holds the admin roster, emails and roles.
Logout goes through `window.location.assign`, a full page load, so the
whole JS context is discarded and no cache can survive it. Nothing in
the codebase calls `queryClient.clear()` on sign-out, and nothing needs
to. If logout ever becomes a client-side navigation, this needs an
explicit reset, and `resetPortalQueryClient()` is the hook for it.
**The one caller of the null check.** `resolveTeam` in
`saas/portal/usersBackend.ts` uses `tryGetPortalQueryClient()` and falls
back to a direct fetch when there is no client, which its comment
describes as the unit-test path; the cache path is preferred because it
honours both `staleTime` and invalidation. A longer-lived client means
the preferred path is taken more often, not less.
## Testing
Three tests in `queryClient.test.tsx`, and the first two fail if the
client goes back to being created per call:
| | |
|---|---|
| A remount is served from cache rather than refetching | the behaviour
this changes |
| Every caller gets the same instance | the mechanism |
| No client is reported until the portal first mounts | the contract
`resolveTeam` reads |
The three existing portal caching suites called the factory expecting a
fresh client per case. They now call `resetPortalQueryClient()` in a
`beforeEach`, which is what keeps `sharing.test.tsx`'s "a later screen
refetches nothing" case honest rather than passing on a leaked cache.
`task frontend:check` passes typecheck, lint and oxfmt, and 2402 of 2404
editor tests. The two failures, `workbenchSession.test.ts` and
`notificationActions.test.tsx`, are untouched here and fail the same way
on `main`.
> [!WARNING]
> Cooldown could not be applied because no publication date was
available from the registry.
>
Bumps the eclipse-temurin group with 1 update in the /docker/backend
directory: eclipse-temurin.
Bumps the eclipse-temurin group with 1 update in the /docker/base
directory: eclipse-temurin.
Bumps the eclipse-temurin group with 1 update in the /docker/embedded
directory: eclipse-temurin.
Updates `eclipse-temurin` from `fbcf915` to `b4c93a5`
Updates `eclipse-temurin` from `fbcf915` to `b4c93a5`
Updates `eclipse-temurin` from `fbcf915` to `b4c93a5`
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
> [!WARNING]
> Cooldown could not be applied because no publication date was
available from the registry.
>
Bumps the ubuntu group with 1 update in the /docker/base directory:
ubuntu.
Bumps the ubuntu group with 1 update in the /docker/unoserver directory:
ubuntu.
Updates `ubuntu` from `561618e` to `33ceb71`
Updates `ubuntu` from `561618e` to `33ceb71`
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Auto-generated by stirlingbot[bot]
This PR updates the frontend license report based on changes to
package.json dependencies.
Signed-off-by: stirlingbot[bot] <stirlingbot[bot]@users.noreply.github.com>
Co-authored-by: stirlingbot[bot] <195170888+stirlingbot[bot]@users.noreply.github.com>
# Description of Changes
This PR removes workflow-specific cache suffixes from Python dependency
caching in several CI workflows.
Previously, the following workflows appended their own `cache-suffix`
even though they use the same Python dependency files:
- `ai-engine.yml`
- `check-generated-models.yml`
- `pre_commit.yml`
- `sync_files_v2.yml`
All of these workflows use the same cache dependency inputs:
- `engine/pyproject.toml`
- `engine/uv.lock`
The workflow-specific suffixes caused separate cache entries to be
created for effectively identical dependency sets. This resulted in
unnecessary cache duplication and reduced cache reuse between workflows.
By removing the suffixes, these workflows can now share the same cache
when their dependency inputs and other cache key components match.
This change reduces redundant cache storage, improves cache hit
potential across CI workflows, and avoids repeatedly creating equivalent
caches under different names.
No functional application behavior is changed. The modification only
affects CI cache key generation and reuse.
---
## Checklist
### General
- [ ] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [ ] I have read the [Stirling-PDF Developer
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md)
(if applicable)
- [ ] I have read the [How to add new languages to
Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md)
(if applicable)
- [ ] I have performed a self-review of my own code
- [ ] My changes generate no new warnings
### Documentation
- [ ] I have updated relevant docs on [Stirling-PDF's doc
repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/)
(if functionality has heavily changed)
- [ ] I have read the section [Add New Translation
Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags)
(for new translation tags only)
### Translations (if applicable)
- [ ] I ran
[`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md)
### UI Changes (if applicable)
- [ ] Screenshots or videos demonstrating the UI changes are attached
(e.g., as comments or direct attachments in the PR)
### Testing (if applicable)
- [ ] I have run `task check` to verify linters, typechecks, and tests
pass
- [ ] I have tested my changes locally. Refer to the [Testing
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing)
for more details.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bumps the iconify group with 1 update in the /frontend directory:
[@iconify-json/material-symbols](https://github.com/iconify/icon-sets).
Updates `@iconify-json/material-symbols` from 1.2.83 to 1.2.89
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/iconify/icon-sets/commits">compare
view</a></li>
</ul>
</details>
<br />
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Description of Changes
Step 4 of the TanStack Query rollout, and the first of the polling
hooks. Follows #7264, #7283, #7285.
## The problem
`useSigningSessions` hand-rolled its own fetch, loading state and
`setInterval`. Two consequences:
- **A raw `setInterval` keeps polling a hidden tab.** Browsers throttle
background timers, they do not stop them, so a backgrounded editor with
Shared Sign open keeps hitting both endpoints for as long as it is open.
- **No tests.** The hook had none, and its quietest behaviour (below) is
the easiest thing to break without noticing.
## End state
One query behind `qk.signingSessions()`, with the polling lifecycle
handed to the library:
- Polling stops while the tab is hidden, and refetches on return rather
than leaving data up to a full interval stale.
- Mounts render from cache while they revalidate, so moving between the
tool picker and the signing tool no longer flashes an empty list.
- 12 tests where there were none.
Same return shape, so no consumer files change.
### What this is not
This is not a deduplication win. The three consumers are never mounted
at the same time: `ToolPanel` renders the tool picker or the active tool
and never both, so the badge cannot be on screen with either of the
others, and `SharedSigningLauncher` and `useSigningSessionController`
sit inside two different tools. The shared key earns its keep on cache
reuse across those transitions, not on concurrent fetches.
## The bit worth reviewing
The hand-rolled `{ silent: true }` flag encoded three states, and no
single Query flag reproduces them:
| | Spinner | Toast on failure |
|---|---|---|
| First load | yes | yes |
| Background poll | no | no |
| Explicit refetch | **yes** | **yes** |
`isLoading` is false during an explicit refetch when data is already on
screen; `isFetching` is true during a background poll. Neither matches,
so the user-initiated case is tracked with a small flag and the failure
toast is gated on `isLoadingError` plus the explicit path.
## Testing
Twelve tests. Rather than trust them, each claim was checked by breaking
the implementation and confirming the relevant test fails:
| Mutation | Caught by |
|---|---|
| `refetchIntervalInBackground: true` | hidden-tab test |
| Drop `refetchOnWindowFocus` | returns-to-view test |
| Drop the user-initiated spinner flag | manual-refresh test |
| Toast on every error | background-failure-is-silent test |
| Give each observer its own key | dedupe test |
Three things worth knowing for the next conversion:
- **`waitFor` flushes renders.** Recording an index *after*
`waitFor(callCount === 2)` skips past the in-flight render, so a "did
the spinner flip on" assertion passes vacuously. The marker has to go
before the poll.
- **Fake timers hide in-flight state.** The fetch settles inside the
same `act()`, so the intermediate render never happens. That test uses
real timers and a held-open promise.
- **`visibilitychange` has to bubble.** query-core listens for it on
`window`, and the real event bubbles from `document`. A test helper
dispatching a non-bubbling event never reaches the focus manager, and
the pause behaviour still appears to work because `refetchInterval`
reads `document.visibilityState` directly at tick time rather than
through the event.
**One claim is deliberately unguarded.** `isLoading` vs `isFetching` for
a background poll produces no re-render at all, so there is nothing
observable for a test to assert and no user-visible difference to
protect.
## Pre-existing failures
`task frontend:check` passes typecheck, lint and oxfmt, and 2363 of 2365
editor tests. The two failures, `workbenchSession.test.ts` and
`notificationActions.test.tsx`, fail identically with this branch's
changes reverted and are untouched by it.
## Scope
This is one of five pollers. The remaining four, `useLocalFolderPoller`,
`WatchedFolderWorkbenchView`, `SessionDetailPanel` and cloud
`TeamSection`, are separate files with their own consumers and follow
separately, now that the silent-refresh pattern has a worked example.
---------
Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com>
# Description of Changes
Step 5 of the TanStack Query rollout, covering the admin People, Teams
and Team details screens. Follows #7264, #7283, #7285.
## The problem
Two separate ones, in the same three files.
**Reads.** Each section fetched and held its own copy of the same
resources: People read the roster and the team list, Teams read the team
list plus the roster again when its add-member modal opened, Team
details read all three. Cost scaled with how many screens you visited
rather than with how much data exists.
**Writes.** Thirteen handlers each did the same five things by hand: set
a processing flag, call the service, toast the outcome, dig a message
out of an axios error, and reload their own slice. Refreshing was a
convention, not a mechanism, and one handler had already forgotten it.
## The fix
Three shared query keys (`adminUsers`, `teams`, `teamDetails`), and one
`useAdminMutation` helper that every write is declared against:
```ts
const createTeam = useAdminMutation({
write: (name: string) => teamService.createTeam(name),
invalidates: ["teams"],
success: t("workspace.teams.createTeam.success"),
errorFallback: t("workspace.teams.createTeam.error"),
onDone: () => { setNewTeamName(""); setCreateModalOpened(false); },
});
```
Each write names the slices it disturbs, which is the part that only
works when reads and writes are designed together: `createTeam`
invalidates the team list, while a membership move invalidates the list,
both teams' detail rows and the roster, because it genuinely changes all
three. Invalidation refetches only mounted queries, so this costs
nothing extra.
The blanket "invalidate everything" helper survives in exactly one role:
child components (invite, password change, seat update) that write
through their own services, where the affected scopes are not visible
from the call site.
## Why it is better, measured
Request counts come from one harness driving `teams -> team details ->
back -> people`, run against the branch point and against this branch.
The assertion is committed, so it cannot silently regress.
| | Before | After |
|---|---|---|
| Requests | 7 | **3** |
| `getTeams` | 4 | **1** |
| `getUsers` | 2 | **1** |
| `getTeamDetails` | 1 | 1 |
| Committed renders | 17 | **15** |
Three is one per distinct resource, the floor for that sequence. The
four `getTeams` were the Teams table, Team details fetching the same
list for its "move to team" dropdown, the explicit refresh on the back
button, and People.
Renders barely move, which is expected: this changes where data lives,
not how often React draws. It is reported because a caching change can
quietly cost renders, and this one does not.
On the code itself, across the three sections:
| | |
|---|---|
| Net lines | **-216** |
| `useState`/`useEffect` removed | **11**, none added |
| Duplicated `isAxiosError` blocks | 13 to **1** |
| `setProcessing` calls | 19 to **0** |
`isAxiosError` is no longer imported by any of the three files.
## Bug fixed
`disableMfaByAdmin` showed a success toast and never refreshed. The menu
item renders only when `user.mfaEnabled` is true, so an admin disabled
MFA, was told it worked, and watched the option stay on screen until a
manual reload. It is covered by a test that fails if the invalidation is
removed.
## Behaviour worth checking in review
- A write no longer blocks its handler before closing the modal. The
dialog closes when the write succeeds and the table updates when the
refetch lands, rather than the button spinning through both.
- Modal submit buttons now track their own mutation rather than one
shared flag. Team details still derives a single busy flag, now from its
five mutations rather than a `useState`, so its row actions disable
together as before.
- The per-handler `console.error` is kept, once, in the shared error
path.
## Testing
Five tests, each verified by breaking the implementation and confirming
that one test, and only that one, fails:
| Mutation | Caught by |
|---|---|
| Drop the shared stale window (`staleTime: 0`) | request-count test |
| Make invalidation a no-op | write-visibility test |
| Ignore the login-enabled gate | login-disabled test |
| Stop invalidating after the MFA write | MFA-refresh test |
| Fall back to the generic error message | server-message test |
The write tests drive the real flows through their modals and menus
rather than calling hooks directly.
`task frontend:check` passes typecheck, lint and oxfmt, and 2383 of 2385
editor tests. The two failures, `workbenchSession.test.ts` and
`notificationActions.test.tsx`, are untouched here and fail identically
with this branch's changes reverted.
## Scope
The three services keep their current shape; nothing outside these three
sections and the new hook module changes. Child modals that write
through their own services still refresh via the blanket helper, and
converting those is separate work.
## What
Every dialog in the processor is the shared `.sui-modal` shell, and its
backdrop was top-aligning the panel:
```css
align-items: flex-start;
padding: 5rem 1.5rem 1.5rem; /* 80px above, 24px below */
```
On a 900px-tall viewport that started every dialog at `y=80` with ~350px
of dead space beneath it. Phones already had an `align-items: center`
override; desktop never got one.
## Change
`frontend/editor/src/core/ui/Modal.css` only:
- Symmetric block inset, `align-items: center`.
- The inset is published as `--modal-inset-block`, and `.sui-modal`'s
`max-height` derives from it. That coupling is the point: if the two
drift apart, a tall modal overflows a centre-aligned backdrop and loses
its header off the top of the screen, unreachable.
- The phone breakpoint now only moves the variable. Measured at 375x812
it resolves to exactly the previous values (`16px 12px`, `max-height:
780px`), so mobile behaviour is unchanged.
One shared file, so this covers flow modals, source / user / pipeline /
API-key modals, billing and procurement.
## Before / After
<img width="2104" height="2284" alt="image"
src="https://github.com/user-attachments/assets/bcb50145-f75e-449e-92c6-a0b085cc091c"
/>
## Testing
- `task frontend:check` passes (lint + typecheck + 2356 tests).
- Phone breakpoint measured directly in the browser, values match the
previous behaviour.
## The problem
AI PRs write comments that restate the line below them, mark sections
with box drawing, and narrate the diff. Nothing in the repo said not to,
and nothing checked. `AGENTS.md` had one line about comments and it was
buried in the Python section.
Banners and `Step N:` narration have zero occurrences in the 15 months
before Aug 2025, so this is new.
## The fix
A written standard, plus a linter that enforces the mechanical part of
it on added lines only.
-
[devGuide/CODE_COMMENTS.md](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/devGuide/CODE_COMMENTS.md)
holds the reasoning and worked examples; a section in
[AGENTS.md](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/AGENTS.md)
holds the operative rules, kept short so they stay in an agent's
context. The two are split by kind rather than duplicated, because the
same prose in two places drifts.
- Rules in
[comment-rules.mjs](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs),
shared by both engines.
- Two engines. `.ts` / `.tsx` / `.mjs` go to an [oxlint JS
plugin](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint-oxlint-plugin.mjs)
so comments come from the parser rather than a line scan; `.java` /
`.py` go to a [line
scanner](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint.mjs).
Neither reads the other's files, so they cannot disagree about one file.
- Between them they read every comment form the repo writes: `//` and
`/* */`, Javadoc and JSDoc, JSX comments, `#`, and Python docstrings.
- Runs in `task pre-commit`, so the git hook and the `pre_commit.yml` CI
job both get it, and as a Claude Code [`Stop`
hook](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint-hook.mjs)
so an agent fixes the comment inside the turn that wrote it.
## The rules
The part worth arguing about. **Every rule blocks.** A rule that only
warns is a rule nobody acts on, so a finding you believe is wrong is a
bug in the rule: narrow it, or mark the line and say why.
| | Fires on |
| --- | --- |
|
[CMT001](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L71)
| Every word in the comment already appears in the code below it. Max 6
words, skipped for prose punctuation and for a bare Arrange/Act/Assert
marker |
|
[CMT002](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L92)
| 4+ rule or box-drawing characters, or a bare section label from [a
fixed
list](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L84)
(`Types`, `Helpers`, `State`, `Handlers`, ...) |
|
[CMT003](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L110)
| `Step N:` with a separator, or `Then,` / `Next,` / `Finally,`.
Suppressed in test files |
|
[CMT004](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L129)
| A comment about the code's own past: `this used to`, `renamed from`,
`was previously called`. Suppressed in test files |
|
[CMT005](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L154)
| 3+ consecutive comment lines where 2/3 [parse as
code](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L143)
|
|
[CMT006](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L31)
| A run of implementation comment over 12 lines, outside the first 5
lines of a file. Doc blocks are exempt, because the standard asks for
thorough contracts |
|
[CMT007](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L180)
| A parameter or return description that adds no word its name lacks.
Reads Javadoc/JSDoc `@param`, Sphinx `:param name:` and Google `name:
description` |
|
[CMT008](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L239)
| An allow directive naming a rule that does not exist, or one that
silenced nothing |
|
[CMT009](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L219)
| A `TODO` / `FIXME` / `HACK` naming no issue or link. An owner is not
accepted: a username goes stale, an issue outlives it |
Each rule carries the readings it deliberately excludes, next to the
rule. Those exclusions came from running the rules over this repo, not
from taste: `CMT004` does not match a bare "no longer needed" because
that is as often about runtime lifecycle as about history, and `CMT003`
needs a separator after the number so a wrapped line beginning "step 2
unmounts + remounts the panel" reads as the prose it is.
A comment sharing a line with code is judged by the rules that do not
depend on the code below it, so a trailing `// TODO fix this` or `/*
this used to run before the flush */` still reports, while `50L * 1024 *
1024 // 50 MB` does not. `CMT001` would have been wrong about six in
seven trailing comments here, so it stays out of them.
If a finding is wrong, `// comment-lint-allow: CMT002` on the line
above. Rule-specific, [no blanket
disable](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L229).
A directive naming a rule that does not exist, or silencing nothing, is
itself a `CMT008` failure, so a typo cannot quietly disable a rule and a
stale one gets deleted rather than accumulating.
No native linter covers `CMT007`. `eslint-plugin-jsdoc`'s
`require-param-description`, Checkstyle's `NonEmptyAtclauseDescription`
and ruff's D-rules all check that a description exists, not whether it
says anything.
## Scoping
Added comment **text** only, not lines git calls new. Reindenting a file
or moving a block makes git mark untouched comments as added; findings
are matched against the comment text at the base, so only genuinely new
content reports.
The whole file is read and every comment in it evaluated. Only the
*reporting* is filtered, so a rule still sees the code a comment
introduces, the full run it belongs to, and the base version of the
file.
Existing tree is untouched. `task pre-commit:comment-lint:all` reports
it and always exits 0:
| | java | ts/js | py |
| --- | --- | --- | --- |
| findings | 1,218 | 741 | 204 |
2,163 across 542 files, mostly `CMT002` banners (1,482) and `CMT001`
restatements (456). Clearing it is separate work, by directory.
Not in this PR: an advisory LLM review layer for the things no pattern
can judge.
## Verification
Run against
[#7494](https://github.com/Stirling-Tools/Stirling-PDF/pull/7494) as CI
would, in a throwaway worktree: **two findings on a 78 file, +4,512 line
change, both genuine banners, in 952ms**. A whole-file scan of those
same files gives 11; the other 9 were withheld because that PR's author
did not write them, and they are the `@param teamId the team ID` shape
this standard exists to stop.
Both scanners blank string and character literals before looking for
comment markers, because a partial lex desynchronises everything after
it: one apostrophe in a Java comment, or one Python template whose
closing quotes start a line, is enough to read dozens of lines of code
as a single comment. Two fixtures carry canaries that stop being
reported if either engine ever desynchronises again.
The [fixture
corpus](https://github.com/Stirling-Tools/Stirling-PDF/tree/claude/ai-pr-comment-quality-dd970e/scripts/lint/fixtures)
pins all 9 rules against both engines, and `--selftest` fails if the two
disagree about the same file.
## Two things reviewers should know
**The oxlint JS plugin API is alpha.** oxlint itself is stable and
already this repo's frontend linter; the plugin API is the new
dependency. Its documented failure mode
([oxc#25203](https://github.com/oxc-project/oxc/issues/25203)) is being
skipped silently while oxlint still reports success. That affects the
standalone release binary rather than the npm package this invokes, but
the class of failure reads exactly like clean code, so the run asserts
`number_of_rules >= 1` from oxlint's own report and a broken engine
exits 2 rather than passing. If the API ever breaks, the fallback is
folding these rules into the line scanner, which already implements all
nine for Java and Python.
**`.claude/settings.json` is now committed**, carrying the hook and
nothing else: 19 lines, no `permissions`, nothing machine-specific. That
partly reverts `c35546a212` ("Ignore claude dir"), which existed because
this file had twice been committed by accident with a personal
`permissions` allowlist, once with absolute machine paths. Personal
config still belongs in `.claude/settings.local.json`, which the new
pattern keeps ignored, and hook entries merge across the two so nobody's
own hooks are lost.
If you already hand-wrote a `.claude/settings.json`, copy it somewhere
first: that path used to be git-ignored, and git overwrites an ignored
file without warning when a commit starts tracking it. Across 19 local
checkouts here, 13 have `settings.local.json` and none has a
hand-written `settings.json`.
To turn the hook off, `{ "env": { "COMMENT_LINT_HOOK": "0" } }` in local
settings. Claude Code can only disable all hooks at once, hence the
switch. The commit-time gate still applies.
## How to test
```bash
task pre-commit:comment-lint:ci
```
The fixture corpus, then the diff. The corpus checks the rules
themselves rather than the code under review, so it runs on CI and
before a rule change, not on every local commit.
```bash
task comment-lint:branch
```
`clean (34 files in scope)`. `task comment-lint` is the same thing
scoped to uncommitted work, which is what the git hook and CI run.
To watch it bite, add `// Is banner` above `export function isBanner` in
`scripts/lint/comment-rules.mjs` and run `task comment-lint`: one
`CMT001`, exit 1. The gate covers its own source, which is why these
scripts have no section dividers.
```bash
task pre-commit:comment-lint:all
```
The standing backlog, report-only.
Verified on the pinned oxlint 1.77.0, not only the 1.79 the plugin was
prototyped against.
# Description of Changes
When #6697 merged, the CI didn't run for some reason so it was never
caught that the tool models were out of date. This PR updates them to
the correct state.
# Description of Changes
Fixes various bugs that affected SaaS (and some self-hosted):
- Refreshing on Editor caused the user to be redirected to Processor
- User was unable to access Processor in SaaS
- Deep link hijacking fixes
- Fix double prefix `/app/app` issue
- Fix going from tool -> editor -> processor -> editor putting you back
into tool
---------
Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com>
## Description of Changes
- Removed overlapping Gradle subdirectory entries from
.github/dependabot.yml.
- Dependabot now monitors the root Gradle project through /.
- Prevents duplicate pull requests for dependencies declared in Gradle
subprojects.
Closes: Not applicable
---
## Checklist
### General
- [ ] I have read the Contribution Guidelines
- [ ] I have read the Stirling-PDF Developer Guide (if applicable)
- [x] I have performed a self-review of my own code
- [ ] My changes generate no new warnings
### Documentation
- [ ] I have updated relevant documentation (if applicable)
- [ ] I have read the translation tag documentation (for new translation
tags only)
### UI Changes (if applicable)
- [ ] Screenshots or videos are attached
### Testing (if applicable)
- [ ] I have tested my changes locally
## What
The `promo` banner tone was a full-bleed `indigo-500 → purple-500`
gradient with white text and a black drop-shadow on the CTA. It was the
only saturated fill in the app, and against the warm neutral palette it
read as a foreign object above the workbench.
The bar is now app chrome:
| | Before | After |
|---|---|---|
| Background | 135° indigo→purple gradient | `--c-bg-raised` |
| Border | `transparent` | `--c-border-subtle` hairline |
| Icon | white glyph, no container | neutral glyph in a
`--c-surface-sunken` chip |
| Text | forced white | `--c-text` / `--c-text-muted` |
| CTA | `premium` accent (violet gradient) | `default` accent (same
primary button as the rest of the app) |
Before
<img width="1504" height="739" alt="Screenshot 2026-08-27 at 4 49 00 PM"
src="https://github.com/user-attachments/assets/a912d9e9-9590-4e1d-8202-1abb22a00f23"
/>
After
<img width="1061" height="665" alt="Screenshot 2026-08-27 at 4 48 30 PM"
src="https://github.com/user-attachments/assets/3be14aec-ccfe-4448-93b3-339a57be4937"
/>
Only caller is the friendly variant of `UpgradeBanner` (self-hosted,
under the free-tier user limit).
## Notes
- **No new theme tokens.** Every value is an existing `--c-*` semantic
token, so light and dark both follow automatically with no per-theme
overrides.
- The `premium` accent itself is untouched, so the upgrade CTAs in
`OfflineActivationCard` and `PairingPanel` are unaffected.
- `--c-hue-indigo` / `--c-hue-purple` are still used by
`SaaSOnboardingSlides`, `PaygFree` and `UpgradeModal`, so no tokens are
orphaned.
- Deleted comments describe rules that no longer exist (the gradient,
the white-on-gradient text overrides, the CTA shadow). No new comments
added.
## Verification
- `task frontend:check:all` passes (typecheck, oxlint, all four theme
linters, stylelint, format, tests, build, storybook build).
- `task frontend:storybook:a11y:changed` passes light and dark: 7
AppBanner stories, 0 violations. Both a11y baselines are empty, so this
is zero known violations rather than a baselined pass.
- Checked in Storybook under **Shared / AppBanner → All Top Bars**,
which renders every top bar the app can show side by side, in both
themes.
Our classification labels were rendering their hardcoded English names
because the en-US locale file had no `classification` section at all, so
this adds the missing keys (labels and category names).
Also wires the category names through i18n, since those had no `t()`
call, and adds a test so a new label can't ship without its key.
# Description of Changes
After ui rework all scrolling in all tool panels stopped working
This fixes this to allow tool panels to be scrollabe again
## What was wrong
PDF/UA is the only convert target whose settings panel overflows the
tool rail. Measured at 1920×1080: overflow was 0px for pdfa, pdfx, png,
docx, epub, and 158px for pdfua. Its action button sat at bottom: 1220
in a 1080px viewport — 140px below the fold — and the info alert was
clipped mid-sentence. The panel could be scrolled, but nothing said so
(Mantine's scrollbar auto-hides).
Normally the app would scroll the button into view for you. It didn't,
because both mechanisms built to do that were dead
## Cause:
Two separate mechanisms, both broken since the same commit (0a50e765b7,
frontend editor restructure, 2026-05-22):
1. ReviewToolStep - shared by all 47 tools. It looked for its scroll
container with:
stepRef.current.closest('[style*="overflow: auto"]')
Mantine's ScrollArea viewport sets inline overflow: scroll, not auto. I
measured it live - closest() returns null, and
document.querySelectorAll('[style*="overflow: auto"]') finds exactly 1
element anywhere in the page, and it isn't an ancestor of the panel. So
the lookup silently found nothing and the scrollTo never ran, for every
tool.
2. Convert.tsx - Convert only. It declared scrollContainerRef and a
scrollToBottom() wired to two useEffects, but the ref was never attached
to any element - createToolFlow() builds the JSX and no ref is passed
through. Always null, so both effects were no-ops.
Nothing else in the codebase has this pattern - I grepped for other
closest('[style*="overflow…"]') lookups and other
scrollToBottom/scrollContainerRef uses and both came back empty.
## The fix
createToolFlow.module.css (new) + createToolFlow.tsx:156 — the execute
button gets a position: sticky; bottom: 0 footer, the house pattern
already used by FormFill.module.css. Applied only when the review step
isn't visible, so it can never float over results. Sticky is inert when
content fits, so the other 46 tools are untouched.
ReviewToolStep.tsx:21 — real findScrollParent() walk replacing the
broken selector, scrolling by the minimum delta needed and only the
panel itself (never scrollIntoView(), which drags every ancestor). Also
added the missing clearTimeout cleanup.
Convert.tsx — deleted the dead ref and its two effects.
---
## Checklist
### General
- [ ] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [ ] I have read the [Stirling-PDF Developer
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md)
(if applicable)
- [ ] I have read the [How to add new languages to
Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md)
(if applicable)
- [ ] I have performed a self-review of my own code
- [ ] My changes generate no new warnings
### Documentation
- [ ] I have updated relevant docs on [Stirling-PDF's doc
repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/)
(if functionality has heavily changed)
- [ ] I have read the section [Add New Translation
Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags)
(for new translation tags only)
### Translations (if applicable)
- [ ] I ran
[`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md)
### UI Changes (if applicable)
- [ ] Screenshots or videos demonstrating the UI changes are attached
(e.g., as comments or direct attachments in the PR)
### Testing (if applicable)
- [ ] I have run `task check` to verify linters, typechecks, and tests
pass
- [ ] I have tested my changes locally. Refer to the [Testing
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing)
for more details.
Links a self-hosted instance to a SaaS team over an ordinary redirect,
and leaves the admin's browser holding a Stirling session at the same
time.
## The problem
A self-hosted server needs a device credential bound to a SaaS team, and
the admin's Supabase JWT must never reach the instance backend. Three
things ruled out the obvious approaches:
- **A customer hostname can never be in Supabase's redirect
allow-list**, so the sign-in cannot happen on the instance's own origin.
That is why SSO and sign-up did not work for linking at all.
- **A device credential identifies a server, not a person.** Every
attended portal read (Usage, Billing, Documents, Infrastructure) goes
through `getPortalSaasToken()` and needs a *user* session, so a
credential-only link left all of them asking for a second sign-in.
- **The previous design relayed a JWT** from the browser into the
instance, which is the thing we wanted to avoid. That path is deleted
here.
## The solution
Redirect and nonce, modelled on desktop's
`authService.loginWithSelfHostedOAuth`: mint a nonce, hand the browser
off, accept only a callback carrying that nonce back. Desktop has the OS
route the reply; self-hosted has no OS hop, so our own approval page
performs it. That is the point — the human half happens on an origin we
control.
```
instance SaaS admin's browser
| POST connect/request | |
| (name, callback, nonce, | |
| claim-secret hash) | |
|-------------------------->| |
| <- requestId + authorizeUrl |
| | GET /link?request=... |
| |<-------------------------------|
| | sign in (SSO works here), |
| | see ACCOUNT + ORIGIN, approve |
| |------------------------------->|
| | 302 callback#nonce+session |
| POST connect/claim | |
| (requestId, claim secret)| |
|-------------------------->| |
| <- device credential | |
```
Four properties carry the safety, and each is stated in the code because
each is easy to lose in a refactor:
- **The redirect target is never caller-supplied.** Validated once at
creation, then read back from the stored row, so nothing in the approval
page's URL can steer the token elsewhere.
- **Approval and minting are separate.** Approval records the team and
hands out nothing usable; the credential is minted only on claim,
authenticated by a secret that never entered a browser.
- **A re-authentication cannot move a server between teams.** The team
is pinned at creation from the credential only that instance holds, so
an approver from another team gets `WRONG_TEAM` instead of a rebind.
- **The approver has to confirm what they are binding.** The page shows
the address and the signed-in account, with a way to switch, and a
checkbox naming the address gates the approve button. The name the
server reports is deliberately not shown: the requester picks it on an
unauthenticated endpoint, and its honest value is the hostname already
in the address.
The session rides the URL fragment, so it stays out of access logs and
`Referer`, and is stripped before anything awaits. The claim is
row-locked, so one approval mints once. A request lives 30 minutes; a
settled one is not offered again, since approving it fails server-side.
Signing in mid-flow no longer loses the request. The id is kept on the
SaaS origin and resumed after any sign-in, which is what makes creating
an account work: the confirmation email opens a new tab, where the
`next` parameter is gone. Reading it does not consume it — the request
may be open in two tabs — and only a recorded decision retires it.
The result lands as a modal over the portal the admin started from, and
the portal re-reads its link status so the page behind agrees with the
modal.
Plaintext `http://` callbacks are accepted rather than refused, because
many self-hosted instances legitimately run plain HTTP on a private
network; the address carries a warning icon explaining the risk, derived
server-side so a requester cannot suppress it. Hard-refusing `http://`
to a public IP literal is a reasonable follow-up; a bare hostname can't
be classified without a DNS lookup, so the warning stays the general
mechanism.
## Configuration
Four surfaces. Placeholders below, not values.
**SaaS backend**
| Setting | Needed | Why |
|---|---|---|
| `stirling.billing.account-link.enabled` | Yes, `true` | The connect
controller and service are `@ConditionalOnProperty` with no default, so
without it the endpoints do not exist. |
| `system.frontendUrl` | Only when the approval page is not on the API's
own origin | Where the approver is sent. Must include the app's base
path if it is served under one, or the redirect misses `/link`. |
**SaaS frontend**
| Setting | Needed | Why |
|---|---|---|
| `VITE_SUPABASE_URL`, `VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY` | Yes |
Its own sign-in. Must be the project the SaaS backend validates tokens
against. |
| `RUN_SUBPATH` | Only if served under a subpath | Moves the approval
page to `<base>/<subpath>/link`, so `system.frontendUrl` has to agree. |
**Self-hosted backend**
| Setting | Needed | Why |
|---|---|---|
| `stirling.billing.account-link.enabled` | Yes, `true` | Defaults to
`false`. |
| `stirling.billing.account-link.saas-base-url` | Yes | Origin of the
SaaS API it links to. Not the SaaS frontend. |
| `system.frontendUrl` | Optional | Externally reachable base URL for
the callback. Otherwise derived from the request's `Origin`, which is
right for ordinary deployments and wrong behind a rewriting proxy. |
**Self-hosted frontend**
| Setting | Needed | Why |
|---|---|---|
| `VITE_SUPABASE_URL`, `VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY` | Yes |
Accepts the session handed over in the callback fragment. |
| `VITE_SAAS_API_URL` | For Usage and Billing | Attended reads go to the
SaaS API with the admin's token. Absent, those surfaces stay on the
mock. |
| `VITE_INCLUDE_PORTAL` | Production builds | Dev builds include the
portal automatically; without it there is no link UI and no callback
route. |
Two things worth stating because neither fails loudly:
- **Both frontends must use the URL *and* key of the same Supabase
project**, and the same one the SaaS backend validates against. A key
from one project with a URL from another is accepted by the browser and
rejected by Supabase, which surfaces much later as "session expired" on
Usage rather than as an error at hand-over.
- **The Supabase redirect allow-list must contain the SaaS app's
`/auth/callback`**, since a confirmation email returns through it.
Entries are matched exactly.
- **`system.frontendUrl` is the existing setting for this**, not a new
one, so each side reads its own value and there is nothing extra to
configure. It also gates share links, so on a stack with storage and
sharing already on, setting it here turns those on too.
The self-hosted side deliberately does **not** configure where the
approval page lives — SaaS answers that in the connect-request reply,
being the only party that knows.
Also here, because testing this needs two stacks side by side:
`linked:staging` / `linked:dev` (which derive `system.frontendUrl` and
`RUN_SUBPATH` themselves), the missing `frontend:staging:saas`, and a
per-mode vite `cacheDir` — two dev servers in different modes otherwise
re-optimise over one shared dep cache.
## How to test
Automated and green: `task frontend:check:all` plus both backend
modules. `ConnectRequestServiceTest` covers callback validation, the
per-IP cap, single-use approval, claim outcomes, expiry, `WRONG_TEAM`
and reauth confirming without minting; `ConnectServiceTest` covers
callback-resolution precedence including a foreign-origin callback being
discarded; `ConnectControllerTest` covers the authorize URL, including
the forwarded-header path and only the first hop being trusted;
`ConnectCallback.test.tsx` covers the fragment being stripped
synchronously and malformed fragments refused;
`LinkAccountModal.test.tsx` covers link and reauth hitting different
endpoints.
Manual walkthrough:
1. `task linked:staging` — added here; brings up a SaaS stack and a
self-hosted instance pointed at it, on discovered ports, and prints the
four addresses.
2. Open the link-account modal in the self-hosted portal and continue.
Expect the SaaS approval page at `/link?request=<id>`.
3. Sign in as a team leader, or create an account and confirm the email.
Either way you should come back to the approval page.
4. Tick the acknowledgement and approve. Expect the fragment gone from
the address bar immediately, a result modal over the portal, the portal
showing linked without a reload, and attended reads (Usage, Billing)
working without a second sign-in.
5. Repeat, approving as a member of a different team. Expect a refusal,
not a rebind.
## Outstanding
- #7415 to be reworked against this design once this lands.
- **No SaaS-side UI to disconnect a server.** `GET
/account-link/instances` and `POST /account-link/instances/{id}/revoke`
are already team-scoped and leader-gated, and the portal has a panel
that uses them, but
`portal-saas/components/settings/accountLinkSettings.tsx` exports `null`
on the reasoning that "SaaS has no account-link concept". That held when
linking was a self-hosted admin managing their own instance; here a
leader approves a server they may not administer, and has no way to
withdraw it. The seam to fill is that one file. Expected to land with
the CTA work in #7415.
---------
Co-authored-by: James Brunton <jbrunton96@gmail.com>
## What
Switching editor -> processor (or reloading) unmounts every editor
provider, which emptied the workbench. This PR mirrors the workbench
into per-tab sessionStorage and refills an empty one from that record on
the next mount:
- **Files, selection, view and active document survive** the shell
switch and reloads. Each recorded file is resolved to its *current leaf*
version on restore, so a file versioned by a policy or another tab comes
back at its latest state.
- **The switch back lands where the user left**: the processor sidebar's
"editor" button consumes a one-shot return path saved at switch time.
- **The app switch respects unsaved changes**: `useOtherAppSwitch`
(proprietary + saas) now routes through `requestNavigation`, so the same
warning guards it as any other navigation.
- Desktop shadows `WorkbenchSessionPersistence` with a stub (OS-launched
files own boot there).
## How to test
I've run through each of these manually:
- Upload several PDFs in the editor, select a couple, and switch to the
Active Files grid. Click "Open PDF Processor" in the sidebar footer,
then switch back to the editor. The same files, selection and view
should return, and you should land on the editor page you left.
- Open a document in the viewer, then reload the tab. The workbench
should refill and come back on the viewer with the same document active.
- With unsaved changes in a tool, click the processor switch. The
unsaved-changes warning should appear, and the switch should only
proceed if you confirm.
- Open a second browser tab with different files. Each tab should
restore its own workbench independently (the record is per-tab
sessionStorage).
- While in the processor, delete one of the open files from storage,
then switch back. The remaining files should restore and a warning toast
should report "Restored X of Y files".
---------
Co-authored-by: Claude <noreply@anthropic.com>
Follow-up to #7580: the escalation it added could never fire.
## What's broken
The auto-run skips a policy that has already run on a file, keyed on
`(categoryId, fileId)`. `recordRunStart` claims that key — and #7580 has
the **browser-side first pass** record its own run under `categoryId:
"classification"` for the uploaded file. So the local heuristic ticks
the very key the server escalation checks, and the AI is never asked, at
any confidence.
Trigger is the default seeded setup: **Classification as the only
on-upload policy**, and a local verdict below `high`. Any other
on-upload policy masks it, because classification then targets that
policy's output — a new file id whose key was never claimed. That's why
this went unnoticed.
Two smaller faults in the same path:
- A chained output carried no `classificationConfidence`, so
`shouldDispatchToAi` waited for a verdict that could never arrive (a
tool-derived file gets no local pass).
- Browser-local runs were polled against the server: 3 × 404 per file,
after which `MAX_NOT_FOUND` marked a local run that had actually
**succeeded** as `FAILED`.
## The fix
- `PolicyRunRecord.browserLocal`; `recordRunStart` skips the dispatch
claim for such a run. It is the first pass, not the policy's run.
- The local pass meters under `classification:local-meter` instead of
the category id, so metering dedupe survives without suppressing
dispatch.
- The poll effect skips browser-local runs.
- `CONSUME_FILES` inherits `classificationConfidence` alongside the
labels, so the verdict survives a version bump.
## How to test
Download
[`low-confidence-classification.pdf`](https://github.com/Stirling-Tools/Stirling-PDF/raw/fix/chained-classification-confidence/frontend/editor/src/proprietary/services/heuristic/fixtures/low-confidence-classification.pdf)
(checked in as a fixture, verdict pinned by a test).
With **Classification as the only on-upload policy**, upload it and
watch the Network tab:
- **Before:** no `POST /api/v1/policies/{id}/run` for classification,
ever. Console shows `local-classification-*` 404s.
- **After:** exactly one, and the engine receives `POST
/api/v1/documents/classify`.
Judge it on that request, not on the resulting label — the model's
answer varies, so a label comparison can pass or fail for the wrong
reason.
Headless equivalent:
```
npx vitest run --project proprietary src/proprietary/components/policies/usePolicyAutoRun.escalation.test.tsx
```
Passes here, fails on `main` on "asks the AI about an unsure verdict
even though the local pass already ran". Its other two cases pass on
both, so the guards still hold: a confident verdict still costs nothing,
and a file with no verdict yet still waits rather than racing the free
pass.
New tests drive the **real** run store — mocking it is what let this
through.
`task frontend:check`: 255 files / 2202 tests.
# Description of Changes
Originally, I wanted to re-enable typed linting on our repo but using
Oxlint this time to avoid the memory and speed issues that ESLint was
causing. Unfortunately, it's not stable enough yet to actually use on
our repo (although it is close, I suspect it'll be stable enough fairly
soon). I was able to remove many of the unnecessary casts that it found
though, so even though this won't be enforced, it's still worth cleaning
up what I've found.
The bundled signing test certificates expired at **07:41:10 UTC on
2026-08-26**. They were issued exactly one year earlier, so they went
from fine to fatal mid-morning with no warning, and they take down
`main` and every open branch, not just one PR.
First casualty was the `docker-compose-tests` job on #6802, which
started at 07:45:
```
java.security.cert.CertificateExpiredException: NotAfter: Wed Aug 26 07:41:10 UTC 2026
at CreateSignatureBase.checkValidity(CreateSignatureBase.java:159)
at CertSignControllerTest.testSignPdfWithPkcs12(CertSignControllerTest.java:205)
```
```
$ openssl x509 -in app/core/src/test/resources/certs/test-cert.pem -noout -dates
notBefore=Aug 26 07:41:10 2025 GMT
notAfter =Aug 26 07:41:10 2026 GMT
```
## What was broken
`CertSignControllerTest` (7 tests) and `PdfSigningServiceImplTest` (2)
fail outright. `ValidateSignatureControllerMoreTest` and
`CertificateValidationServiceMoreTest` read the same fixtures.
Auditing the rest of the repo turned up three more time bombs that had
not gone off yet:
| Fixture | Was | Problem |
|---|---|---|
| `app/core/.../certs/test-cert.*` + `test-key.*` | expired 2026-08-26 |
**already breaking every branch** |
| `test-certs/valid-test.p12`, `valid-test.jks` (proprietary + frontend
copies) | expire 2027-03-25 | same failure, seven months out |
| `test-certs/not-yet-valid-test.p12` | valid **from** 2027-03-25 |
becomes valid, so its test silently stops proving anything, on the same
day |
## What this does
**Regenerates every fixture** with the identical subject DN, alias,
password, key size and signature algorithm as before, changing only the
validity window. Nothing that any test asserts on has moved.
- valid fixtures: `2025-01-01` to `2125-01-01`
- `not-yet-valid-test.p12`: `2125-01-01` to `2126-01-01`, so it stays in
the future
- `expired-test.p12`: pinned to its permanently-past 2024 window
**Adds `scripts/generate-test-certs.sh`** as the source of truth, so the
next regeneration is one command instead of archaeology. It documents
every DN, alias and password, pins the validity windows, and runs on
Linux, macOS and Git Bash.
**Adds two guard tests** that fail with an actionable message, naming
the script, while there is still a year of runway:
- `BundledTestCertificateExpiryTest` (app/core) checks all seven formats
parse, are in their validity window, and have more than 365 days left
- `BundledWorkflowCertificateExpiryTest` (proprietary) does the same for
the valid pair, and additionally asserts the expired fixture is still
expired and the not-yet-valid one is still in the future
That last pair matters: those two fixtures exist to test a validity
outcome, and each one silently stops testing anything once the clock
passes its window.
## Verification
Run locally against the regenerated bytes, on the exact content
committed here:
```
./gradlew :stirling-pdf:test --tests '*CertSignControllerTest*' --tests '*BundledTestCertificateExpiryTest*' \
--tests '*PdfSigningServiceImplTest*' --tests '*ValidateSignatureControllerMoreTest*' \
--tests '*CertificateValidationServiceMoreTest*'
BUILD SUCCESSFUL
./gradlew :proprietary:test --tests '*BundledWorkflowCertificateExpiryTest*' --tests '*CertificateValidationIntegrationTest*' \
--tests '*SigningFinalizationServiceMoreTest*' --tests '*ServerCertificateServiceTest*' \
--tests '*CertificateSubmissionValidatorTest*' --tests '*WorkflowSessionServiceTest*'
BUILD SUCCESSFUL
```
`spotlessCheck` passes on both modules.
# Description of Changes
e2e Playwright tests are currently failing intermittently on all
platforms for different reasons, most notably WebKit, which seems to
fail much more often than the others. This PR attempts to fix the
issues. I've ran the e2e tests a few times now and they don't seem to be
inconsistent any more, but it's difficult to tell if all the issues are
genuinely fixed due to the inconsistent nature. As far as I can tell,
I've not broken anything though.
Updated the Java formatter to Google Java Format 1.35.0 and enabled long-string reflow/skip-javadoc formatting in Spotless, then reformatted a large set of Java sources across app/common, app/core, and app/proprietary for consistent wrapping and readability. Also hardened the folder identity test to skip symbolic-link checks when the runner does not support them, and refreshed the VS Code Java settings to match the newer formatter.
2026-08-23 01:21:26 +02:00
739 changed files with 23394 additions and 5431 deletions
- [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable)
- [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable)
- [ ] I have performed a self-review of my own code
- [ ] Every comment I added says something the code does not ([guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/CODE_COMMENTS.md))
@@ -21,6 +21,43 @@ Task `desc:` fields should describe **what** the task does, not **how** it does
-`task docker:build` — build standard Docker image
-`task docker:up` — start Docker compose stack
## Comments
A comment must carry information the code cannot. If a reader could derive it from the code in front of them, delete it.
Comment the current state. Not what the code used to do, not what changed, not why it changed: git holds that. Where history explains the shape, state the reason instead, so "this used to reimplement the modal internals" becomes "thin wrapper over the shared Modal: duplicating its portal and focus trap is how dialogs drift apart". Future state goes in a TODO with an issue.
Write a comment when it does one of these four jobs:
- **Contract.** What a caller must know that the signature cannot say: preconditions, invariants, units, ownership and lifetime, thread-safety, error semantics, side effects. Document the contract of everything a caller outside the file can reach, and nothing else. Goes on the type/method/module as Javadoc, JSDoc, or a docstring.
- **Why.** The constraint the code satisfies, the bug it avoids, the alternative rejected and the reason.
- **Hazard.** "Must stay in sync with X", "order matters because Y", "do not remove, it prevents Z".
- **Map.** A short orientation at the top of a genuinely complex file: what it owns, and what it deliberately does not.
Never write:
- A comment that restates the next line. `// Handle drag start` above `handleDragStart` is noise.
- Section banners or position markers: `// --- Types ---`, `// Helpers`, `// =====`.
- Step narration in a function body (`// Step 1:`, `// Then we`). If the steps need labels they need names: extract functions. Numbering a genuinely numbered thing, like a wizard step, is fine.
- Commented-out code. Delete it.
- Doc tags that restate the signature. `@param blob - The blob` says nothing; omit the tag rather than pad it.
- Docs on self-explanatory members with no constraint to state.
Two tests before keeping a comment:
- **Delete it.** Is any information lost? If not, it stays deleted.
- **Could a name carry it instead?** A better identifier, an extracted function, or a named constant beats a comment. Prefer the code change.
A comment at the end of a line usually decodes that line, and that is worth keeping: `{0x25, 0x50} // "%PDF"`, `50L * 1024 * 1024 // 50 MB`. The rules that compare a comment against the code below it do not apply there, but a trailing TODO or a trailing bit of history is judged like any other.
A reference is supplementary, never load-bearing: the comment must survive deleting it. `// See #1234` is a dead end; `// saving first loses every annotation (#6865)` is not. Prefer a spec (`RFC 3161`) or CVE where one applies.
A TODO needs an issue, not an owner: `// TODO(#1234): re-enable the gate once account syncing lands`. If it is not worth an issue, it is not worth a TODO. A question is not a TODO.
A comment block over ~12 lines outside a file or type header usually means the code needs restructuring, or that the prose is product documentation and belongs in the docs repo.
`task comment-lint` checks the mechanical part of this on the lines you add, and runs inside `task pre-commit`. Reasoning, worked examples and the linter's own rules: @devGuide/CODE_COMMENTS.md
## Common Development Commands
### Build and Test
@@ -70,7 +107,7 @@ The project structure is defined in `engine/pyproject.toml`. Any new dependencie
- Avoid nested functions and nested classes unless the language construct requires them.
- Prefer composition to inheritance when combining concepts.
- Avoid speculative abstractions. Add a layer only when it removes real duplication or clarifies lifecycle.
-Add comments sparingly and only when they explain non-obvious intent.
-Comments follow the repo-wide rules in the "Comments" section above.
#### Python Typing and Models
- Deserialize into Pydantic models as early as possible.
@@ -42,6 +42,7 @@ Please make sure your Pull Request adheres to the following guidelines:
- Keep commits atomic. One commit should contain one change. If you want to make multiple changes, submit multiple Pull Requests.
- Commits should be clear, concise, and easy to understand.
- References to the Issue number in the Pull Request and/or Commit message.
- Every comment in the diff should say something the code does not. See [Code comments](devGuide/CODE_COMMENTS.md); `task comment-lint` checks the mechanical part.
// -PjpdfiumPlatforms=auto|all|none|<csv of linux-x64,linux-arm64,linux-musl-x64,linux-musl-arm64,darwin-x64,darwin-arm64,windows-x64> (windows-arm64 natives not published yet)
@@ -1119,19 +1121,25 @@ public class ExceptionUtils {
PDF_CORRUPTED(
"E001",
"error.pdfCorrupted",
"PDF file appears to be corrupted or damaged. Please try using the 'Repair PDF' feature first to fix the file before proceeding with this operation."),
"PDF file appears to be corrupted or damaged. Please try using the 'Repair PDF'"
+" feature first to fix the file before proceeding with this operation."),
PDF_MULTIPLE_CORRUPTED(
"E002",
"error.multiplePdfCorrupted",
"One or more PDF files appear to be corrupted or damaged. Please try using the 'Repair PDF' feature on each file first before attempting to merge them."),
"One or more PDF files appear to be corrupted or damaged. Please try using the"
+" 'Repair PDF' feature on each file first before attempting to merge them."),
PDF_ENCRYPTION(
"E003",
"error.pdfEncryption",
"The PDF appears to have corrupted encryption data. This can happen when the PDF was created with incompatible encryption methods. Please try using the 'Repair PDF' feature first, or contact the document creator for a new copy."),
"The PDF appears to have corrupted encryption data. This can happen when the PDF"
+" was created with incompatible encryption methods. Please try using the"
+" 'Repair PDF' feature first, or contact the document creator for a new"
+" copy."),
PDF_PASSWORD(
"E004",
"error.pdfPassword",
"The PDF Document is passworded and either the password was not provided or was incorrect"),
"The PDF Document is passworded and either the password was not provided or was"
+" incorrect"),
PDF_NO_PAGES("E005","error.pdfNoPages","PDF file contains no pages"),
PDF_NOT_PDF("E006","error.notPdfFile","File must be in PDF format"),
@@ -1139,20 +1147,25 @@ public class ExceptionUtils {
CBR_INVALID_FORMAT(
"E010",
"error.cbrInvalidFormat",
"Invalid or corrupted CBR/RAR archive. The file may be corrupted, use an unsupported RAR format (RAR5+), encrypted, or may not be a valid RAR archive."),
"Invalid or corrupted CBR/RAR archive. The file may be corrupted, use an"
+" unsupported RAR format (RAR5+), encrypted, or may not be a valid RAR"
+" archive."),
CBR_NO_IMAGES(
"E012",
"error.cbrNoImages",
"No valid images found in the CBR file. The archive may be empty, or all images may be corrupted or in unsupported formats."),
"No valid images found in the CBR file. The archive may be empty, or all images may"
+" be corrupted or in unsupported formats."),
CBR_NOT_CBR("E014","error.notCbrFile","File must be a CBR or RAR archive"),
CBZ_INVALID_FORMAT(
"E015",
"error.cbzInvalidFormat",
"Invalid or corrupted CBZ/ZIP archive. The file may be empty, corrupted, or may not be a valid ZIP archive."),
"Invalid or corrupted CBZ/ZIP archive. The file may be empty, corrupted, or may not"
+" be a valid ZIP archive."),
CBZ_NO_IMAGES(
"E016",
"error.cbzNoImages",
"No valid images found in the CBZ file. The archive may be empty, or all images may be corrupted or in unsupported formats."),
"No valid images found in the CBZ file. The archive may be empty, or all images may"
+" be corrupted or in unsupported formats."),
CBZ_NOT_CBZ("E018","error.notCbzFile","File must be a CBZ or ZIP archive"),
// EML errors
@@ -1205,7 +1218,8 @@ public class ExceptionUtils {
FFMPEG_REQUIRED(
"E063",
"error.ffmpegRequired",
"FFmpeg must be installed to convert PDFs to video. Install FFmpeg and ensure it is available on the system PATH."),
"FFmpeg must be installed to convert PDFs to video. Install FFmpeg and ensure it is"
@@ -1221,7 +1235,10 @@ public class ExceptionUtils {
OUT_OF_MEMORY_DPI(
"E081",
"error.outOfMemoryDpi",
"Out of memory or image-too-large error while rendering PDF page {0} at {1} DPI. This can occur when the resulting image exceeds Java's array/memory limits (e.g., NegativeArraySizeException). Please use a lower DPI value (recommended: 150 or less) or process the document in smaller chunks.");
"Out of memory or image-too-large error while rendering PDF page {0} at {1} DPI."
+" This can occur when the resulting image exceeds Java's array/memory limits"
+" (e.g., NegativeArraySizeException). Please use a lower DPI value"
+" (recommended: 150 or less) or process the document in smaller chunks.");
"APIs for converting, editing, securing, and analysing PDF documents. Use these endpoints to automate common PDF tasks (like split, merge, convert, OCR) and plug them into your own apps and backend jobs."));
"APIs for converting, editing, securing, and"
+" analysing PDF documents. Use these"
+" endpoints to automate common PDF tasks"
+" (like split, merge, convert, OCR) and"
+" plug them into your own apps and"
+" backend jobs."));
})
.build();
}
@@ -79,7 +84,9 @@ public class SpringDocConfig {
openApi.getInfo()
.title("Stirling PDF - Management API")
.description(
"Endpoints for authentication, user management, invitations, audit logging, and system configuration."));
"Endpoints for authentication, user management,"
+" invitations, audit logging, and system"
+" configuration."));
})
.build();
}
@@ -102,7 +109,8 @@ public class SpringDocConfig {
openApi.getInfo()
.title("Stirling PDF - System API")
.description(
"System information, UI metadata, job status, and file management endpoints."));
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Ludy87