Repository navigation
fix(postgres): run the installed command, dedupe by instant, refuse out-of-range offsets - #49
Merged
Merged
Conversation
…ut-of-range offsets Three findings Greptile raised on the MongoDB destination that this package shares: - The entrypoint check compared `process.argv[1]` against `cli.js`, but npm installs the `bin` as a symlink named `workos-sync-postgres`, so the installed command imported `main()` and exited. Both sides are now resolved to real paths and compared. - `dedupeByKey` compared `updatedAt` as text, so a batch mixing offset and UTC forms could keep the older row. It now orders by the instant, and falls back to text for a value that is not a date, which the database refuses on the write anyway. - `parse_iso8601` range-checked the clock but not the offset, so `+00:99` was accepted and applied. The function is recreated on every `init`, so existing deployments pick the check up on their next one; no generated column expression changes.
 [Medium impact] The PR appears safe to merge; the previous timestamp precision issue is fixed. SummaryThis PR fixes the installed command, timestamp ordering, and invalid timezone offsets.
The previous precision finding is fixed. No new actionable issues were found. Reviews (2) · Last reviewed commit: "fix(postgres): order a batch's timestamp..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… keeps Date.parse reads three fractional digits and a timestamptz keeps six, so two removals a microsecond apart tied and the later row in the batch won, whichever was newer. The rest of the fraction is read alongside it now.
gjtorikian
added a commit
that referenced
this pull request
Nov 5, 2026
… out-of-range offsets The same three fixes Postgres (#49) and MySQL (#50) got. npm installs the bin as a symlink named after the command, which the old argv check did not recognise, so `npx workos-sync-clickhouse` imported the CLI and did nothing. Duplicate keys in a batch are now judged by the instant each timestamp names rather than by its text, so `01:00:00+01:00` no longer outranks `00:30:00Z`. And the timestamp projection range-checks the clock and the offset ahead of the best-effort parser, which read `24:00` as the next midnight and `+00:99` as a zone where the other destinations project NULL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three bugs Greptile found in the MongoDB destination (#41) that
sync-postgresshares, each with a test:process.argv[1]againstcli.js, but npm installs abinas a symlink namedworkos-sync-postgres, sonpx workos-sync-postgresimportedmain()and exited.invokedDirectly()now resolves both the argv path and the module URL to real paths before comparing. The Docker image was unaffected, since it runsnode dist/cli.jsdirectly.dedupeByKeycompared timestamps as text, so a batch mixing offset and UTC forms (01:00:00+01:00vs00:30:00Z) could keep the older row on the delete, relink and unlink paths. It now orders by the instant, falling back to text for a value that is not a date, which the database refuses on the write anyway.parse_iso8601accepted out-of-range offsets such as+00:99and applied them. The offset's hours and minutes are now range-checked like the clock's. The function is recreated on everyinitbefore the fingerprint check, so existing deployments pick the check up on their nextinit; no generated column expression changes, so there is no drift to repair.