fix(cli): timeline reports real media durations instead of unauthored

[miguel-heygen]

What changes

hyperframes timeline printed duration=unauthored for a video, audio or image row that has a data-start but no data-duration, so an agent could not tell how long the clip plays. The command now reports the real resolved length.

• Video and audio rows are probed with the same ffprobe path the producer already uses. The length, playback start and rate are applied by the shared resolver from feat(core): video, audio and image length come from one shared resolver #4166 (resolveMediaDuration), so this PR carries no duration arithmetic of its own.
• A timed image with no data-duration reports the resolver's default.
• When the probe cannot answer (file missing, remote source, ffprobe error or timeout) the row prints pending: <reason> and the JSON carries durationSource: "pending" with a pendingReason. It never guesses a number.
• Authored durations are unchanged, and a composition host keeps the summed length of its children, labelled inferred.
• Probes stay inside the project folder, at most four run at once, and each has a deadline.
• describeProject is now async, and its one caller awaits it. The skill reference and manifest describe the new fields.

This PR is stacked on #4166 (base branch feat/media-duration-resolver) and retargets to main once that merges. It removes the describeRow entry from the resolver's list of readers not yet converted.

Verification

• Timeline tests: 30 pass, including real ffprobe on a small audio fixture, the resolver's shared fixtures that need no file, remote, absolute and .. sources, and a concurrency test where jobs arrive while slots are held.
• Deliberate mutations each turn one of those tests red: probe result not passed to the resolver, playback rate dropped, remote check removed, probe reason dropped, and the gate releasing a slot before waking the next waiter.
• Typecheck, lint and format checks pass, as do the skill lint and mirror checks.
• Not exercised: the repo's dead-code and duplication scan on the final head (it needs git history that the test machine lacks), and Windows.

Review

An independent review found no blocking issues. Its should-fix items are closed: the misleading inferred label on leaves, the probe gate's slot handoff and its test, the reason text and tests for remote, absolute and .. sources, and a second copy of the "authored duration" rule, which now lives only in the resolver. One note: the engine's authored-duration reader (which understands references like a-roll + 1) is passed to the resolver instead of the parsers reader; the two agree on every literal input.

[jrusso1020]

First review on this PR. The shape is right and the absolute-time tests are the best part of it — the two eval-miss regressions (a/b sub-second overlap, and ordering videos across different sub-compositions) pin behaviour that a reader could not previously get out of this command at all.

One blocking item, in added code, one line. Everything else below is non-blocking.

Verified rather than taken from the body

• "no duration arithmetic of its own" — holds for media. The only arithmetic left in describeProject.ts is resolveContainerDuration's children.reduce(Math.max), which is the pre-existing authored ?? inner moved intact, not new.
• "at most four run at once" — createProbeGate is correct. Slots are handed off rather than released (the waker does not decrement, the woken caller does not increment), so active is conserved and can only rise while active < max. The concurrency test discriminates: it asserts peak === 4 and adds a second wave of jobs while slots are held, so both an over-wide gate and a deadlocked one fail it.
• "each has a deadline" — true, via runFfprobe's deadlineAtMs: Date.now() + 30_000, not in this diff. Worth knowing that 4 × 30 s is the worst case for one timeline call on a project full of unprobeable files.
• "describeProject is now async, and its one caller awaits it" — true; packages/cli/src/commands/timeline.ts is the only non-test caller repo-wide and it awaits.
• The reader mismatch you self-flagged is the safe direction. resolveReferencedDuration (engine) returns null unless data-duration > 0, or data-end - start > 0; readAuthoredDurationSeconds (parsers) returns end - start with no positivity check. So the parsers reader can return a non-positive where the engine one returns null — and resolveMediaDuration re-checks > 0 anyway, so both collapse to the same branch. The consequence worth having: durationAuthored and durationSource: "authored" cannot disagree, which is not obvious from the two call sites.

Blocking — a probe that succeeds with no usable duration is reported as a measurement

extractMediaMetadata and extractAudioMetadata do not throw when the container has no duration. Both end with

const containerDuration = output?.format.duration ? parseFloat(output.format.duration) : 0;

and extractMediaMetadata has an explicit still-image branch that returns durationSeconds: 0 outright. That is not incidental — packages/engine/src/utils/ffprobe.test.ts:636 pins it: expect(meta.durationSeconds).toBe(0) on a resolved call.

So for a <video>/<audio> row pointing at a still image, or at any container ffprobe opens whose format.duration is absent:

