Skip to content

fix(ios): post an event's ts in unix milliseconds (#154) - #158

Merged
V3RON merged 6 commits into
mainfrom
issue-154-ios-events-report-a-timestamp-in-seconds
Oct 8, 2026
Merged

V3RON merged 6 commits into
mainfrom
issue-154-ios-events-report-a-timestamp-in-seconds

Conversation

@V3RON

@V3RON V3RON commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Closes #154

What changed

An event posted from an iOS app now reports its time in milliseconds, the unit docs/PROTOCOL.md §4 documents and the only one Android sends. The iOS core divided timers.now() by 1 000 on the way onto the wire even though that clock already reports milliseconds, so the app's own timestamp was 1 000× in the past and showed as a 1970 date wherever data.ts is printed. One division removed, plus a Swift test that pins the frame's ts to the fake clock's milliseconds so the unit is now specified rather than assumed. The daemon, CLI, MCP surface and the Kotlin core are untouched; @appduct/react-native picks the fix up by regenerating its vendored copy of this file at build time — packages/react-native/scripts/sync-native-core.mjs copies packages/native/ios/Sources/AppductCore/Real into ios/Core/ (gitignored; the only tracked files there are the bridge's own two), and deleting ios/Core/ before pnpm build regenerates it with the fix.

Acceptance criteria

# Criterion Test Tier
1 An event frame an iOS app posts while active carries ts in Unix milliseconds, as docs/PROTOCOL.md §4 documents and Android sends AppductClientTests.testPostEventStampsTheEventWithUnixMillisecondsNotSeconds — an active client on FakeClientTimers(startMs: 1_752_600_000_000) posts an event, and the frame the fake transport received has ts == 1752600000000 unit (Swift)
2 A user can read what changed: the changelog names the command whose output shifts, and the unit is now stated in words on the surfaces that show this output CHANGELOG.md under Unreleased; docs/PROTOCOL.md §4 ("ts is Unix milliseconds"); website/src/content/docs/reference/cli.md, appduct events tail; skills/appduct/references/cli.md, same command device-free review

Criterion 2 is a doc/changelog surface, not a runtime assertion; criterion 1 was red before the fix (the frame carried 1752600000) and is green after. swift test at the repo root: 157 tests, 0 failures. pnpm build, pnpm lint, pnpm typecheck clean; pnpm test 923 passed / 1 skipped in appduct, 262 passed in @appduct/react-native; pnpm check:links clean.

Criterion 2's first evidence line was wrong when this PR was opened, and the review caught it: it cited website/src/content/docs/reference/protocol.md, which #152 deleted before this head, so no live page stated the unit at all — only the 13-digit literal in docs/PROTOCOL.md and a Swift doc comment did. #152 also moved docs/ out of the writing-user-docs skill's user-facing list, so §4 alone would not have closed it either. The unit is now written out in §4 for anyone implementing a client, and on the two surfaces a user or agent actually reads for this output: the website's events tail reference (its data.ts, in Unix milliseconds, distinguished from the daemon's receive time leading the line) and the same command in the shipped skill's CLI table. The changelog entry now names data.ts rather than ts, since that is the key it publishes.

Review follow-ups

  • Nit (round 1): no test asserted that SystemAppductClientTimers.now() itself returns milliseconds — every AppductClient test injects FakeClientTimers, whose startMs is a free parameter. AppductClientTimersTests.testTheRealClockReportsUnixMilliseconds now pins the production adapter, as the SPKI pin and lease store already pin theirs. Checked by mutation: dropping the * 1_000 fails it (1791298761 against 1791298761603), which the old test did not.

Why a Swift unit test and not a conformance fixture

packages/native/fixtures has no event-frame vectors today — event-registry-frames.json covers only event_registry_* — and its scenario shape has no way to pin a timestamp: the Kotlin core has no clock seam at all (System.currentTimeMillis() inline, no interface to fake), so a fixture asserting a fixed ts could not run in Kotlin, and a loose one (ts > 1e12) would re-assert a real wall clock rather than the unit. That directory's stated rule is for parsing/validation behaviour three implementations share; this is one SDK's stamping of a field, so the SDK's own test through the public API is the lowest tier that observes it.

E2E evidence

pending — left for the e2e-device skill run. Needs an iOS simulator: appduct sessions link --open ios-sim, then appduct events tail --json and tap the playground's event button; the line's data.ts should be ~1.75e12, matching Android.

