Skip to content

docs: demote cargo stdout to hint; registry is authoritative (#99 follow-on)#117

Merged
EffortlessSteven merged 1 commit into
mainfrom
feat/99-stdout-as-hint-docs
Apr 17, 2026
Merged

docs: demote cargo stdout to hint; registry is authoritative (#99 follow-on)#117
EffortlessSteven merged 1 commit into
mainfrom
feat/99-stdout-as-hint-docs

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Member

Closes one of the remaining #99 follow-ons: make it explicit that cargo's stdout/stderr is a hint for Shipper's retry classification, not the authoritative answer.

What this changes

  • `ErrorClass` rustdoc (shipper-types) — new "Classification is a hint, not truth" section. Explains that pattern-matching cargo text is a fast-path signal, and that `Ambiguous` outcomes MUST be reconciled against registry truth via `ReconciliationOutcome`. Never blind-retry.
  • `classify_cargo_failure` rustdoc (shipper runtime::execution) — states up-front "This is a hint, not authoritative truth" and points readers at the reconcile flow.
  • `docs/explanation/why-shipper.md` — new section "Cargo stdout is a hint; the registry is the truth" connecting the contract to why Shipper can be safer than a naive `cargo publish` shell loop.

Why this matters

After #111 (in-flight reconcile) and #115 (resume-path reconcile) landed, the reconciliation flow is authoritative in code. This PR makes it authoritative in the documented contract — so contributors adding new patterns to cargo-failure classification know where the hint ends and truth begins.

Verification

  • `cargo doc --no-deps -Dwarnings` clean
  • `cargo test -p shipper-types` — 268 pass
  • `cargo test -p shipper --lib runtime::execution` — 134 pass
  • No code changes, no schema changes, no snapshot churn

Files

File Change
`crates/shipper-types/src/lib.rs` +`ErrorClass` rustdoc "hint vs truth" section
`crates/shipper/src/runtime/execution/mod.rs` +`classify_cargo_failure` rustdoc hint disclaimer
`docs/explanation/why-shipper.md` +section explaining the hint/truth contract at product level

Net: 3 files, +46 / -2.

#99 scorecard after this merge

Related

…low-on)

Closes one of the remaining #99 follow-ons identified in the
retrospectives: make it explicit, in the rustdoc that downstream tooling
and contributors will actually read, that cargo's stdout/stderr output is
a classification HINT for Shipper's retry loop — not the authoritative
answer about what happened.

## What this changes

- `ErrorClass` rustdoc (shipper-types): adds a "Classification is a hint,
  not truth" section explaining that pattern-matching cargo text drives
  retry scheduling but ambiguous outcomes MUST be reconciled against
  registry truth. Cross-refs ReconciliationOutcome and the reconcile
  module.
- `classify_cargo_failure` rustdoc (shipper::runtime::execution): states
  up-front "This is a hint, not authoritative truth" and points at the
  reconcile flow for the authoritative resolution on Ambiguous outcomes.
- `docs/explanation/why-shipper.md` gains a new section "Cargo stdout is
  a hint; the registry is the truth" making the same point at the product
  level — and connecting it to why Shipper can be safer than a naive
  shell-script `cargo publish` loop.

## Why this matters

Contributors reading `ErrorClass` and wondering "can I just pattern-match
more cargo strings?" now see the contract: cargo text is the fast-path
hint, registry queries are truth. The reconcile flow introduced in #111
(in-flight) and #115 (resume-path) is now documented as the authoritative
side of that contract, not just implemented.

## Scope

Pure docs/rustdoc change. No code changes. No schema changes. No
snapshot churn. Covered by existing tests.

- `cargo doc --no-deps -Dwarnings` clean
- `cargo test -p shipper-types` + `-p shipper --lib runtime::execution`
  pass (no regressions)

## Related

- Parent: #99 Reconcile
- Prior PRs: #111 (in-flight reconcile), #115 (resume-path reconcile)
- Pillar: Reconcile (#102)
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@EffortlessSteven has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 54 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 54 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e9c9badb-6738-4f84-8749-fc8286f6a689

📥 Commits

Reviewing files that changed from the base of the PR and between 2b419a7 and 1b6c260.

📒 Files selected for processing (3)
  • crates/shipper-types/src/lib.rs
  • crates/shipper/src/runtime/execution/mod.rs
  • docs/explanation/why-shipper.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/99-stdout-as-hint-docs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request adds documentation to clarify that cargo's output is treated as a non-authoritative hint, while the registry remains the source of truth for reconciliation. The changes include updates to the ErrorClass enum documentation, the classify_cargo_failure function, and the project's high-level explanation. One review comment suggests improving documentation links and using idiomatic crate-relative paths for internal references.

Comment on lines +68 to +69
/// flow — never from the cargo text alone. See the `ErrorClass` rustdoc
/// and `shipper::engine::parallel::reconcile` for the "hint vs truth"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using a link for ErrorClass improves navigation. Also, using crate:: for internal module references is more idiomatic and ensures the path resolves correctly within the crate's documentation.

Suggested change
/// flow — never from the cargo text alone. See the `ErrorClass` rustdoc
/// and `shipper::engine::parallel::reconcile` for the "hint vs truth"
/// flow — never from the cargo text alone. See the [`ErrorClass`] rustdoc
/// and `crate::engine::parallel::reconcile` for the "hint vs truth"

@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@EffortlessSteven EffortlessSteven merged commit 0a6aa24 into main Apr 17, 2026
21 checks passed
@EffortlessSteven EffortlessSteven deleted the feat/99-stdout-as-hint-docs branch April 17, 2026 08:51
EffortlessSteven added a commit that referenced this pull request Apr 17, 2026
… + state-files cheat sheet

Closes the "scary-but-correct UX" papercuts flagged across multiple
external retrospectives. Three new docs covering operator-legibility
gaps where Shipper's behavior is correct but the operator's mental
model wasn't supported.

## New docs (Diátaxis)

- **`docs/explanation/finishability.md`** — why `finishability = not_proven`
  is the correct answer on first publish, not danger. Maps each case
  (first publish, token lacks ownership, network flake) to concrete
  operator action. Notes the future rehearsal-registry path (#97) that
  will promote more NotProven cases to Proven.

- **`docs/how-to/inspect-a-stalled-run.md`** — live triage. "Is the train
  alive or hung?" 30-second check using events.jsonl tail. Maps common
  questions (current crate, how long waiting, what will resume do, why
  did it fail) to the authoritative file + jq recipe. Distinguished from
  the existing `inspect-state-and-receipts.md` which covers post-hoc
  "what happened."

- **`docs/reference/state-files.md`** — one-page cheat sheet. Authority
  order (events > state > receipt), per-file purpose table, key field
  paths (including the `.packages[].state.state` nesting caveat —
  common misread), jq one-liners for the most frequent queries, sidecar
  files reference.

## Navigation updates

- `docs/README.md` (Diátaxis index): add all three new entries under
  their correct quadrants
- Root `README.md`: extend the quick-links strip to include stalled-run
  triage, state-files cheat sheet, and the not_proven explainer

## Scope

Pure docs. No code, no schema, no snapshot churn. ~450 lines of new
content in 3 files + 2 index updates.

## Why these three specifically

Three external retrospectives converged on the same "scary-but-correct"
list:
- `not_proven` reads alarming but is epistemically honest for first-publish
- Operators don't always know which file answers which question
- There's no live-triage guide for "is it still alive?" vs "did it stall?"

This pack closes all three in a single focused PR.

## Related

- #103 Narrate umbrella (operator legibility is the theme; see scout comment)
- #99 follow-ons — complements #117 (cargo-stdout-as-hint docs) merged earlier this session
- #93 events-as-truth (the INVARIANTS the cheat sheet references)
EffortlessSteven added a commit that referenced this pull request Apr 17, 2026
… + state-files cheat sheet (#118)

Closes the "scary-but-correct UX" papercuts flagged across multiple
external retrospectives. Three new docs covering operator-legibility
gaps where Shipper's behavior is correct but the operator's mental
model wasn't supported.

## New docs (Diátaxis)

- **`docs/explanation/finishability.md`** — why `finishability = not_proven`
  is the correct answer on first publish, not danger. Maps each case
  (first publish, token lacks ownership, network flake) to concrete
  operator action. Notes the future rehearsal-registry path (#97) that
  will promote more NotProven cases to Proven.

- **`docs/how-to/inspect-a-stalled-run.md`** — live triage. "Is the train
  alive or hung?" 30-second check using events.jsonl tail. Maps common
  questions (current crate, how long waiting, what will resume do, why
  did it fail) to the authoritative file + jq recipe. Distinguished from
  the existing `inspect-state-and-receipts.md` which covers post-hoc
  "what happened."

- **`docs/reference/state-files.md`** — one-page cheat sheet. Authority
  order (events > state > receipt), per-file purpose table, key field
  paths (including the `.packages[].state.state` nesting caveat —
  common misread), jq one-liners for the most frequent queries, sidecar
  files reference.

## Navigation updates

- `docs/README.md` (Diátaxis index): add all three new entries under
  their correct quadrants
- Root `README.md`: extend the quick-links strip to include stalled-run
  triage, state-files cheat sheet, and the not_proven explainer

## Scope

Pure docs. No code, no schema, no snapshot churn. ~450 lines of new
content in 3 files + 2 index updates.

## Why these three specifically

Three external retrospectives converged on the same "scary-but-correct"
list:
- `not_proven` reads alarming but is epistemically honest for first-publish
- Operators don't always know which file answers which question
- There's no live-triage guide for "is it still alive?" vs "did it stall?"

This pack closes all three in a single focused PR.

## Related

- #103 Narrate umbrella (operator legibility is the theme; see scout comment)
- #99 follow-ons — complements #117 (cargo-stdout-as-hint docs) merged earlier this session
- #93 events-as-truth (the INVARIANTS the cheat sheet references)
EffortlessSteven added a commit that referenced this pull request Apr 17, 2026
…omes (#99 follow-on)

Closes the last remaining #99 follow-on: end-to-end integration coverage
for the three reconciliation outcomes the state machine can produce.

## Three scenarios added to engine::parallel::tests

Each exercises publish_package() against a real tiny_http mock registry
with a fake cargo binary configured to exit ambiguously (exit 1, empty
stdout/stderr — which classify_publish_failure maps to
CargoFailureClass::Ambiguous per existing unit tests).

Readiness polling is disabled (readiness.enabled = false) so each
reconcile site reduces to a single registry query — avoids flaky request
counts that a full polling loop would produce.

### 1. reconcile_bdd_ambiguous_resolves_to_published

- cargo fails ambiguously
- entry + post-cargo quick checks return 404
- reconcile's single version_exists poll returns 200
- expected: PackageState::Published, attempts=1 (no second cargo)
- verifies PublishReconciling + PublishReconciled{Published} events
  emitted to the on-disk event log

### 2. reconcile_bdd_ambiguous_resolves_to_not_published_then_retries

- cargo fails ambiguously on every attempt
- registry consistently returns 404
- reconcile resolves to NotPublished, falls through to normal retry
- after max_attempts=2, package ends Failed
- verifies PublishReconciled{NotPublished} event emitted
- verifies cargo was actually retried (attempts=2)

### 3. reconcile_bdd_resume_from_ambiguous_state_skips_republish

- state pre-seeded with PackageState::Ambiguous (simulating a prior
  interrupted run)
- entry check returns 404 (bypasses "already published" short-circuit)
- resume-path reconcile (from PR #115) fires BEFORE retry loop
- single version_exists query returns 200 → Published
- expected: attempts=0 (cargo NOT invoked)
- verifies via SHIPPER_CARGO_ARGS_LOG that cargo binary was never called
- verifies PublishReconciling + PublishReconciled{Published} events emitted

## Verification

- cargo test -p shipper --lib -- reconcile_bdd: 3 pass
- cargo test -p shipper --lib -- engine::parallel: 70 pass (no regressions)
- cargo clippy --all-targets -- -D warnings: clean
- cargo fmt --check: clean

## Why events are read from disk

publish_package flushes event batches to events.jsonl and clears the
in-memory log after each write. Assertions read via
EventLog::read_from_file(&events_path) to see the full persisted stream.

## #99 tracker after this merge

- [x] In-flight reconcile state machine (#111)
- [x] Resume-path reconciliation (#115)
- [x] Docs/contracts: cargo text is a hint, registry is truth (#117)
- [x] BDD integration scenarios for all 3 outcomes (this PR)

#99 is now fully closed.
EffortlessSteven added a commit that referenced this pull request Apr 17, 2026
…omes (#99 follow-on) (#119)

Closes the last remaining #99 follow-on: end-to-end integration coverage
for the three reconciliation outcomes the state machine can produce.

## Three scenarios added to engine::parallel::tests

Each exercises publish_package() against a real tiny_http mock registry
with a fake cargo binary configured to exit ambiguously (exit 1, empty
stdout/stderr — which classify_publish_failure maps to
CargoFailureClass::Ambiguous per existing unit tests).

Readiness polling is disabled (readiness.enabled = false) so each
reconcile site reduces to a single registry query — avoids flaky request
counts that a full polling loop would produce.

### 1. reconcile_bdd_ambiguous_resolves_to_published

- cargo fails ambiguously
- entry + post-cargo quick checks return 404
- reconcile's single version_exists poll returns 200
- expected: PackageState::Published, attempts=1 (no second cargo)
- verifies PublishReconciling + PublishReconciled{Published} events
  emitted to the on-disk event log

### 2. reconcile_bdd_ambiguous_resolves_to_not_published_then_retries

- cargo fails ambiguously on every attempt
- registry consistently returns 404
- reconcile resolves to NotPublished, falls through to normal retry
- after max_attempts=2, package ends Failed
- verifies PublishReconciled{NotPublished} event emitted
- verifies cargo was actually retried (attempts=2)

### 3. reconcile_bdd_resume_from_ambiguous_state_skips_republish

- state pre-seeded with PackageState::Ambiguous (simulating a prior
  interrupted run)
- entry check returns 404 (bypasses "already published" short-circuit)
- resume-path reconcile (from PR #115) fires BEFORE retry loop
- single version_exists query returns 200 → Published
- expected: attempts=0 (cargo NOT invoked)
- verifies via SHIPPER_CARGO_ARGS_LOG that cargo binary was never called
- verifies PublishReconciling + PublishReconciled{Published} events emitted

## Verification

- cargo test -p shipper --lib -- reconcile_bdd: 3 pass
- cargo test -p shipper --lib -- engine::parallel: 70 pass (no regressions)
- cargo clippy --all-targets -- -D warnings: clean
- cargo fmt --check: clean

## Why events are read from disk

publish_package flushes event batches to events.jsonl and clears the
in-memory log after each write. Assertions read via
EventLog::read_from_file(&events_path) to see the full persisted stream.

## #99 tracker after this merge

- [x] In-flight reconcile state machine (#111)
- [x] Resume-path reconciliation (#115)
- [x] Docs/contracts: cargo text is a hint, registry is truth (#117)
- [x] BDD integration scenarios for all 3 outcomes (this PR)

#99 is now fully closed.
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