Skip to content

sdk/lifecycle-completion: resolveCompletionTimestamp's declared types contradict its runtime behaviour on both the input and the return, and it credits updated_at for a value it never supplied #808

Closed

Description

@unbraind

Verified on @unbrained/pm-cli 2026.7.29 (/sdk root export, plain node -e, no bundler involved).

resolveCompletionTimestamp (GH-675) is exactly the right primitive to export — we adopted it in pm-changelog so the changelog generator and the tracker agree on what counts as completion time. But its published types force an unsound cast on any TypeScript consumer, and one of its return values is actively wrong.

1. The return type says timestamp: string, but the property is absent

// dist/sdk/lifecycle-completion.d.ts
export interface ResolvedCompletionTimestamp {
    /** ISO timestamp selected for reporting. */
    timestamp: string;          // <- not optional
    source: CompletionTimestampSource;
    fallback: boolean;
}
$ node -e 'const {resolveCompletionTimestamp}=require("@unbrained/pm-cli/sdk");
           const r=resolveCompletionTimestamp({});
           console.log(JSON.stringify(r), typeof r.timestamp)'
{"source":"updated_at","fallback":true} undefined

The property is not merely undefined, it is missing from the object. A consumer that trusts the declaration writes resolved.timestamp.slice(0, 10) and gets a TypeError that the compiler promised could not happen.

2. source reports updated_at when updated_at supplied nothing

The same call returns source: "updated_at" for an item that has no updated_at at all. Since CompletionTimestampSource is documented as "Metadata field that supplied the timestamp", this credits a field that supplied nothing — and because fallback: true accompanies it, a consumer aggregating provenance counts (which is the stated purpose of exposing source) silently attributes phantom items to updated_at.

This matters more than it looks: a reporting consumer cannot distinguish "placed by a real updated_at" from "no completion signal exists at all". We hit precisely this — our reviewer caught our own code inheriting the mislabel and we had to add a distinct local "none" outcome to keep the counts honest.

Suggested shape: either timestamp?: string with source omitted when nothing was supplied, or a discriminated result ({ resolved: true, timestamp, source, fallback } | { resolved: false }). Either makes the empty case unrepresentable-by-accident.

3. The input type forbids the case the function exists to handle

export declare function resolveCompletionTimestamp(
  item: Pick<ItemMetadata, "completed_at" | "closed_at" | "updated_at">
): ResolvedCompletionTimestamp;

ItemMetadata declares completed_at?: string and closed_at?: string, but updated_at: string (required). So the signature refuses an item lacking updated_at — yet handling absent fallback fields is the function's entire job, and the runtime clearly accepts {}.

A consumer whose own item type models all three as optional (correct for legacy records that predate the lifecycle fields) therefore cannot call this function without an assertion. The natural workaround is the worst one:

const resolveSdkCompletion = resolveCompletionTimestamp as unknown as (
  item: { completed_at?: string; closed_at?: string; updated_at?: string }
) => { timestamp?: string; source: CompletionTimestampSource; fallback: boolean };

That double-cast is what our reviewer flagged, correctly: it masks the declared return type, so a future SDK change to this path would not fail our compile. We removed it by guarding on updated_at !== undefined and replicating the two-field precedence locally when it is absent — which is a workaround we would rather not carry, since the point of adopting the SDK primitive was to stop maintaining our own copy of that precedence.

Expected

Pick<ItemMetadata, ...> widened for this signature (e.g. Partial<Pick<ItemMetadata, "completed_at" | "closed_at" | "updated_at">>), and a return type that can represent "nothing supplied a timestamp" without lying about either timestamp or source. Then the primitive is adoptable with zero assertions, which is the whole value of shipping it in the public SDK.

Activity

  1. coderabbitai commented on Jul 29, 2026

    @coderabbitai
    🔗 Related PRs

    #477 - feat(sdk): expand pm primitives and lifecycle aliases [merged]
    #656 - feat(sdk): temporal digital-twin context engine [merged]
    #673 - feat(sdk): harden workspace mutation and migration primitives [merged]
    #753 - feat(sdk): universal context, concurrency, and discovery primitives [merged]
    #801 - SDK lifecycle provenance and context-rich CLI primitives [open]


    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  2. unbraind commented on Jul 29, 2026

    @unbraind
    OwnerAuthor

    pm tracker item: pm-qhnq6t

    Tracked as a distinct public SDK contract/provenance regression discovered from the completed GH-675 lifecycle delivery, with runtime, declarations, docs, public-surface, and packed-consumer acceptance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions