Skip to content

Use schema options for session permission mode#2087

Open
julianknutsen wants to merge 3 commits into
mainfrom
fix/pr1931-schema-options
Open

Use schema options for session permission mode#2087
julianknutsen wants to merge 3 commits into
mainfrom
fix/pr1931-schema-options

Conversation

@julianknutsen
Copy link
Copy Markdown
Collaborator

Supersedes #1931.

Summary

  • Adds POST /v0/city/{cityName}/session/{id}/permission-mode using existing schema options / template_overrides persistence.
  • Keeps permission_mode durable until changed again; no live process permission mutation system.
  • Validates permission_mode against the resolved provider options_schema before persisting.
  • Rejects running, active, awake, creating, draining, quarantined, and recent-wake sessions so changes apply only before the next launch.
  • Adds legacy /v0/session/{id}/permission-mode parity with X-GC-Request and X-GC-Index.
  • Resolves provider/agent collisions from existing metadata and persisted provider/template data; does not persist real_world_app_session_kind.
  • Updates OpenAPI, generated clients, dashboard generated types, and docs.

Verification

  • .githooks/pre-commit passed, including lint, generated docs/schema checks, go vet ./..., and observable go test ./....
  • make dashboard-check passed.
  • Targeted regression tests passed for permission-mode updates, legacy route parity, durable overrides, runtime replay, and provider/agent collision handling.
  • $review-pr dual-model loop passed with zero blockers and zero majors in both Claude and Codex lanes after fixes.

@julianknutsen julianknutsen requested a review from csells as a code owner May 13, 2026 20:15
@julianknutsen julianknutsen added the status/needs-review-auto PR review requested with auto approval label May 13, 2026
@mintlify
Copy link
Copy Markdown

mintlify Bot commented May 13, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
gascityinc-5c0069dd 🟢 Ready View Preview May 13, 2026, 8:16 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@julianknutsen julianknutsen added status/reviewing status/needs-review-auto PR review requested with auto approval and removed status/needs-review-auto PR review requested with auto approval labels May 13, 2026
@github-actions github-actions Bot added the status/needs-triage Inbox — we haven't looked at it yet label May 13, 2026
@julianknutsen julianknutsen added status/needs-review-auto PR review requested with auto approval status/reviewing and removed status/needs-review-auto PR review requested with auto approval status/needs-triage Inbox — we haven't looked at it yet status/reviewing labels May 13, 2026
@julianknutsen julianknutsen added status/review-failed PR-review workflow failed before merge-ready and removed status/needs-review-auto PR review requested with auto approval status/reviewing labels May 13, 2026
csells
csells previously approved these changes May 15, 2026
Copy link
Copy Markdown
Contributor

@csells csells left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Collaborator

@quad341 quad341 left a comment

Choose a reason for hiding this comment

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

@julianknutsen — first, thank you. This is genuinely excellent work and we want to ship it. Replacing the persisted real_world_app_session_kind discriminator with the metadata-derived UseAgentTemplateForProviderResolution helper (with the legacy reader keeping old beads working) untangles a real class of provider/agent collision bugs, and the truth-table tests in internal/session/provider_resolution_test.go plus the wake-in-flight grace boundary tests are exactly the shape we want. @csells's approval still stands.

The only thing in the way is the CONFLICTING merge state, and I tried to take that on my side and my automation tapped out — almost certainly because of the scope of the regeneration step, not anything about your code. Rather than make you wait while I rework the maintainer-side tooling, could you do one rebase pass and let it ride?

Concretely, the one thing I need:

  1. Rebase fix/pr1931-schema-options onto fresh main.
  2. Regenerate the conflicted artifacts rather than hand-merging them:
    • cmd/gc/dashboard/web/src/generated/{index.ts,schema.d.ts,sdk.gen.ts,types.gen.ts} — via make dashboard-check.
    • docs/schema/openapi.json, docs/schema/openapi.txt, internal/api/openapi.json, internal/api/genclient/client_gen.go — via the OpenAPI/genclient regeneration step from the Huma source of truth.
    • docs/reference/cli.md — via the doc-generation step.
  3. For the non-generated conflicts (handler_sessions.go, session_runtime.go, handler_session_create.go, cmd_session.go, worker_handle.go), keep your helper-based call patterns — those are the substantive contribution and they're right.
  4. Push. I'll re-verify with .githooks/pre-commit + make dashboard-check + the targeted Go tests, clear the stale status/review-failed label (the dispatched-suite rows are SKIPPED, not failed), and merge. No additional review needed — this is just the rebase.

If the rebase turns up anything ambiguous in the non-generated files, ping me and I'll dig in directly. And again — really nice work on this one, especially the boundary coverage around the wake grace. Sorry for the round-trip.

Review coverage:

  • completed: claude codex
  • unavailable: qwen

@julianknutsen julianknutsen force-pushed the fix/pr1931-schema-options branch from 6c75e41 to f3de38b Compare May 16, 2026 20:45
@julianknutsen julianknutsen added status/needs-review-auto PR review requested with auto approval status/reviewing and removed status/review-failed PR-review workflow failed before merge-ready status/needs-review-auto PR review requested with auto approval labels May 16, 2026
@julianknutsen julianknutsen added status/needs-review-auto PR review requested with auto approval and removed status/reviewing labels May 16, 2026
@julianknutsen julianknutsen added status/reviewing status/needs-review-auto PR review requested with auto approval and removed status/needs-review-auto PR review requested with auto approval labels May 16, 2026
if resolved == nil || len(resolved.OptionsSchema) == 0 {
return nil, nil
}
mergedOptions := make(map[string]string, len(resolved.EffectiveDefaults)+len(optionOverrides))
Resolve conflict in generated dashboard index.ts by regenerating via
make dashboard-check; openapi spec already carries the new
permission-mode endpoint from the contributor's branch.

Co-Authored-By: julianknutsen <julianknutsen@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@blacksmith-sh

This comment has been minimized.

@julianknutsen julianknutsen added status/review-failed PR-review workflow failed before merge-ready and removed status/reviewing labels May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/review-failed PR-review workflow failed before merge-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants