Skip to content

fix(deploy): keep gateway 5xx, repeated sightings and new deploys truthful (r2) - #107

Merged
sgardoll merged 1 commit into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/gate-deploy-truthfulness-r2
Sep 28, 2026
Merged

sgardoll merged 1 commit into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/gate-deploy-truthfulness-r2

Conversation

@sgardoll

Copy link
Copy Markdown
Owner

Supersedes #101 on the current head

Re-port of PR #101 (sgardoll/gate-deploy-truthfulness) onto the moved milestone head, because a parallel Devin branch landed on the milestone and changed the same region. The old branch is untouched; this is a fresh branch carrying only the deploy-truthfulness findings that are still missing.

Dropped finding (D1 / overlay hide-timer): Dr's cancelCommitProgressHide() already implements the deploy-progress overlay hide-timer fix on this head, so it is not re-added here.

Ported findings:

  1. 5xx → UNCONFIRMED — deployOutcomeOfStreamResult (src/provisionStream.js) now maps only 4xx → FAILED; a 5xx stays UNCONFIRMED because a gateway can answer 502/503/504 after forwarding the request to the runner.
  2. explainedByVerified ledger — resolveProjectPubspec (app.js) marks a sighting explained by a verified write as spent evidence, so repeated exports of unchanged content can no longer retire an uncertain write.
  3. Truthful "runner returned HTTP " message — provisionMissingCodeFiles names the received HTTP status in its UNCONFIRMED throw instead of claiming the connection dropped.

Full CI passes on this head.

…thful (r2)

Re-port of PR #101's deploy-truthfulness work onto the moved milestone head.
Drops the D1 overlay-timer finding (covered by Devin's cancelCommitProgressHide).

(1) deployOutcomeOfStreamResult now maps only 4xx -> FAILED; 5xx stays
    UNCONFIRMED since a gateway may fail after forwarding to the runner.
(2) resolveProjectPubspec gains an explainedByVerified ledger so a repeated
    export of unchanged content cannot retire an uncertain write.
(3) provisionMissingCodeFiles names the received HTTP status in its
    UNCONFIRMED throw instead of claiming the connection dropped.
@sgardoll
sgardoll marked this pull request as ready for review September 28, 2026 15:35
@greptile-apps

greptile-apps Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

This PR does not match any of the 1 configured review trigger rule.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Devin Review

Comment thread app.js
Comment on lines +3111 to +3112
} else if (!record.explainedByVerified) {
record.uncertain -= 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Repeated exports clear pending class writes

With two uncertain writes of identical content, resolveProjectPubspec retires one on each unchanged export. The second write can land later, after its dirty entry has cleared and the stale snapshot was cached.

Learn more

The dirty ledger tracks uncertain provisioning attempts by file path and content. An export showing that content establishes at most one observable transition; repeated exports of the same snapshot do not establish that another pending attempt wrote it. The explainedByVerified flag only spends sightings attributed to verified writes. When no verified credit exists, repeated sightings can each decrement uncertain, emptying the ledger and enabling caching while a runner can still write.

Example: Two requests for class X time out. The first writes X; two consecutive exports both show X. Both uncertain credits disappear, and an export is cached. The second request then writes X over a newer edit, while the cache still describes the earlier project state.

Recommended fix: Track the last observed content or distinct landing evidence per path, including sightings consumed by uncertain writes. Never retire another uncertain credit on an unchanged export; preserve uncertainty where identical writes cannot be distinguished.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread app.js
Comment on lines +3110 to +3112
record.explainedByVerified = true;
} else if (!record.explainedByVerified) {
record.uncertain -= 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Returned class content keeps exports uncached

After a verified X sighting, explainedByVerified stays true when an export shows another version. If a pending write then restores X, its new sighting cannot retire the dirty entry, so every deploy re-exports.

Learn more

The project source cache is bypassed while any class write has an outstanding dirty entry. A verified write of X sets explainedByVerified on the X record, but observations of any other content do not reset it. If another edit replaces X with Y and the pending X write later restores X, the ledger rejects that distinct X sighting. It can remain dirty for the rest of the session, forcing a complete project export on each deploy.

Example: A timed-out provision of X is followed by a confirmed retry of X. An export sees X and consumes the confirmed credit. A later sync changes the class to Y, then the original provision writes X. Exports now show X again, but the pending X credit never clears.

Recommended fix: Track the last observed content per path and allow a new transition back to X to supply fresh evidence. Do not reset the evidence solely because time passes or the same snapshot is exported again.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@sgardoll
sgardoll merged commit f3dc176 into milestone/stu-implement-updated-ui-ux Sep 28, 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.

1 participant