Checklist

  • CHANGELOG.md has an entry under Unreleased (writing-changelog skill), or the change is not user-visible
  • User-facing docs updated for every surface the change touches (writing-user-docs skill), or the change is not user-visible
  • No new import past a module's index.ts; no new direct node:* I/O outside an adapter
  • Simplification checklist from the architecture skill applied, exceptions explained above
  • docs/ARCHITECTURE.md updated if a surface it describes changed

Out of scope

  • The notification-level ts (the leading time on a events tail line, and the flat ts appduct_events/appduct_wait_for_event/AppClient.waitForEvent return) is stamped from the daemon's clock and was never wrong; the issue described that value as showing 1970. Only the app-supplied data.ts was. If callers should see the app's clock rather than the daemon's, that is a separate design question. The two surfaces now say the two values are separate, since the names invite mixing them.
  • AppductClientTest.kt:808 asserts only that an event frame has a ts key, so nothing pins Android's unit either; worth a follow-up.
  • AppductClient+Session.swift and AppductConnectionManager.swift also divide timers.now() by 1 000, correctly — bootstrap expiresAt is Unix seconds. Left alone.
  • packages/appduct/src/mcp/events-tool.ts describes appduct_events' flat ts without a unit. Left alone: that value is the daemon's, its unit is EventNotification.ts's "Unix ms", and iOS events report a timestamp in seconds, not milliseconds #154 did not change it.

Status

Implement: pending Review: pending E2E: pending Ready: no

V3RON added 3 commits October 6, 2026 12:11
Fails on the current core: the frame carries 1752600000 where the protocol
says 1752600000000.

1 failing
timers.now() is already milliseconds, so the extra / 1_000 put every iOS
event 1000x in the past. The vendored @appduct/react-native copy regenerates
from this file at build time.

1 failing -> 0 failing
…report-a-timestamp-in-seconds

# Conflicts:
#	CHANGELOG.md

/// Emits an `event` frame while active (PROTOCOL.md §7); rejects otherwise.
/// Emits an `event` frame while active (PROTOCOL.md §4); rejects otherwise. `ts` is Unix
/// milliseconds, the same unit `timers.now()` reports and Kotlin's `System.currentTimeMillis()`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should-fix — the unit this fix establishes is stated in words nowhere a user reads, and criterion 2 was checked against a deleted file.

docs/PROTOCOL.md §4 documents ts only as the literal 1752600000000 — the guard sentence right below it explains the frame's plumbing but never says "milliseconds". The one sentence that ever did, "ts is in milliseconds", lived on website/src/content/docs/reference/protocol.md:182 — the second path criterion 2 cites. #152 deleted that file (2e587bd, an ancestor of this head), so it is not in this tree, the docs-site page 404s today, and llms.txt lists no protocol page under Reference. That leaves the unit carried by a 13-digit number and a Swift doc comment.

Concrete cost: the contract's own header says the docs are what a new client implementation reads, and #154 is what happens when one SDK guesses. The bug shipped from #49 through 0.14.0 with Android right and iOS wrong because nothing readable pinned the unit; after this PR, still nothing readable does.

Fix direction: state it in §4's event paragraph next to the guard sentence ("ts is Unix milliseconds") — one sentence, and it makes the shipped value and the contract agree in prose, not just by example. The PR body's criterion-2 evidence also needs correcting, since one of its two paths no longer exists.

@V3RON V3RON Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the deletion (git log --diff-filter=D -- website/src/content/docs/reference/protocol.md -> 2e587bd) and that no live page covers the unit: nothing under website/src/content/docs states a unit for an event's ts, and the only "millisecond" left in the website's prose is writing-tools' parameter advice. Agreed, so 93d2f07 says it in words in three places rather than one:

  • docs/PROTOCOL.md §4, next to the guard sentence, as you suggested — plus that the guard accepts any finite number, so a seconds stamp is not a validation error but a silent 1 000x-past timestamp. That is the part §4 was missing for the next client implementer, not just the unit.
  • website/src/content/docs/reference/cli.md, events tail: each line's data.ts is the app's stamp in Unix milliseconds, and the time leading the line is the daemon's receive time. I went to the website too because docs: shorten README, add banner and remove stale docs #152 moved docs/ out of the writing-user-docs skill's user-facing list, so §4 alone would have been the contributor contract saying what the user docs still didn't — the same gap that let this ship.
  • skills/appduct/references/cli.md, same command: an agent writing events tail --json is the reader most likely to parse the wrong ts, so the line spells out which of the two timestamps is the app's.

Also corrected: criterion 2's evidence line now cites these three, and the changelog says data.ts where it said ts, since that is the key it publishes.


