Repository navigation
Conversation
briansrls
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1f6d42b14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn print_inputs_admitted(version: String, digest: String, bed_clear: String, run_attempt: String) -> Bool { | ||
| sm_version_id_is_number(version: version) && bed_clear == "true" && run_attempt == "1" && | ||
| match sha256_hex_digest(hex: digest) { Present { value: _ } => true Absent => false } |
There was a problem hiding this comment.
When an operator submits the same print as two separate workflow runs, both runs have GITHUB_RUN_ATTEMPT=1, so this predicate admits both; the workflow's concurrency group merely queues the second run and does not deduplicate it. Each run can therefore upload and publish the same physical start, despite the stated no-replay guarantee. Admission needs a durable claim keyed by the intended print operation rather than only the per-run attempt counter.
Useful? React with 👍 / 👎.
| match deliver_project_to_printer_with_access(printer: printer, payload: payload, token_source: token_source, access: PrinterExistingVersion { version: version }) { | ||
| ProjectDelivered { receipt: receipt } => { | ||
| let remote = receipt_remote_name(r: receipt) | ||
| match start_print_on_printer_with_access(printer: printer, remote_name: remote, token_source: token_source, access: PrinterExistingVersion { version: version }) { |
There was a problem hiding this comment.
If the selected printer is already running a job started earlier or outside this workflow, this path proceeds directly from digest verification to upload and start without observing device state. The local operator route explicitly requires IDLE/FINISH and print_error == 0, but the Actions route relies only on the human bed_clear input; workflow concurrency cannot detect an existing device job. Apply the same fresh report gate here before transferring or publishing the start.
Useful? React with 👍 / 👎.
| " w=csv.writer(f);w.writerow(['id','part','kind','scope','x_mm','y_mm','z_mm','notes'])\n", | ||
| " for p in model['parts']:w.writerow([p['id'],p['name'],p['kind'],p['scope'],*p['extent_mm'],p['notes']])\n", |
There was a problem hiding this comment.
The parts.csv header labels these values x_mm, y_mm, and z_mm, but every row writes p['extent_mm'], which contains the part's width, height, and depth rather than its assembly position. Consumers of the review package will therefore interpret dimensions as coordinates; either rename the columns to dimension fields or emit the placement from at_um.
Useful? React with 👍 / 👎.
briansrls
changed the title
|
Heads-up before queueing: this PR adds |
|
Closed without folding in the v1 closeout bankruptcy (#13641). A conflicting printer draft, superseded by the printer work landed in #13334. Under the bankruptcy rule, only work that serves the frozen seed emission, v2-native development or live operations, and that is complete, survives. The branch is kept for archaeology; no follow-up obligation is created. — sent from neat-wolf-604 |
1 participant
Printer starts now include automatic exact-job telemetry reconciliation. An MQTT publisher failure can occur after the printer has begun; the shared workflow now observes rather than asks an operator to determine the result or sends another start.
Both Actions and operator entry points use the same ntfy approval, fresh IDLE/FINISH readiness checks and durable escalation-bound start claim. Approval confirms physical bed clearance; readiness and approval expiry are checked again after upload. Only error 0 and the vendor-documented non-existent 0500C011 are nonblocking.
After an acknowledged or uncertain publish, the workflow makes bounded read-only observations through the existing credential session. Success requires RUNNING with the exact digest-derived filename and a nonblocking error in one fresh report. Partial, mismatched and completed-job reports cannot confirm. Pause/failure stops with a blocked outcome; exhaustion stays uncertain. Raw reports are retained beside the claim, and the receipt includes the final outcome and evidence path. Cleanup faults remain failures. The publisher has a 20-second timeout plus 2-second kill grace. No outcome authorizes a second publish.
The connection-drop root cause is not established. This change handles the ambiguity; it does not claim application-level exactly-once MQTT delivery. The new live path will be exercised on the next authorized print, not by restarting the current jobs.
Existing authorization work: one sealed existing-version credential session, explicit receipt token source, centralized printer roster, independent credential version pins, modeled LAN runner identity and privileged-site census. The WIF and operator-token routes retain their documented authorization-pattern divergences.
Validation on this revision: 5 reconciliation, 6 approval/readiness and 24 publisher/no-replay witnesses passed locally (35 total). CI is not claimed green. Earlier census and workflow-binding validation is recorded in the review document.
Deployment:
/var/lib/gunbc/printer-startsmust remain private and persistent on the modeled LAN host. Never delete a claim to retry an uncertain start. The operator host has the tested source revision staged separately for future jobs.Targets
maindirectly. Includes the approval-client cutover commit from #13218, which routes requests to the active approval store; that prerequisite does not need to land separately before this PR. Related work: CAD/cassette consolidation #13295; slicer preparation #13223. Seedocs/plans/printer-workflow-review.mdfor behavior and limits.