Repository navigation
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: zhouyi7.
|
🦋 Changeset detectedLatest commit: 3b31425 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The JS SDK's parseOutput set OutputMessage.timestamp to `new Date().getTime() * 1000` (client clock, microseconds), ignoring the `timestamp` field the kernel sends in each stdout/stderr message. The Python SDK uses the backend `data["timestamp"]` directly, and the OutputMessage docs describe it as "Unix epoch in nanoseconds" — so the JS value was both from the wrong clock and the wrong unit, diverging from Python and from the documented contract. Read `msg.timestamp` instead, matching the Python reference impl. This is JS-only: Python already does the right thing. Co-Authored-By: Kscc <noreply@owtffssent.com>
8cd9153 to
3b31425
Compare
|
We require contributors to sign our Contributor License Agreement, and we don't have @lemonzoey on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
the timestamp should be a Date, not str |
Summary
The JS Code Interpreter's
parseOutputsetOutputMessage.timestamptonew Date().getTime() * 1000— the client-side clock in microseconds — ignoring thetimestampfield the kernel sends with every stdout/stderr message.This diverged from Python (which uses the backend
data["timestamp"]directly) and from the documented contract (OutputMessage.timestampis described as "Unix epoch in nanoseconds"). The JS value was from the wrong clock and the wrong unit.Fix: read
msg.timestampin the stdout and stderr branches ofparseOutput, matching the Python reference implementation. JS-only — Python is already correct.Closes #1969.
Why this is a real bug (not a pseudo-gap)
data["timestamp"]with no fallback, so the kernel always sendstimestamp; JS readsmsg.textfrom the same message but dropsmsg.timestamp.parseOutputa stdout message withtimestamp: 1234567890yields~1791599674953000(Date.now()*1000), not1234567890.Verification
New tests assert
OutputMessage.timestampequals the backendmsg.timestampfor both stdout and stderr (construct anExecution, feedparseOutputa JSON line with a fixedtimestamp, assert the captured message carries it through).Usage example
Changeset
@e2b/code-interpreterpatch —OutputMessage.timestampnow reflects the backend timestamp instead of the client clock.