1. probeSource returns { ok: true, seconds: 0 } — no throw, nothing to catch.
2. resolveMediaDuration({ sourceDurationSeconds: 0, ... }) skips the === null pending branch, and resolveNaturalDurationSeconds(0, 0, 1) is finite, so it returns { seconds: 0, source: "media" }.
3. The row lands as duration: 0, durationSource: "media", pendingReason: null, and prints duration=media.

The number is unchanged from main — main printed 0-0s here too. What is new is the claim about where it came from: main said duration=unauthored, i.e. "nobody authored this and I do not know"; this PR says ffprobe measured it and the answer is zero. That is the one distinction the command now exists to make, and it is the branch that gets it wrong. An agent reading duration=media has been told the clip is genuinely zero-length and can act on that — trim it, drop it, schedule around it — where pending: <reason> would have sent it to look at the file.

It also contradicts the PR's own headline: "It never guesses a number." It does not guess, but it reports 0 with the confidence reserved for a real measurement. And the test this PR removes was named "marks a clip without an authored duration instead of reporting 0 as a fact" — its subject was the img, so replacing it for images is correct, but the principle in that name is exactly what the probe path now breaks for video and audio.

One line, in resolveMediaRowDuration:

sourceDurationSeconds: probe.ok && probe.seconds > 0 ? probe.seconds : null,

A non-positive probe result then routes to the pending branch it belongs in. It wants a reason to go with it — see the next item, which the same edit closes.

Non-blocking

1. The resolver's own reason is discarded. resolveMediaDuration returns reason: "source reported a non-finite duration" on the non-finite path, and the caller never reads result.reason — it substitutes the probe's reason, which does not exist when probe.ok is true:

   pendingReason: result.source === "pending" ? (probe.ok ? null : probe.reason) : null

   Any probe that succeeds with a non-finite duration therefore yields durationSource: "pending", pendingReason: null, and formatTimeline prints the literal pending: null. That also contradicts the JSON contract this PR adds in the same diff ("pendingReason … null unless durationSource is "pending""). I have not found an input that makes ffprobe emit a non-numeric duration into the JSON writer, so I am treating this as latent rather than reachable — but probe.ok ? (result.reason ?? null) : probe.reason costs nothing and makes the blocking fix above carry a reason instead of a null.

2. No symlink case for a media src. probeSource goes through the same realFileInside fence as sub-compositions, and remote / absolute / .. are all covered — but the symlink-escape test only exists on the sub-composition path. Same helper, so this is a coverage gap rather than a hole.

3. runOneLiner leaks temp dirs. It is now async and called four-up through Promise.all, and each call runs inversionProject(), which reassigns the module-level dir. afterEach removes only the last one, so three temp dirs survive each run of that test.

4. The independent review is not on the PR, so I have not counted it as coverage — everything above is derived from the diff and the files it touches.

— Rames

[jrusso1020]

APPROVE at 4bdd83910299c04258a56da90751585210f7f253. Re-review of my CHANGES_REQUESTED 5255696455 at 0a484588. All four items are resolved, and the blocker is fixed at the root rather than at the call site.

The blocker — fixed where I asked

const measured = probe.ok && probe.seconds > 0 ? probe.seconds : null;

This is the right guard, and it covers more than the case I named. 0, a negative, and NaN all fail > 0, so every shape of "the probe opened the file but cannot answer" now reaches resolveMediaDuration as null and comes back source: "pending". That closes the still-image path through extractMediaMetadata (durationSeconds: 0, no throw) that made the original finding reachable.

The regression test is on the right axis

expect(rows[0]).toMatchObject({
  durationSource: "pending",
  pendingReason: "source reports no duration",
  duration: 0,
});
expect(text).toContain("pending: source reports no duration");

This is the part worth calling out. duration: 0 is unchanged between the buggy and fixed code — a test that asserted only the number would pass either way, which is exactly why the defect survived the first pass. Asserting durationSource and the rendered text puts the test on the attribution, which is the only axis that moves. Against the old code it reads durationSource: "media" and pendingReason: null, so it fails red. The test can actually fail, which is what makes it a regression test rather than a description.

Using a real 1×1 PNG behind <video src="still.png"> also drives the actual ffprobe still-image branch instead of a mock, so it pins the engine behaviour I cited rather than a restatement of the fix.

The other three

