HF-307: license key reader (entitlement key format, rev 6), resolution, and capability table - #1730
Merged
sequba merged 22 commits intoSep 24, 2026
Merged
Conversation
Task 2.1: LicenseCapabilityMissingError in src/errors.ts, mirroring the existing ~30 error classes; private ensureCapability(feature) in HyperFormula.ts mirroring ensureEvaluationIsNotSuspended. Task 2.2: ensureCapability wired as the FIRST statement (before argument validation) in the ~20 methods spec'd for PR 2 - NamedExpressions (addNamedExpression, changeNamedExpression, removeNamedExpression), Clipboard (copy, cut, paste), Crud (addRows, removeRows, addColumns, removeColumns, moveCells, moveRows, moveColumns, addSheet, removeSheet, clearSheet, setSheetContent, renameSheet, setCellContents), UndoRedo (undo, redo), Batching (batch, suspendEvaluation, resumeEvaluation). Read-only accessors (listNamedExpressions, getNamedExpression, getAllNamedExpressionsSerialized) are left ungated, resolving the open "getter scope" question from the handoff: gate B's own precedent already draws this line at mutation vs. read (it blocks calling a function, not reading a cell's existing value), so a restricted entitlement can still see named expressions that already exist. Task 2.3: BuildEngineFactory.ensureNamedExpressionsCapability - same allowsFeature(FeatureId.NamedExpressions) check, applied only when the namedExpressions argument to buildFromSheets/buildFromSheet/buildEmpty (the three factories buildFromArray/buildFromSheets/buildEmpty resolve to) is non-empty. Deliberately not applied to rebuildWithConfig, which re-serializes named expressions an already-built instance was already allowed to create, rather than accepting them fresh from a caller. Every ensureCapability call is a single boolean read (config.isLicenseGateActive) on the fast path, matching gate B's hot-path property; this ships without a real license-key payload adapter (PR 3), so every entitlement Config can produce today is unrestricted and the guard is a correct, independently-testable no-op in production. Found while writing tests: PR 1's licence.spec.ts restrictEngine() test helper granted an empty feature set, which now also blocks the setCellContents calls those tests use to set up their formulas, before gate B ever runs. Fixed by having that helper grant Crud by default - those tests are about gate B's function-level check, not this PR's Crud feature gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuUdq242TtFaepKRNdEkj9
Self-review of the just-opened PR found four public mutating methods that ensureCapability never covered: swapRowIndexes, setRowOrder, swapColumnIndexes and setColumnOrder. They permute sheet structure exactly like moveRows/moveColumns, which were gated - so a restricted entitlement with no Crud grant could still reorder every row and column in a sheet, which defeats the gate for a whole class of structural mutation. Each of the four is gated in its own right rather than relying on the swap* method the set*Order pair delegates to, so the license error still precedes their own argument validation (task 2.2's first-statement rule). Also writes down where the line is drawn, because "we chose not to gate this" was previously indistinguishable from "we forgot this": - gated: mutations that create value (sheet, clipboard, undo history, named expressions) - not gated: reads, and teardown that only removes state (clearClipboard, clearUndoStack, clearRedoStack, destroy) - gating cleanup would strand an integration mid-teardown and give a licensee nothing And records the gate-A asymmetry as an invariant: ensureCapability checks entitlement only, never key validity, which is what preserves today's behaviour where a missing key yields #LIC! in cells but keeps the CRUD API working. A later PR that resolves an invalid key to a restricted rather than unrestricted entitlement would silently turn that into a breaking API change - the note is there so that happens on purpose or not at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuUdq242TtFaepKRNdEkj9
Cursor Bugbot flagged this on the open PR; verified and fixed. resumeEvaluation is the only exit from a suspended engine, and _evaluationSuspended survives rebuildWithConfig. So an instance suspended while Batching was granted, whose entitlement then loses Batching through updateConfig, was stuck suspended permanently: every read throws EvaluationSuspendedError and the sole recovery path threw LicenseCapabilityMissingError. No public escape. suspendEvaluation and batch stay gated - those are the entry points that make the feature worth licensing, and if you cannot enter batching you can never extract value from it. Gating the release valve only strands the caller, which is the same reasoning that already left teardown (clearClipboard, clearUndoStack, clearRedoStack) ungated. Stated as a rule on ensureCapability so it does not get re-added: a capability check must never be reachable only on the way OUT of a state it let the caller into. Two regression tests cover it (resume works after the grant is revoked; the engine is actually left unsuspended afterwards). Both verified by mutation - re-adding the gate fails them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuUdq242TtFaepKRNdEkj9
Contributor
|
Task linked: HF-307 Implement feature packages and add-ons in HF |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
hyperformula-docs | 6b627a6 | Sep 24 2026, 11:45 AM |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Performance comparison of head (6b627a6) vs base (954d8c8) |
Collaborator
Author
|
bugbot run |
Collaborator
Author
|
bugbot run |
12 tasks
…kaging gap
Fixes three confirmed findings from an independent spec-to-ship review of PR2
(5-dimension multi-agent workflow, adversarially verified):
1. HIGH - hard-gating bypass. paste() checked only FeatureId.Clipboard, but
CrudOperations.paste() dispatches to moveCells() internally when the clipboard
holds a cut - the same cell-relocating mutation the public moveCells() requires
Crud for. A Clipboard-only entitlement could reach it via cut()+paste(), with
no Crud grant ever checked. Fixed by adding a public CrudOperations.isCutClipboard()
wrapper and gating paste() on Crud too when the clipboard holds a cut - still
checked before argument validation, matching every other ensureCapability call.
Verified by mutation: reverting the fix makes the new regression test fail
(paste succeeds and actually moves the cell) with the fix reverted.
2. HIGH - packaging gap. LicenseCapabilityMissingError was never imported/exported
from src/index.ts, so it was unreachable from the package's public entrypoint:
`import {LicenseCapabilityMissingError} from 'hyperformula'` failed to compile
(TS2614), the default-export static form was undefined, and no deep import
worked either (package.json's exports map only defines "." and the i18n
subpaths). A consumer had no supported way to catch this error by type. Fixed
by adding it alongside the other ~30 error classes already exported there.
3. MEDIUM - documentation. Added @throws [[LicenseCapabilityMissingError]] to all
27 gated instance methods and the 3 static build factories (buildFromArray/
buildFromSheets/buildEmpty, which throw it via BuildEngineFactory whenever
namedExpressions is non-empty against a restricted entitlement) - matching
this file's own established per-method @throws convention, which every other
exception type already follows. Also corrected the class-level @see list on
LicenseCapabilityMissingError itself: it wrongly named resumeEvaluation (which
this PR deliberately does NOT gate, to avoid stranding a suspended engine) and
omitted all 17 Crud methods; it now lists every method that can actually throw
it and explains the resumeEvaluation exclusion.
Also generalizes an earlier, narrower finding (a separate manual review had
flagged only setCellContents as untested): mutation-testing all 27 ensureCapability
call sites found 17 with zero regression protection of their own - a future
accidental removal of any one of them would ship silently, since the suite's
"one test per feature group" strategy only pinned the group representative.
Added one dedicated throw test per previously-uncovered method (setCellContents,
removeRows, addColumns, removeColumns, moveCells, moveRows, moveColumns, addSheet,
removeSheet, clearSheet, setSheetContent, renameSheet, cut, paste x3 for the
bypass fix itself, redo, changeNamedExpression, removeNamedExpression), plus one
test proving LicenseCapabilityMissingError is reachable from the public entrypoint
(importing from the package root rather than src/errors directly, which is why
the packaging gap went unnoticed by the existing suite).
Verified: tsc --noEmit clean; eslint 0 new errors; full private suite green
(511/511 suites, 6370/6373 tests, 3 pre-existing skips); the paste/cut fix and
the index.ts export both re-verified against a freshly rebuilt commonjs package.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuUdq242TtFaepKRNdEkj9
marcin-kordas-hoc
force-pushed
the
hf-307-entitlement-gating-pr3
branch
from
August 18, 2026 08:54
05882e1 to
ab1bba1
Compare
Contributor
Tobiadefami
reviewed
Aug 24, 2026
Tobiadefami
previously approved these changes
Aug 24, 2026
Tobiadefami
dismissed
their stale review
August 24, 2026 15:38
Approval retracted because the repository requires obsolete PR descriptions to be updated. The code and paired tests remain reviewed with no scope-aligned finding; re-approval only requires confirming the refreshed PR description and unchanged head.
Tobiadefami
reviewed
Aug 24, 2026
Tobiadefami
left a comment
Tobiadefami
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at ab1bba1 together with the paired tests at 9977ea88. The typed-key reader, license resolution, capability mapping, legacy-key invariant, and current automated findings were checked. The six changed paired suites pass (131 tests), and the current engine checks are green. I found one material issue: the expired-key console date ignores the key's grace period, as noted inline.
…able Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
marcin-kordas-hoc
force-pushed
the
hf-307-entitlement-gating-pr3
branch
from
September 18, 2026 10:15
8dcca66 to
e88b96b
Compare
marcin-kordas-hoc
changed the title
sequba
requested changes
Sep 22, 2026
Comment on lines
+72
to
+74
| `allowJs` is off in HyperFormula's `tsconfig.json` and `strict` is on, so these files are a port | ||
| rather than a copy. Beyond adding types, the semantics were kept identical except for the | ||
| following, which a drift review should expect to see: |
Contributor
There was a problem hiding this comment.
Can we add exceptions to tsconfig so these files could be the byte-identical copies of the upstream?
Collaborator
Author
There was a problem hiding this comment.
@sequba I would like this too, and it is worth more than tidiness: byte-identical files turn drift control from "re-read and re-port the changed file" into sha256sum, which is the whole point of the hash table in that document.
But it is not just allowJs. The files would have to be emitted and bundled as .js inside a strict build that also emits declarations, which means either checkJs: false plus a hand-written .d.ts per file, or @ts-check-clean upstream sources. I have not measured whether the rollup and declaration steps survive that.
Before I spend that: is the goal (a) a mechanical drift check, or (b) literally no local edits? If (a), there is a cheaper route — keep the port and add a CI job that clones the upstream at the pinned commit and compares. §7.1 of the spec asks for a drift check in CI anyway, and the document currently admits the check is manual.
One thing that collides either way: there is a deliberate local divergence in extractKeyData.ts (see my separate comment on the divergence list). A byte-identical copy cannot keep it.
Contributor
There was a problem hiding this comment.
The goal is (b). What options do we have to achieve that?
Collaborator
Author
There was a problem hiding this comment.
@sequba Options:
- Verbatim upstream
.js+allowJs+ hand-written.d.ts— prototyped; builds, and only theisIsoDatetests fail. - Same, via git subtree.
- Upstream publishes a public, reader-only npm package.
- Upstream ships TS or strict-clean JS.
Any of them drops that guard (upstream accepts usage_until: ["2099-12-31"]). I'd upstream it first, then do 1.
Comment on lines
+179
to
+185
| /** The group tokens each package adds, exactly as the packaging doc's §4 table states them. */ | ||
| const MATH_ENGINE_GROUPS = ['fun:math.a', 'fun:stat.a', 'fun:logic.a', 'fun:operator.a', 'fun:info.a', 'fun:lookup.a'] | ||
| const CALCULATED_FIELDS_GROUPS = ['fun:time.b', 'fun:text.b', 'fun:logic.b', 'fun:math.b', 'fun:stat.b'] | ||
| const SPREADSHEET_GROUPS = [ | ||
| 'fun:lookup.c', 'fun:math.c', 'fun:stat.c', 'fun:time.c', 'fun:text.c', 'fun:info.c', 'fun:logic.c', | ||
| 'fun:finance.c', 'fun:engineer.c', 'fun:array.c', | ||
| ] |
Contributor
There was a problem hiding this comment.
These groups should not be hardcoded. The code should operate on a capabilities level and should know nothing about what capabilities are in what package.
Collaborator
Author
There was a problem hiding this comment.
@sequba On the group constants you are simply right: MATH_ENGINE_GROUPS / CALCULATED_FIELDS_GROUPS / SPREADSHEET_GROUPS are the packaging table transcribed into the engine — the comment above them literally says "exactly as the packaging doc's §4 table states them". That belongs to whatever mints keys, not here.
Your broader point I want to scope before acting on, because it touches a decision we recorded in August (D8): the HyperFormula token registry belongs to us — rev 6 §2.2, and the shipped 4.0.0 schema declares hyperformula: [] with the whitelist off. Two readings:
- (a) the engine knows token → functions but nothing about package → tokens. Then the three constants and the
functions_1..4grants go, and thefun:<family>.<tier>table stays. - (b) the engine knows nothing, and a key carries function ids directly. That retires the group-token vocabulary and reverses D8.
Which one? I would rather reopen D8 deliberately than by accident. This also gates the unrestricted reshape, since both rewrite the same file.
Collaborator
Author
There was a problem hiding this comment.
@sequba Done. license-key's default schema still mints functions_1..4 for HF — it needs to emit the fun:* group tokens instead. Should spreadsheet / import_export become feat:* too?
Pasting a cut relocates cells, which is the mutation moveCells() requires Crud for, so the paste path reached a Crud-shaped effect through a clipboard-only entitlement. That route is accepted as a product decision: granting the clipboard grants what the clipboard does. Drops the second capability check and the wrapper added to reach it; the clipboard-state helper it delegated to predates this work and stays. The paired suite now pins the accepted behaviour rather than the refusal, so the gate cannot come back unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/license/CapabilityRegistry.ts
…rovenance Renames `src/license/vendor/` to `src/license/handsontable-license-key-parser/` so the directory names the repository it mirrors, and renames the paired test directory to match. Drops two provenance paragraphs that only described the older, superseded port rather than the code beside them. Corrects the divergence list, which is the part a drift review reads. Item 3 described upstream's own date check as if it were this port's, which the code has not matched since a type guard was added: upstream matches the stringified value against the date shape, so a value that merely spells a date once stringified is carried into a restricted entitlement, while here it takes the invalid-key path. That is the only divergence which changes what a key is worth, so it is now listed in its own right, with what the key specification says about the field and the note that upstream has not adopted it. The public guide no longer describes the key formats, and the validation section now points at the terms of the contract instead of enumerating the dates it compares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The throws tag still said paste() refuses without the Crud feature when the clipboard holds a cut. That check was dropped when the bypass was accepted, so the tag documented an exception the method cannot raise, three lines above a comment explaining why it does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 27a672c. Configure here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guide no longer documents what a proprietary key looks like, and the changelog entry carried the same description in the same words. It says what changed and what is unaffected instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@sequba One more thing in the vendored parser, found while acting on your comments rather than from your review — flagging it because it is the only divergence from upstream that changes which keys are accepted.
Two problems with how that was documented, both now fixed in
On the substance: the key spec's addendum (T7) makes that field a real calendar date, so the stricter reading is the specified one and it is upstream that deviates. Upstream has not adopted it — checked at |
A key can run out along two axes and only one of them is about the build in use. A maintenance key stops covering releases, so an older version keeps working and installing one is the fix; a usage-based key stops being valid at all, and telling its holder the key "is not valid for the installed version" sends them to downgrade, which changes nothing. Both went through one message, because the wording predates the usage axis this work introduced. The axis now reaches the message, and the classic 25-character format keeps the old wording, being release-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merges hf307-fix/pr2 (unrestricted -> functions/features axis split) into this PR3 branch. One conflict in CapabilityRegistry.ts's resolve() JSDoc, where PR3 had independently added the case-insensitive token matching paragraph (normalizeCapabilityToken) - kept both, combined into one paragraph. Also fixes a JSDoc line in HyperFormula.ts (licenseGrantsFunction) that named the removed `licenseCapabilities.unrestricted` field; it now describes both axes being set to 'all'. No other file in this PR reads ResolvedCapabilities.functions/.features directly - getRegisteredFunctionNames and the metadata API route through allowsFunction/licenseAllowsFunction, which already handle the new shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The licenseGrantsFunction JSDoc still named the removed licenseCapabilities.unrestricted field; the edit was made on disk in the same session as the CapabilityRegistry.ts merge conflict but not staged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Following the review's answer to which reading applies: the engine knows what each capability token grants and nothing about which tokens make up a package. - The package tokens functions_1..4 are gone, with the three group lists and cumulative grants that defined them. A package reaches the engine only as the list of group tokens a generator writes into the key, so a key's functions are the union of the tokens it names. Measured against the packaging document's own membership lists, every package keeps exactly the functions it had. - The always-granted core token is gone. The operator callable forms such as HF.ADD are now gated like any other function, through fun:operator.a or their own single-function token, as the review asked. The infix operators are not function calls and never reach the entitlement check, so they work under any key. - fun:all now also covers the operator callable forms, since it names the whole catalog. The ordering note on the table is dropped: the registry's reverse index is only ever consulted to ask whether a function is covered at all, so which token covers it first no longer matters. The upstream generator's default schema still words HyperFormula packages as functions_1..4, so a key minted from it grants no function until that schema emits group tokens instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…iption The invariant no longer has a core token to fall back on, and the spec it cites lives in the paired test repository, which the comment now says. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Base automatically changed from
hf-307-entitlement-gating-pr2
to
hf-307-entitlement-gating-pr1
September 24, 2026 11:44
sequba
added a commit
that referenced
this pull request
Sep 24, 2026
…cate its static form (#1743) > **Update 2026-09-22 — review round with @sequba.** The sections below are the PR as written > before that round, kept for the reasoning they record. Where a sentence no longer matches the > code it is corrected inline and marked, rather than rewritten away. ## Update 2026-09-18 — rebased onto the collapsed engine/tests PRs `hyperformula#1730` and `hyperformula-tests#32` were folded from five PRs each into one, so this PR's base moved to `#1730`. Nothing in this PR's own diff changed; the "9/9" numbering and the `#1741` base reference below are stale — the stack is now `#1728 → #1729 → #1730 → this PR`. --- 9/9 of the HF-307 stack, stacked on #1741. Pairs with `hyperformula-tests#43` — **merge the tests PR first**. Finishes decision **D2** on the one API it missed. ## The problem `getRegisteredFunctionNames()` returned the whole catalogue under a restricted key (measured: 422 names on a calculated-fields key, including functions that evaluate to `#LIC!`). A function picker built on it has exactly the problem #1724 (HF-349) and #1731's metadata filter exist to prevent. Flagged in #1731's "found, not fixed here" section; this closes it. ## What changed **Instance method** — now lists exactly what the instance can evaluate, sharing the one `licenseListsFunction` rule with the interpreter and the metadata API so the three surfaces cannot drift: - reads `getListableFunctionIds()` instead of `getRegisteredFunctionIds()` — the protected built-ins are included uniformly (`OFFSET` was missing before; `getAvailableFunctions` already listed it); - reads `config.translationPackage` — the instance's own snapshot — instead of a fresh global `getLanguage` lookup, which can report a localized name the instance refuses to evaluate, and which throws once the host unregisters that language code; - filters by the license, with the invariant intact: a missing, invalid, or expired key does **not** shorten the list. **Static method — deprecated, not removed.** See below; this is a change from the first version of this PR. ## The static: deprecated rather than removed (changed after review) The first version of this PR deleted `HyperFormula.getRegisteredFunctionNames(code)`, citing HF-349 as precedent. That was wrong, and the review caught it: ``` $ git show 3.4.0:src/HyperFormula.ts | grep -c "public static getRegisteredFunctionNames" # 1 ``` The method **is in the released 3.4.0 tag**, and HF-349's own commit message says its removal was free precisely because *"Both methods are unreleased, which is the only free moment to remove them."* So the precedent does not extend here: deleting it would be a breaking change in a minor release, against the Semantic Versioning this project states it follows, and DEV_DOCS's Definition of Done would require a migration-guide section that `docs/guide/` has no 3.x home for. So it is now `@deprecated` with the wording this repo already uses for that situation (`arraySizeMethod` / `arrayFunction` in 3.1.0: "deprecated and will be removed in one of the next major releases"), plus a `Deprecated` changelog entry. **Overturnable in one comment** if you would rather take the break now — the direction is Kuba's D2 either way; only the timing changed. The deprecation notice is explicit that the two are not interchangeable: the static translates into any registered language without an engine, so migrating means building one (`HyperFormula.buildEmpty({ language: 'plPL' }).getRegisteredFunctionNames()`). ## D2's precondition, discharged Kuba's D2 answer made the removal conditional: *"perhaps we can remove the static methods, I'll check which methods does formula-builder use"*. That check is done — searched the org, and **both consumers call the instance form, not the static one**: | Consumer | Call site | Form | |---|---|---| | `formula-builder` | `packages/core/src/engine/functionCatalog.ts:81` — `this.engine?.getRegisteredFunctionNames?.()`, in try/catch, declared optional in `engine/types.ts:66` | instance | | `aurasheet` | `src/core/FormulaEngine.ts:97` — `this.hf.getRegisteredFunctionNames()` | instance | Both use it to build a **function picker** (`functionCatalog.ts`; `FormulaAutocompletePlugin.ts`) — the surface #1724's rationale was about — so the license filter added here improves both rather than disturbing them. Under `gpl-v3` or any unrestricted key their lists are unchanged. ## Spec-to-ship review (2026-08-20): also fixed here - **The translation-snapshot change was unpinned.** Mutation-verified: reverting it to the global lookup left 309 tests green, even though the commit message and the new JSDoc both name it. Now pinned by a test that unregisters the language after the engine is built — the scenario that distinguishes the two implementations, since `getLanguage` throws for an unregistered code while the snapshot keeps working. - **The licence guide listed only two of the three narrowing methods.** It now names this one too, in both places. - **The new JSDoc over-claimed parity** with `getAvailableFunctions`: for a function whose translation is the empty string this method returns `''` while that one falls back to the canonical id. The claim is now scoped to the ids and the licence rule, with the naming difference stated. - Reverting the static removal also removed the docs-build change it required, so `docs/.vuepress/config.js` is untouched by this PR again (it no longer builds one engine per documentation page). ## Testing New suite pins the alignment in both directions (agrees with `getAvailableFunctions` name for name; narrows on a restricted key; never narrows on a bad key; aliases gate with their canonical; answers from the instance's own snapshot). Full private suite: **517 suites / 6462 passing, 3 pre-existing skips**; `tsc --noEmit` and ESLint clean. ## Note for review `functions-metadata.spec.ts` now skips listed ids with no plugin: `OFFSET` is listed (it is callable) but parse-time resolved, so it legitimately has no registry metadata. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019pxNP45obT2LZfjitaCv9o ## The codecov/project dip, traced `codecov/project` was red at −0.02% while `codecov/patch` reported 100% of the diff hit. Rather than write that off as a threshold artifact, I measured coverage on this commit and on its base and diffed the per-file numbers: | file | base (8/9) | this PR, before the fix | |---|---|---| | `src/HyperFormula.ts` | 674 / 675 | 675 / 676 | | `src/interpreter/FunctionRegistry.ts` | 130 / 130 | **129 / 130** | | total | 12 933 / 13 268 | 12 933 / 13 269 | So the covered count did not move and one previously covered line stopped executing — a real consequence of this change, not a rounding artifact. The line was the **instance** `FunctionRegistry.prototype.getRegisteredFunctionIds()`, whose only caller in the whole repository was the method this PR rewrites. Nothing in `src/`, nothing in the private suite, and nothing outside (the class is not exported from `src/index.ts`) calls it any more. Removed, since this change is what orphaned it. The static `FunctionRegistry.getRegisteredFunctionIds()` is untouched and still used — by the deprecated static method above and by three specs. For the record, the single uncovered line left in `HyperFormula.ts` is pre-existing and not mine: `removeNamedExpression`'s unreachable `return []`, which already carries a `codecov note` comment explaining why it cannot be hit. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation and deprecation only; no runtime behavior changes in this diff. > > **Overview** > **Deprecates** the static `HyperFormula.getRegisteredFunctionNames(code)` API (removal planned in a future major) and documents how it differs from the instance method. > > The static form is documented as reflecting the **global** function registry and any registered language without an engine; callers should migrate via `HyperFormula.buildEmpty({ language: '…' }).getRegisteredFunctionNames()`. The **instance** `getRegisteredFunctionNames()` JSDoc now states it lists everything in the instance registry (registration, not license entitlement) and points license-aware UIs to `getAvailableFunctions()`. > > `CHANGELOG.md` adds a **Deprecated** entry for the static method. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a8ffef3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --- ### Open in the 2026-09-22 review round - @sequba: "`getRegisteredFunctionNames` should still list ALL functions". Agreed on the principle — this method answers for the REGISTRY, and registration is not entitlement, while `getAvailableFunctions()` and `getFunctionDetails()` answer about availability and stay filtered. That removes most of this PR's substance, leaving the deprecation note and a docs correction. **Awaiting a decision on whether to reduce this PR to that or close it with the deprecation folded into #1730.** - Fixed here meanwhile: `docs/guide/release-notes.md` now carries the same retroactive `### Removed` section for 3.4.0 that `CHANGELOG.md` gained in this PR's history. The two are mirrors and only one had it. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Kuba Sekowski <jakub.sekowski@handsontable.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## hf-307-entitlement-gating-pr1 #1730 +/- ##
=================================================================
- Coverage 97.34% 95.17% -2.17%
=================================================================
Files 198 204 +6
Lines 15851 16256 +405
Branches 3509 3602 +93
=================================================================
+ Hits 15430 15472 +42
- Misses 413 744 +331
- Partials 8 40 +32
🚀 New features to boost your workflow:
|
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
4 participants
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.
Update 2026-09-18 — folded #1731, #1736, #1737, #1740, #1741 into this PR
So that no PR in this stack shows code a later PR rewrites, this PR now contains, squashed with
no loss of content: the entitlement reader (rev 6, was #1740), the notice-window warning (was
#1736), the two add-on tokens (was #1737), the function-metadata filtering (was #1731), and both
capability-token dialects (was #1741). Those five PRs are closed, folded here. The stack is now
#1728 → #1729 → this PR → #1743.Two things below are stale as a result and are corrected here rather than edited in place, so the
history stays legible: the title above (fixed), and every PR-number reference in the body below
still says
#1740/#1741/etc. — read those as "now #1730". The CHANGELOG entries in the diff arealready repointed to #1730.
Everything below this line is the PR as it was written across 2026-08-11 through 2026-08-26,
before the fold — the decisions and verification it documents did not change.
Third of four. Stacked on PR 2 (#1729), so review the commits on top of it.
Paired tests: handsontable/hyperformula-tests#32 — merge that first.
Why this PR exists at all
A genuine typed license key did not work before this.
checkLicenseKeyValidityrecognizes three fixed strings and the older 25-character format; a typed key matches none of them and falls through toINVALID. I verified this by building an engine with a real, unexpired[SUB]key before touching anything:So a paying customer pasting a valid key got
#LIC!in every cell plus a console warning. An entitlement adapter alone would not have helped — gate A kills the formulas first.What lands here
1. Vendor the typed-key reader into
src/license/handsontable-license-key-parser/(src/license/vendor/allowJsis off,strictis on, so this is a port, not a copy). This follows the key spec's own recommendation of a vendored copy plus a drift check rather than a shared package: a private dependency would breaknpm installfor open-source users of this GPL package.PROVENANCE.mdrecords the upstream commit and a per-file sha256, so drift is detectable by re-cloning and re-hashing, and lists the deliberate divergences.2. Resolve typed keys into both gates.
resolveLicensereads the key once and answers both gates from that single reading, so they cannot disagree about what the string says. Anything that is not a typed key —gpl-v3, an older-format key, an empty string, a typed key with a broken checksum — falls through tocheckLicenseKeyValiditywith its verdict unchanged. That function is extracted from rather than rewritten, so both paths share one warning-per-page flag instead of each getting their own. Two additive edits to the file it lives in, both affecting the legacy path too and both deliberate:formatDatenow reads its date in UTC (it printed the expiry a day early west of UTC — a latent bug on the legacy path as well), and an@internalreset for the warn-once flag was added because it made the console path untestable.3. The real capability table, plus reading both payload shapes.
Please confirm two things
The gate-A/gate-B invariant. Only a VALID typed key resolves to a restricted entitlement; missing, invalid and expired all resolve to
unrestrictedEntitlement(). Gate A already stops formula evaluation on its own — letting a bad key restrict the entitlement as well would make PR 2'sensureCapabilitythrow from the CRUD API, turning today's "formulas fail, the API still works" into a silent breaking change for every user whose key lapsed. Mutation-tested.The capability table is a DRAFT. Membership is transcribed from the packaging design's own per-function evidence file rather than invented, and the transcript is checked two ways: it reproduces that file's five published counts exactly (370 rows; 17 / 51 / 127 / 355 cumulative plus 15 operators), and all 423 registered function ids resolve into the 370 canonical entries once the 53 declared aliases are canonicalised, with zero uncovered. But the design is still under review — the free tier's exact contents and the placement of several function families are not settled. Landing it now was a deliberate call, not a claim that it is final.
Judgement calls worth a reviewer's eye
capabilities: the shipped shape (tier/addons/exp) and the newer specified shape (capabilities/usage_until/release_until/notice/flags). They disagree about nearly every field, the newer one is still under review, and only the first can be minted today. The newer spec also contradicts itself on whether its dates areYYYY-MM-DDstrings or numeric timestamps, so both are accepted.coretoken, which made feature gating inert by construction. The five features now live on their ownfeat:*tokens: a rev-5 key grants exactly thefeat:*tokens it carries; the shipped shape — whose vocabulary predates feature tokens — is granted all five by the adapter, so a shipped-shape key's API behaviour is unchanged; legacy keys resolve unrestricted (Kuba's carve-out). The exits stay ungated (resumeEvaluation, teardown), per the never-gate-the-exit rule.flags; the earlier coupling (silent || unrecognized > 0) suppressed strictly more than D3 asks for and was confirmed an implementation error by Kuba.spreadsheethas no agreed content — the packaging design names a package "Spreadsheet" and the pricing work names a "Spreadsheet Bundle" add-on, and whether those are the same set is unsettled; guessing either way silently sells an empty add-on or duplicates a whole tier. Pinned by a test so filling it has to be deliberate.noticeDaysis 0 for the shipped shape, which has no notice field. Inventing a default in the parser would put a product decision in the wrong place. The newer shape carriesnoticeand it is used.PROVENANCE.mdnames the upstream private repo and commit. That is what makes the documented drift check runnable. Ratified by Kuba on the task (D7-A, 12.08): kept as-is.Update 13.08 — key spec rev 5 read against the code
Reading rev 5 (doc, updated 12.08) and running its own example payload through this branch turned up three things, all fixed in the last commit:
functions_1..4,spreadsheet,import_export— there is nofeat:*entry at all. So a key that names no feature token cannot be saying "no features"; nothing in circulation can express one. Minting the spec's own §2 payload and running it:setCellContents,addRows,copy,undo,addNamedExpressionandbatchall threw. The same hole hit Handsontable-only keys and keys withhyperformula: null, which lost the API thatcoreused to give them, silently (gate-A VALID = no console warning). A key that does name afeat:*token still gets exactly what it names, which is the gating Kuba asked for.no-console-warnsis honoured. rev 5 spells the flag three ways — §2.3 and the §2 example sayno-console-warns, §4.3/§5.2 saysilent-console, earlier revisions saidsilent. Only the last two were recognised, so a doc-conformant SaaS key printed console warnings it had explicitly asked to suppress.capabilitiesrejects the key. Present-but-not-an-array fell through to the shipped-shape branch, which was a free pass twice: every feature granted, and the rev-5 dates never read — a subscription expired in 2020 resolved as perpetual.Still divergent from rev 5, deliberately: §4.1 says a non-trial key never hard-blocks, even past grace ("Trial: block. Non-trial: error only (18.1)"). This branch keeps EXPIRED →
#LIC!, per Kuba's D5=A (hard blocking stays this release; the notice/soft-stop/hard-stop model is a follow-up).Verification
Full unit suite green — 512 suites, 6344 tests (after the 13.08 update). Each of the three fixes above is mutation-verified: removing the opt-in rule fails 9 tests, removing
no-console-warnsfails 1, removing thecapabilitiesguard fails 2, and cutting the console notification fails 1 (it used to fail none). Beyond that, the parts that matter were mutation-tested rather than trusted: a wrong SHA-512 round constant, a one-bit UTF-8 error, a broken invariant and a moved package boundary each fail the tests that are supposed to catch them.Note
High Risk
Changes authentication-adjacent license parsing, entitlement gating for formulas and public API, and console behavior for all proprietary keys—including new trial entitlement keys in production.
Overview
Adds proprietary license entitlements: keys can grant subsets of functions and API features via case-insensitive capability tokens (
fun:*,feat:*, and add-onsspreadsheet/import_export). Restricted functions evaluate to#LIC!; gated API areas throwLicenseCapabilityMissingError. Invalid, missing, or expired keys still leave the CRUD API unrestricted while formulas fail on gate A.Entitlement key format (rev 6) is vendored and wired through
resolveLicense, replacing the never-shipped tagged format. Legacy 25-character keys andgpl-v3keep the old path. Console messaging is split by expiry axis (usage vs release), dates are formatted in UTC, and valid usage-based keys can print a one-time “valid until …” notice.Capability resolution uses a static packaging-aligned table (function groups,
fun:all, per-function tokens).getAvailableFunctions()/getFunctionDetails()now list only what a valid partial key includes; bad keys still show the full catalogue.licenseAllowsFunctionkeeps the interpreter and metadata API aligned.Docs and changelog describe feature packages,
#LIC!semantics, and add-on grants (Spreadsheet Bundle → CRUD, undo/redo, clipboard, batching).Reviewed by Cursor Bugbot for commit 6b627a6. Bugbot is set up for automated code reviews on this repo. Configure here.
⛔ Update 18.08 — upstream retired this key format the same day, and one claim above is wrong
A scoped spec-to-ship review of this PR (5 dimensions, every finding adversarially verified) confirmed 5 findings. Four are test-hardening and are fixed in the paired PR — see hyperformula-tests#32's "Update 18.08". The fifth needs a decision, not a patch, so nothing here has been changed for it yet.
The vendored reader cannot read the format upstream now ships.
handsontable/license-keycommit01eae6530d4f(2026-08-18 08:44 UTC, DEV-2512, "Replace typed license keys with the entitlement key format (rev 6)"), released as 4.0.0, deletessrc/typed-key/outright — the exact five filesPROVENANCE.mdpins and hashes. Its own docs are explicit: "The typed key format was removed, not deprecated… this package cannot read a typed key." The replacement differs in three ways this reader gates on:[SUB]_/[FREE]_/[TRIAL]_/[PERP]_tag (extractKeyData.ts:147)v: 1(extractKeyData.ts:199)vfield at all — payload is exactly{"products":{…}}extractKeyData.ts:105)Measured, not inferred: an authentic 4.0.0-format key built per upstream's own
build-payload.jsgivesextractTypedKeyData(key)→null,resolveLicense(key).validityState→invalid, andC1→#LIC!"License key is invalid.". Passing only the[…]block behaves the same. The retired tagged format still resolves correctly on the same build — so this is a format gap, not a robustness one.Independently, Budzio published the byte-level spec for that format this morning as a new page under key-spec rev 6 ("Technical implementation", sections
T1–T8, created 09:45 UTC). It matches upstream and confirms all four rows above. Three of its rules this branch already satisfies: checksum verified before any decode or parse ("order is normative"), uppercase hex rejected rather than normalised, and__proto__-safe result construction.What is genuinely at stake, and what is not. The invariant holds throughout: a rev-6 key resolves INVALID and therefore to
unrestrictedEntitlement(), so the CRUD API keeps working and only formulas fail (verified). And myHOT PR #73 (merged tomaster12:37 UTC today, pinninglicense-key#4.0.0) states "the typed keys never reached production", with the legacy 25-character keys still generated in parallel and validating exactly as before — that path is untouched here. So the correction owed to this PR's own opening claim: no customer ever held a typed key, so "a paying customer pasting a valid key got#LIC!" describes a format that was never issued. The live exposure is narrower but real — per that PR's surfacing table, trial orders and the HubSpot trial webhook now surface the entitlement key, while commercial orders and transactional emails still show the legacy key only.Why this is not fixed in this PR. Re-porting is the obvious move and upstream made it cheaper than the original port (
sha512.jsandutils.jsare unchanged; the newextractEntitlementKeyDatais deliberately schema-free and built to be vendored, so only the extractor, schema and constants need re-porting). But it is not a patch to slip in: adopting the new shape rules verbatim also makescapabilitiesmandatory and invalidates every payload shape this branch currently reads, rev 6 is still status "for review", and the work is arguably HF-329's (unassigned, due 21.08, untouched since 04.08 — it now has concrete content). Raised on the task as D9; the merge freeze until the spec owner returns means escalating is the action, not a delay.Also worth a reviewer's eye, since it sharpens the still-unanswered D8: rev 6's own examples use
capabilities: ["functions_1", "spreadsheet"]/["core"]— i.e. exactly the token names this branch implements — while the packaging design published 12.08 states its vocabulary isfun:<family>.<A|B|C>and that "nothing else about packaging exists at the technical layer". The two documents disagree, and this branch conforms to the key spec rather than to the packaging doc. A key minted in the packaging vocabulary resolves VALID with the whole gated API and zero functions, silently (gate A says VALID, so nothing prints, andunrecognizedCapabilitiesis exposed nowhere). That outcome is now pinned by a test in the paired PR — pinned, not endorsed, so whichever way D8 lands has to change it.Changed in the 2026-09-22 review round
PROVENANCE.md: two paragraphs about the supersededsrc/typed-key/port removed; the divergence list corrected. Its item 3 described upstream's date check as if it were this port's, which has not been true since a type guard was added, and that guard — the only divergence that changes which keys are accepted — was missing from the list entirely.developcan see; it is not for a subscription key.impliesfield and its recursive expansion are gone (see HF-307: license-key entitlement gating — capability model, key reader, API guards #1728), so no capability token refers to another.