Document licensing, security, and SumUp; improve local precommit runs - #1918
Conversation
📝 WalkthroughWalkthroughThe change adds the AGPL license, updates security and payment documentation, and adds adaptive Deno worker configuration plus cross-process precommit locking with ordering and contention tests. ChangesDocumentation and licensing
Precommit coordination
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Runner as precommit runner
participant Order as runChecksBeforePush
participant Lock as withPrecommitLock
participant File as withFileLock
Runner->>Runner: resolveDenoJobs()
Runner->>Order: run checks and push
Order->>Lock: run local checks under lock
Lock->>File: acquire temporary lock
File-->>Lock: execute checks
File-->>Lock: await unlock
Lock-->>Order: checks complete
Order->>Runner: push checked-in changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c4194e7a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 406-408: Run the repository’s `deno task precommit` command for
this README-only change, then resolve any reported failures before merging while
preserving the license text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1cd01545-dda1-467e-bd1e-72f7f9b8ac77
📒 Files selected for processing (4)
LICENSEREADME.mdsrc/docs/crypto.tssrc/docs/payments.ts
stefan-burke
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4475f3d58e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 153916decf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f7a198b20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- lock.ts: make lock path injectable so tests stay off the live
PRECOMMIT_LOCK_PATH during test:coverage runs
- lock.ts: write PID via single writeTextFileSync({createNew:true}) —
atomic create+write removes the empty-file window the open-then-write
sequence left
- lock.ts: document the macOS/Darwin signal-0 fallback (Deno.kill with
0 as unknown as Deno.Signal) — Deno's runtime honours it for existence
checks even though its type doesn't include numeric 0
- lock.ts: extract removeIfExistsSync to scripts/not-found.ts and use it
for release/stealFromDeadHolder — kills the jscpd duplicates the
refactor surfaced within lock.ts and cross-file with specs/run.ts and
stripe-mock/install.ts
- lock.test.ts: unique per-run temp lock path so tests never collide with
a real precommit run; spawn a process that exits immediately for a
deterministic dead PID instead of guessing 999999 isn't in use
- workers.ts: extract resolveDenoJobs(hardwareConcurrency, ci, env) so the
DENO_JOBS env-setting behaviour is directly testable without spawning
precommit; runner.ts calls it and only sets DENO_JOBS when it returns
a value (explicit operator override wins)
- workers.test.ts: drop the duplicated parseWorkerCount tests (they
already live in test-groups.test.ts); add resolveDenoJobs coverage
- TODO.md: fix the inaccurate redirect description — the redirect path
calls retrieveSession -> classifySessionIntent -> completePaidBooking,
not retrieveSession -> finalizeSessionIfUnresolved (that's replaySuccess,
the already-ledgered path)
- README.md: say 'keyed role (owner, manager, or agent)' rather than
'keyed owner' — all three keyed roles receive a wrapped data key, so
any of them can recover the protected data, not just owners
- test-groups.test.ts: cover the new removeIfExistsSync helper
Co-Authored-By: Codex <noreply@github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d561fa6468
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/precommit/lock.ts`:
- Line 1: Make the polling delay injectable in acquirePrecommitLock and
pollOnce, defaulting to POLL_INTERVAL_MS while passing the configured value into
pollOnce’s setTimeout. Update the “waits…” test to use withVirtualBackoff or an
injected near-zero interval, removing its dependency on real-time sleeps.
In `@scripts/precommit/runner.ts`:
- Around line 125-147: Remove the unreachable !lock.acquired branch from
withLock, including its logging and exit behavior, since acquirePrecommitLock is
invoked with its default waiting behavior. Preserve the existing task execution
and lock.release cleanup flow; only add an explicit no-wait option if withLock
is intentionally extended to support it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 529b57ee-5ce3-42b4-afc2-33a6e3e9b6d1
📒 Files selected for processing (10)
README.mdTODO.mdscripts/not-found.tsscripts/precommit/lock.tsscripts/precommit/runner.tsscripts/workers.tssrc/docs/payments.tstest/lib/test-groups.test.tstest/scripts/precommit/lock.test.tstest/scripts/workers.test.ts
stefan-burke
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4bf464ce5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae7a97c065
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scripts/lock-file.test.ts`:
- Around line 68-122: Add a regression test alongside the existing withFileLock
tests that stubs the opened file’s unlock method, blocks it with a deferred
promise, and verifies the withFileLock promise remains unsettled and the file is
not closed before releasing that promise. Reuse the existing Deno.open stubbing
and lock-file test utilities, then release the deferred unlock and assert the
operation completes and cleanup occurs.
- Around line 8-11: Update LOCK_PATH to use the platform-aware tmpdir() helper
instead of the hardcoded "/tmp" fallback, and import tmpdir() following the
existing pattern in lock.ts. Preserve the TMPDIR override behavior if
applicable.
In `@test/scripts/precommit/run-order.test.ts`:
- Around line 5-53: Add a rejection-path test for runChecksBeforePush where the
checks callback fails; assert the returned promise rejects with that error and
verify the push callback is never invoked, while preserving the existing
successful ordering and CI coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b9043bd7-5432-49d5-a5f6-6c6a540f4e8e
📒 Files selected for processing (11)
README.mdTODO.mdscripts/lock-file.tsscripts/precommit/run-order.tsscripts/precommit/runner.tsscripts/workers.tssrc/doc.tssrc/docs/payments.tstest/scripts/lock-file.test.tstest/scripts/precommit/run-order.test.tstest/scripts/workers.test.ts
1 participant
Summary
DENO_JOBSoverridesWhy
The documentation now matches the software people run. Local precommit gates also avoid competing for the same machine without PID files, stale-lock cleanup, cross-user permission conflicts, or platform-specific process probes.
Testing
633305aa: passed lint, typecheck, duplication, edge build, and the full test suite with 100% coveragenix develop -c deno task test:files test/scripts/lock-file.test.ts test/scripts/precommit/lock.test.ts test/scripts/precommit/run-order.test.ts test/scripts/workers.test.ts test/lib/test-groups.test.ts(46 passed)deno checkand Biome checks for the final precommit source and test filesscripts/lock-file.ts,scripts/precommit/lock.ts, andscripts/precommit/run-order.ts(100% branches and lines)The local all-in-one precommit command was intentionally skipped at the maintainer request. The equivalent GitHub workflow passed every gate.