• result.reason discarded (I graded this LATENT) — now read through pendingReason(probe, resolverReason), with ?? "source duration unavailable" so a literal pending: null can no longer print. ProbeResult is a proper discriminated union ({ok: true; seconds} | {ok: false; reason}), so probe.seconds after the !probe.ok early return narrows correctly.
• No symlink test on the media src — added, and it pins the real guard: realFileInside returning null at describeProject.ts:147 yields "source file not found", which is the string the test asserts. The guard and the test agree at source.
• runOneLiner leaked temp dirs — now try/finally with rmSync(dirname(own), …).

Scope

Delta since my review is 2 files, +62/-14, one commit. Nothing else moved, so the four verified-clean items from the first pass still stand and I have not re-derived them: createProbeGate slot handoff, the 30s runFfprobe deadline, the single awaited describeProject caller, and resolveReferencedDuration's > 0 guard making durationAuthored / durationSource: "authored" unable to disagree.

Nothing further from me.

— Rames

[jrusso1020]

Re-reviewed at dcd0ad59. The push since my approval at 4bdd8391 is one commit, two files. Approving again — the production change is a clean behaviour-preserving extraction, and the guard test I'd flagged is intact. One should-fix on the new skip guard.

The production change is sound

measureWithFfprobe is a verbatim move of the code that was inline in probeSource, and the seam is wired correctly rather than just added:

• measure is non-optional on DocScope, so every construction site must supply it or the build breaks. Both do — describeProject passes the parameter, readSubComposition propagates parent.measure. That last one matters: had it defaulted to ffprobe instead of inheriting, nested media would have silently hit the real binary while the top level used the fake.
• The call stays inside both withProbeSlot and the existing try/catch, so the concurrency bound and the throw→{ok:false, reason} mapping are unchanged.
• measure: MeasureMedia = measureWithFfprobe keeps every existing caller compiling and behaving identically.

fileURLToPath for REAL_AUDIO is the right fix, not a patch over the symptom — .pathname yields /C:/… on Windows, and it also leaves percent-escapes undecoded on every platform.

The rename from "probes a media leaf's real duration" to "takes a media leaf's duration from the probe" is the honest one now that it doesn't, and expect(probed).toEqual(["audio:sting.mp3"]) pins the tag routing through the seam, which the old real-ffprobe test never checked. Both are net gains.

Should-fix: the skip guard probes a different thing than the code it guards

describeProject.test.ts:23

const hasFfprobe = spawnSync("ffprobe", ["-version"]).status === 0;

That is a bare PATH lookup. Production doesn't resolve ffprobe that way — measureWithFfprobe → extractAudioMetadata/extractMediaMetadata → getFfprobeBinary() → findFfBinary("ffprobe") (packages/parsers/src/ffBinaries.ts), which checks HYPERFRAMES_FFPROBE_PATH first, then scans PATH, then falls back to the bare name.

So the two disagree in exactly one direction: with HYPERFRAMES_FFPROBE_PATH set and no ffprobe on PATH, hasFfprobe is false while the code under test would probe fine — and it.skipIf(!hasFfprobe) at :284 silently skips the only test that exercises the default wiring. It's the one assertion standing between an audio/video extractor swap and a green suite, so it's worth making it fire whenever it can.

One line, using the resolver the package already depends on:

import { findFfBinary } from "@hyperframes/parsers/ff-binaries";
const hasFfprobe = !!findFfBinary("ffprobe");

Worth separating this from the it.skipIf(!hasJq) precedent at :547 it resembles — jq ships on ubuntu-latest, so that guard actually runs in CI. This one is modelled on it but isn't equivalent.

Related, and the reason I'd fix the detector rather than the guard

:194 reports a still image used as a video source as pending, not a measured zero — the test that closed my original blocker — passes no fake and asserts the exact string pendingReason: "source reports no duration". That string only appears when a probe ran and reported nothing; an unreachable binary throws and lands in the catch, giving "[FFmpeg] ffprobe not found. Please install FFmpeg." instead.

So that test requires a working ffprobe, un-faked and un-skipped, which sits awkwardly against the commit message's premise that the Ubuntu Test job lacks one. Either the binary is reachable there — in which case hasFfprobe is under-reporting and :284 is skipping for no reason — or it isn't, and :194 is the next thing to go red. Both readings resolve the same way: make the detector match the resolver, then decide deliberately whether :194 should take the fake too. Nothing in the suite currently distinguishes "probed and got nothing" from "couldn't probe", which is what lets the ambiguity sit.

Not blocking — the shipped behaviour is unchanged by this push and the coverage is strictly better than the real-ffprobe version it replaced.

— Rames