Read the custom instructions limit from an environment variable

[maxime-pharmania]

The custom instructions limit was the build-time constant USER_PROMPT_MAX_LENGTH, so a deployment that needs a longer prompt has to patch the source.

• resolveUserPromptMaxLength() in @proval/types reads PROVAL_USER_PROMPT_MAX_LENGTH; a value that is not a positive integer is ignored and the constant applies.
• The API enforces the resolved limit and announces it on GET /api/auth/me (userPromptMaxLength).
• The repository form uses the announced limit for the textarea and the counter, so the screen and the API agree.
• Tests: the repository service now checks that the variable is honoured and that invalid values fall back.

[proval-0e6d07]

Hi @seoes — this PR makes the custom-instructions limit configurable via PROVAL_USER_PROMPT_MAX_LENGTH and wires the resolved value end-to-end: server enforcement, /auth/me announcement, and the client form counter all flow through one shared resolver. The design is clean and I have no blocking concerns.

Overview

Summary

The hard-coded USER_PROMPT_MAX_LENGTH is replaced by a defensive resolveUserPromptMaxLength in packages/types/src/user-prompt.ts, used by both the repository validation path and the new userPromptMaxLength field on AuthMeResponse. The client picks the announced value up in +layout.svelte through a new validated store, keeping the textarea maxlength and character counter in sync. Enforcement is well tested on the API side; remaining gaps are documentation and typing polish noted below.

Good points

• 🟢 resolveUserPromptMaxLength (packages/types/src/user-prompt.ts:8) trims and rejects every non-positive-integer input before falling back to the 2000 default, so a malformed env value can never disable validation or announce a nonsensical limit.
• 🟢 Both the announcement (apps/api/src/api/auth/auth.service.ts:76) and enforcement (apps/api/src/api/repository/repository.service.ts:461) call the same exported resolver, so the limit shown to users and the limit the server applies cannot drift apart.
• 🟢 The invalid-env test (apps/api/src/api/repository/repository.service.test.ts:326) drives a parameterized list of bad values through the real update path with try/finally cleanup, directly pinning the "garbage falls back to 2000" contract.
• 🟢 The demo-mode load branch (apps/client/src/routes/+layout.ts:23) fills userPromptMaxLength so the demo payload stays shape-identical to the real AuthMeResponse, and setUserPromptMaxLength (apps/client/src/lib/store/instance.ts:10) re-validates anything the API sends before it reaches the form.

Additional Notes

• .env.example does not document PROVAL_USER_PROMPT_MAX_LENGTH even though the JSDoc in packages/types/src/user-prompt.ts:5 names it as operator-facing; a commented line matching the COOKIE_SECURE style would make the knob discoverable.
• apps/client/src/routes/+layout.svelte:16, const { data, children } = $props(); has no LayoutProps annotation, so data.auth?.userPromptMaxLength in the $effect is untyped — other route components use PageProps, and typing this would catch a renamed or dropped field at compile time.
• apps/api/src/api/repository/repository.service.test.ts:316 — the env tests delete PROVAL_USER_PROMPT_MAX_LENGTH in their finally blocks rather than restoring a pre-existing value, and the default-limit test at line 309 never clears it first, so an ambient export in CI would break the suite and be wiped for later tests.
• There is no test for the /auth/me announcement half (apps/api/src/api/auth/auth.service.ts:76) — a small getMe assertion for default/custom/invalid env values would guard against the announced limit silently diverging from enforcement.

[maxime-pharmania]

• .env.example documentation: fixed in 7c880c2 — added a commented PROVAL_USER_PROMPT_MAX_LENGTH=2000 line in the COOKIE_SECURE style.
• Untyped $props() in +layout.svelte: fixed in 7c880c2 — annotated const { data, children }: LayoutProps = $props() with the generated ./$types import, matching the PageProps pattern used elsewhere.
• Env cleanup in the repository tests: fixed in 7c880c2 — the default-limit test now clears PROVAL_USER_PROMPT_MAX_LENGTH first, and all three tests restore a pre-existing value via a restoreUserPromptMaxLengthEnv helper instead of deleting unconditionally.
• Missing /auth/me announcement test: fixed in 7c880c2 — new apps/api/src/api/auth/auth.service.test.ts asserts getMe announces the default, custom ("9000"), and invalid-env limits.

[proval-0e6d07]

Hi @seoes — this follow-up cleanly closes out every gap raised in the previous review: env-safe test cleanup, .env.example documentation, +layout.svelte prop typing, and the previously missing /auth/me announcement tests. No blocking concerns.

Overview

Summary