let text = try XCTUnwrap(transport.sentMessages.first { $0.contains("\"type\":\"event\"") })
let frame = try JSONValue.parse(text)
XCTAssertEqual(frame.objectValue?["ts"]?.doubleValue, Double(nowMs))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — this pins the arithmetic, not the whole chain, so one link is still assumed: no test anywhere asserts that SystemAppductClientTimers.now() actually returns milliseconds (grep -rn SystemAppductClientTimers packages/native/ios/Tests is empty).

State that slips through: drop or double the * 1_000 in AppductClientTimers.swift:31. This test stays green — it injects FakeClientTimers, so it only proves postEvent forwards the port's number unscaled — and iOS is back to a 1970 data.ts plus grace/backoff windows 1000x off. That adapter also exists twice in the tree (packages/native/ios/.../Real/ plus the build-time vendored copy in packages/react-native/ios/Core/), so drift is a two-file affair rather than one.

Cheap close: one test in AppductClientTests (or a small AppductClientTimersTests) asserting SystemAppductClientTimers().now() is the same magnitude as Date().timeIntervalSince1970 * 1_000 — the repo already tests real adapters this way (SPKI pin, lease store). The tier choice itself looks right to me: I confirmed the fixture arguments — packages/native/fixtures has no event-frame vectors, and Kotlin has no clock seam to fake (System.currentTimeMillis() inline in all 13 sites, no Clock interface).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair — the fake made criterion 1 prove only forwarding. Added c621870: AppductClientTimersTests.testTheRealClockReportsUnixMilliseconds asserts SystemAppductClientTimers().now() equals Date().timeIntervalSince1970 * 1_000 within 1 s, which is the adapter's own definition rather than a magnitude band, so a seconds or microseconds clock is out by three orders and fails. Verified your mutation: * 1_000 dropped, the new test fails with 1791298761 against 1791298761603 and the postEvent test stays green, which is exactly the hole you described. Restored, 157 tests 0 failures.

No #if APPDUCT_ENABLED guard, following AppductAPITests, which covers the same Real/ directory ungarded: the define is a build setting on the AppductCore target only, and swift test builds that target in Debug, so the test is not silently skippable. Say the word if you would rather have it guarded.

On the second copy: it is not committed, so drift is a one-file affair. packages/react-native/scripts/sync-native-core.mjs (note: under packages/react-native/, not the root scripts/) replaces ios/Core/ wholesale from packages/native/ios/Sources/AppductCore/Real on that package's build and prepack; ios/Core/ is gitignored and git ls-files packages/react-native/ios returns only the bridge's two files, so Core/AppductClient.swift exists here solely because pnpm build regenerated it. I checked android/core and android/core-noop the same way, and git grep --no-index 'public actor AppductClient' finds only the source and that regenerated copy. Deleted ios/Core/ before the final pnpm build to confirm it comes back with the fix; the PR body now says this instead of just asserting it.

@V3RON V3RON left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment — 0 blockers, 1 should-fix, 1 nit. The fix is correct and complete: data.ts is now Unix milliseconds on iOS, matching Android and docs/PROTOCOL.md §4; I checked every other timers.now()/currentTimeMillis site in both native cores and the RN JS layer for the same unit bug and found none, and no daemon/CLI/MCP consumer of data.ts assumed seconds (ordering, cursors and retention all run off seq).

Spec: issue #154 plus the PR body's criteria table.

Fix first: the unit is now correct in code but stated in words nowhere a user reads — website/.../protocol.md:182, cited as criterion 2's evidence, was deleted by #152 before this head, so say "ts is Unix milliseconds" in docs/PROTOCOL.md §4 and correct that criterion.

V3RON added 2 commits October 6, 2026 19:37
Review nit: every AppductClient test injects FakeClientTimers, so dropping the
* 1_000 in SystemAppductClientTimers.now() left the suite green. Verified by
mutation: with the multiplier removed this test fails (1791298761 vs
1791298761603) and 156 -> 157 with 1 failing.

swift test: 157 tests, 0 failures.
Review should-fix: #152 deleted website/src/content/docs/reference/protocol.md,
which held the only sentence ever written about the unit. docs/PROTOCOL.md 4 now
states it, and the two surfaces a user or agent reads for this output do too:
the website's events tail reference and the shipped skill's CLI table.
@V3RON
V3RON marked this pull request as ready for review October 8, 2026 20:36
@V3RON
V3RON merged commit 21d9e3d into main Oct 8, 2026
10 checks passed
@V3RON
V3RON deleted the issue-154-ios-events-report-a-timestamp-in-seconds branch October 8, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS events report a timestamp in seconds, not milliseconds

1 participant