Skip to content

fix: merge duplicate build log steps before reading output - #256

Merged
Fermionic-Lyu merged 2 commits into
mainfrom
codex/fix-depot-log-metadata
Sep 18, 2026
Merged

Fermionic-Lyu merged 2 commits into
mainfrom
codex/fix-depot-log-metadata

Conversation

@Fermionic-Lyu

@Fermionic-Lyu Fermionic-Lyu commented Sep 18, 2026

Copy link
Copy Markdown
Member

A build-log response can repeat a step digest, including a stale record after completion. Merge steps across all metadata pages before reading output, preserving the latest completion and failure details plus log availability. This prevents duplicate output requests while leaving legitimate repeated log lines intact.

Completion ordering retains UTC RFC3339 nanosecond precision. A later successful completion clears an older failure.

Validation: typecheck and build passed; 1,606 tests passed with the ambient local login isolated, and the six home-directory tests passed separately with the normal home. Added order/pagination/precision regressions and mutation-checked them. Offline replay of the retained production failure reduced two output reads to one, preserving all 119 records and all output markers. Independent review passed.

@agent-zhang-beihai agent-zhang-beihai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Yang Dong

This consolidates paginated duplicate steps by digest, preserving logs and the newest terminal metadata before reading output. The implementation matches the producer contract; I found no findings and approve.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/build-logs.ts Outdated

@agent-zhang-beihai agent-zhang-beihai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Wang Miao

This collapses build-log step records that share a digest across paginated step pages into one entry, keeping the record with the later completedAt and OR-ing hasLogs, so a duplicated digest no longer triggers a second fetch of the same step's output. I traced the producer chain — compute/internal/build/logs.go emits completedAt via t.UTC().Format(time.RFC3339Nano), platform passes it straight through and its response schema keeps the field — and the merge, the recency rule, and the trailing-zero-trimmed fraction handling all hold up. Approve.

No findings.

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - approved.

@Fermionic-Lyu
Fermionic-Lyu merged commit 2120501 into main Sep 18, 2026
3 checks passed
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.

2 participants