The push replaces blind delete process.env.PROVAL_USER_PROMPT_MAX_LENGTH cleanup with a save-and-restore helper across all six env-mutating tests, adds a new auth.service.test.ts covering the default/custom/invalid matrix for the announced limit, documents the knob in .env.example, and types $props() with LayoutProps. This resolves all four Additional Notes from the prior review.

Good points

• 🟢 The restoreUserPromptMaxLengthEnv helper (apps/api/src/api/repository/repository.service.test.ts:7, mirrored at apps/api/src/api/auth/auth.service.test.ts:45) restores the exact prior env value (or deletes it if unset) in finally blocks, so ambient CI exports no longer leak into assertions or get wiped for later tests.
• 🟢 The new announcement tests (apps/api/src/api/auth/auth.service.test.ts:55) cover the full env matrix — default, "9000", and invalid "abc" — via the real getMe query flow rather than a stubbed service, pairing with the enforcement matrix already in repository.service.test.ts.
• 🟢 LayoutProps annotation on $props() (apps/client/src/routes/+layout.svelte:18) compile-checks data.auth?.userPromptMaxLength against the load's return type, so a future rename or dropped field in +layout.ts fails type-checking instead of silently leaving the store at its default.
• 🟢 .env.example:4 documents PROVAL_USER_PROMPT_MAX_LENGTH=2000 with a descriptive comment, using the exact env var name consumed at repository.service.ts:461 and auth.service.ts:76, with a default matching USER_PROMPT_MAX_LENGTH.

Main Issues

None — no critical or problem-level findings in this push.

Additional Notes

• apps/api/src/api/repository/repository.service.test.ts:317 — every userPrompt test asserts rejection only; there is no accept-path test proving a prompt at exactly the effective limit (default 2000, or exactly 50 with PROVAL_USER_PROMPT_MAX_LENGTH=50) passes, so the inclusive boundary at repository.service.ts:462 (trimmed.length > maxLength) is unpinned against a > → >= regression.

[maxime-pharmania]

Right, and worth pinning: fixed in 1918e5e.

A new case accepts a prompt of exactly USER_PROMPT_MAX_LENGTH and, in the same test, exactly the limit set by PROVAL_USER_PROMPT_MAX_LENGTH=50, asserting the value comes back from the update. I checked that the test does pin the boundary by mutating trimmed.length > maxLength into >=: the suite went from 62 pass / 0 fail to 7 pass / 1 fail on the new case, then back to 63 pass / 0 fail once restored.

[proval-0e6d07]

Hi @seoes — this follow-up adds the accept-path boundary test that was the only open note from the previous review, and it closes it cleanly.

Summary

A single new test in apps/api/src/api/repository/repository.service.test.ts asserts that a userPrompt of exactly the effective limit (default USER_PROMPT_MAX_LENGTH, and exactly 50 under PROVAL_USER_PROMPT_MAX_LENGTH=50) is accepted, so a > → >= regression at the trimmed.length > maxLength check (apps/api/src/api/repository/repository.service.ts:462) now fails on the accept side, not just the reject side. Env save/restore follows the established restoreUserPromptMaxLengthEnv helper pattern. No blocking concerns.

Good points

• 🟢 The new test (apps/api/src/api/repository/repository.service.test.ts:329) pins the inclusive boundary at exactly the default limit and exactly 50 with a custom env value — precisely the cases a > → >= regression would break, since the awaited update() would throw before touching the DB.

Main Issues

None — no critical or problem-level findings in this push.

Additional Notes

• apps/api/src/api/repository/repository.service.test.ts:336 — expect(updatedAtDefault.userPrompt).toBe(atDefaultLimit) echoes the row the test itself injects into updateReturningMock (line 334), and update() returns toResponse(result[0]) straight from that mock, so the assertion doesn't verify what was actually passed to the DB; consider also asserting updateSetMock.mock.calls.at(-1)?.[0] matches { userPrompt: atDefaultLimit }. The awaited non-throwing still genuinely covers the boundary.

[seoes]

Hi @maxime-pharmania, thanks for the PR.

Yeah I set a limit for user prompt to 2000 and now that I look at it, it seems too short. I didn't mean to give users a short limit. It can be like 100,000 or more. (Hosting with own model and can't even set user prompt because of limit is crazy) I'm willing to change that 2000 limit.

and to be honest, I’ve built Proval from the start around keeping things minimal and simple, and I’ve tried to stick to that in the code and structure. Your implementation is solid, but for “user prompt max length” alone it still feels like a lot of surface area.

If you’re ok with it, I’d rather do a small change that only bumps PROVAL_USER_PROMPT_MAX_LENGTH to 100_000 instead of landing this PR. You put real work into this and I’m sorry to suggest backing off from it. Let me know what you think, I’d really appreciate it.

BTW, Proval recently got support for reading each repo’s AGENTS.md in #77