Repository navigation
Add an auto_packaging switch so entries are packaged only on Update Package (0.21.5) - #421
Conversation
…ackage (0.21.5) When the webhook secret sets auto_packaging to "false", entry events and new Canvases no longer queue a packaging run. The Canvas is drawn from the entry's current package (or shows "Package Not Created") and Update Package becomes the only trigger. Refresh Canvas redraws without packaging. The default stays "true", so existing deployments are unchanged, and the secret is re-read within about a minute so no redeploy is needed. Profiles carry it as packages.autoPackaging, synced by sync-secrets, and the setup wizard preserves an existing secret's value when rebuilding a profile. Rebuilt against main from the idea in #395, which was stacked on the parked 392-configurability branch. Refs #392. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
 [Medium impact] The PR changes when Benchling events start packaging runs across deployments that use the webhook. Fix setup preservation, reopened-package updates, and the manual Canvas state before merging. Findings
Reviews (1) · Last reviewed commit: "Add an auto_packaging switch so entries ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pkg_prefix: config.packages.prefix, | ||
| pkg_key: config.packages.metadataKey, | ||
| ...(config.packages.workflow ? { workflow: config.packages.workflow } : {}), | ||
| auto_packaging: config.packages.autoPackaging === false ? "false" : "true", |
There was a problem hiding this comment.
Re-running setup with update-integration-secret uses buildProfileConfigFromParameters, which drops autoPackaging even when the saved profile has false. This writer then writes "true", silently restoring automatic packaging during a credential update. Preserve the setting in both profile builders before syncing.
Knowledge Base Used:
| pkgPrefix: normalizeString(payload.pkg_prefix ?? payload.pkgPrefix), | ||
| pkgKey: normalizeString(payload.pkg_key ?? payload.pkgKey), | ||
| workflow: normalizeString(payload.workflow), | ||
| autoPackaging: parseBoolean(payload.auto_packaging ?? payload.autoPackaging), |
There was a problem hiding this comment.
The runtime accepts auto_packaging: "0" as false, but parseBoolean returns undefined for it. Rebuilding a profile from that secret therefore omits autoPackaging, and the next sync writes "true". Accept the same boolean spellings as the runtime so setup does not silently restore automatic packaging.
Knowledge Base Used: Runtime authentication and configuration
| if not config.auto_packaging: | ||
| _send_final_canvas_best_effort(payload) |
There was a problem hiding this comment.
A new Canvas in manual mode uses a renderer that always draws package links when a bucket is configured, even when no package exists. It also receives no update time, so the final footer says “Pending update.” Nothing is queued to clear that state. Check for the current package and show “Package Not Created” when it is absent.
Knowledge Base Used: Webhook event and canvas handling
| if not config.auto_packaging: | ||
| return _manual_packaging_response( | ||
| payload, | ||
| "Automatic packaging is off; click Update Package on the entry's Canvas to package it.", | ||
| ) |
There was a problem hiding this comment.
If an operator turns automatic packaging off and then reopens a previously accepted entry, skipping the review event also skips removing the package seal. The Canvas still hides its buttons, and even an old Update Package button reaches a workflow that returns SEALED. Keep the reopen/unseal step separate from automatic package creation.
Knowledge Base Used: Webhook event and canvas handling
| - A new Canvas shows the entry's current package if one exists, otherwise "Package Not Created", together with the linked packages. It never shows "Updating..." indefinitely. | ||
| - **Update Package** on the Canvas still queues a packaging run, so it becomes the only way an entry is packaged. | ||
| - **Refresh Canvas** redraws from the current package without queuing anything. | ||
|
|
There was a problem hiding this comment.
The documented Refresh Canvas action is not available on a normal bucketed Canvas. create_main_navigation_buttons still offers only Browse Package and Update Package whenever a bucket is configured. Add a refresh button for manual mode, or explain that this action only exists on bucketless Canvases.
Knowledge Base Used: Webhook event and canvas handling
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
🟡 Changes recommended
Wizard flows can silently re-enable automatic packaging, and manual-mode Canvas rendering does not correctly represent missing packages or completed refreshes.
4 open issues
What changed in this PR
Adds a Secrets Manager–backed switch for manual-only entry packaging, addressing the no-auto-creation requirement in #392 without the broader App Config routing changes.
Changes:
- Gates automatic packaging while keeping Update Package available.
- Carries the setting through profile configuration, secret synchronization, and wizard imports.
- Adds tests and documentation, and updates release versions to 0.21.5.
| File | Description |
|---|---|
test/sync-secrets.test.ts |
Tests default and disabled secret values. |
README.md |
Documents manual-only packaging. |
package.json |
Bumps npm version to 0.21.5. |
package-lock.json |
Synchronizes npm release versions. |
lib/wizard/types.ts |
Adds the imported secret setting. |
lib/wizard/profile-config-builder.ts |
Preserves the setting in existing-config rebuilds. |
lib/wizard/benchling-secret.ts |
Reads the setting from secrets. |
lib/types/config.ts |
Adds the profile field and JSON schema. |
docker/uv.lock |
Synchronizes Python package version. |
docker/tests/test_config_validation.py |
Tests boolean parsing and rejection. |
docker/tests/test_app.py |
Tests manual-mode event and button behavior. |
docker/src/secrets_manager.py |
Parses and defaults the runtime switch. |
docker/src/config.py |
Applies the switch from cached secrets. |
docker/src/config_schema.py |
Adds schema fields and shared flag validation. |
docker/src/app.py |
Gates queuing and requests final Canvas rendering. |
docker/pyproject.toml |
Bumps Python version to 0.21.5. |
docker/app-manifest.yaml |
Updates the Benchling app version. |
CHANGELOG.md |
Records the 0.21.5 feature changes. |
bin/commands/sync-secrets.ts |
Writes the profile switch to Secrets Manager. |
Review effort: Balanced
|
|
||
| def _send_bucketless_final_canvas_best_effort(payload: Payload) -> None: | ||
| """Start the final bucketless Canvas render after the quick acknowledgment. | ||
| def _send_final_canvas_best_effort(payload: Payload) -> None: |
| if not config.auto_packaging: | ||
| _send_final_canvas_best_effort(payload) |
Adds the confirm prompt beside the other package settings, defaulting to the existing profile's value, and honours a CLI or profile value in non-interactive mode. Only a No is written to the profile, as packages.autoPackaging: false. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…/"1" accepted - Review-record events are queued even when automatic packaging is off, because acceptance seals the package and reopening unseals it. - A bucketed Canvas checks that the entry's package exists and renders "Package Not Created" when it does not, instead of links to nothing. - The final Canvas render stamps its footer when automatic packaging is off, since no package-revision event will follow. - The wizard's secret reader accepts "1" and "0" like the runtime does, so rebuilding a profile from such a secret keeps auto_packaging. - README no longer claims a Refresh Canvas button on bucketed Canvases. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| ) | ||
| # A review event seals the package on acceptance and unseals it when the | ||
| # review is reopened, so it must run even when automatic packaging is off. | ||
| if not config.auto_packaging and payload.event_type != "v2.entry.updated.reviewRecord": |
| if not self.config.s3_bucket_name or not self.config.auto_packaging: | ||
| updated_at = datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC") |
Summary
Adds
auto_packagingto the webhook secret. When"false":v2.entry.created,v2.entry.updated.fields,v2.entry.updated.reviewRecord) are acknowledged and skipped, nothing is queued;Defaults to
"true", so existing deployments are unchanged. The secret is re-read on the existing 60s cache, so flipping it needs no redeploy. An unparseable value is a configuration error, likeenable_webhook_verification.On the CLI side, profiles carry
packages.autoPackaging(type + JSON schema),sync-secretswrites it to the secret, and the setup wizard asks "Package entries automatically on every entry event?" beside the other package settings (default: the existing profile's value), and preserves an existing secret's value when it rebuilds a profile, so re-running the wizard does not silently turn automatic packaging back on.This is the small, standalone version of #395, which was stacked on the parked
392-configurabilitybranch (#394) and had drifted from main. Refs #392.Changes
docker/src/secrets_manager.pyBenchlingSecretData.auto_packaging(defaultTrue), parsed withparse_booldocker/src/config.py,config_schema.pyConfig.auto_packaging, applied from the secret; schema field and shared bool validatordocker/src/app.pyhandle_refresh_canvasonly creates a package when on;_send_final_canvas_best_effort(renamed, was bucketless-only) renders the Canvas when nothing is queuedlib/types/config.ts,lib/wizard/*,bin/commands/sync-secrets.tspackages.autoPackagingthrough profile, wizard and secret syncREADME.md,CHANGELOG.mdTests
test_app.py: entry event skipped; Canvas creation renders without queuing; Refresh Canvas does not queue; Update Package still queues.test_config_validation.py: default true;"false"/false/"0"/"true"/trueparsed; garbage rejected.sync-secrets.test.ts:auto_packagingis"true"by default and"false"when the profile turns it off.Dev check (2026-10-26): the branch image run locally against the real quilt-dtt tenant and the
quilt-ernest-stagingbucket, with a copy of the nightly webhook secret carryingauto_packaging: "false"and the packaging request queue pointed at a throwaway FIFO queue.entry.created,entry.updated.fieldsandcanvas.createdreturned 202 SKIPPED (mode manual) and queued nothing; Update Package returned ACCEPTED and queued one message;entry.updated.reviewRecordreturned ACCEPTED and queued a second. The Canvas render path ran (entry fetched, final render attempted); the Benchling update itself 404ed because the fixture canvas id is stale, so the visible render is covered by the unit tests only.🤖 Generated with Claude Code