feat(memory): add pluggable OMS backends - #331
Sertaç Özercan (sozercan) wants to merge 63 commits into
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
![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: 61198a0709
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce2b1cf93c
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa4afb6695
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
orka/internal/cli/client/client.go
Lines 290 to 294 in a1396e8
Preserve namespaces encoded in operation locations
In the orka memory update --file ... --wait path, a manifest namespace may intentionally override the client's configured default namespace, and the server returns that target namespace in the operation Location. When the waiter follows the location with a nil query map, this block overwrites the already-encoded namespace with c.Namespace, so it polls the wrong namespace and typically fails with 404 while the mutation continues. Only inject the client default when neither the path's query nor the supplied query contains a namespace.
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76e80d7287
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a5035e417
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f86b91bd9
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 258deac801
ℹ️ 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bbd3f1e80
ℹ️ 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 (@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 (@codex) address that feedback".
| if err := k8sClient.Create(ctx, secret); err != nil { | ||
| return nil, fmt.Errorf("creating child transaction token placeholder: %w", err) |
There was a problem hiding this comment.
When the tool context is cancelled after this Secret is created but before the child Task is created or adopts it, cleanup is attempted with the same cancelled context, so its read/delete cannot complete; a worker exit in this window has the same effect. Because the placeholder has no owner reference and there is no placeholder janitor, repeated timed-out delegations permanently accumulate Secrets and can eventually exhaust the namespace's Secret quota. Give placeholders durable expiry-based cleanup or perform cancellation-safe bounded cleanup.
Useful? React with 👍 / 👎.
| payload, ownershipCertificate, statusCode, err := doOMSProbeRaw( | ||
| ctx, httpClient, target.Endpoint, omsprotocol.PathOwnershipClaim, target.BearerToken, ownershipRequest, | ||
| http.StatusOK, http.StatusConflict, | ||
| ) |
There was a problem hiding this comment.
When a backend advertises a maxRequestBytes smaller than the ownership request or returns an ownership response above maxResponseBytes, ProbeBinding still uses doOMSProbeRaw, which enforces only the protocol-wide hard caps. Fresh evidence after the runtime OMSClient limit fix is that this separate activation path already has the decoded capability limits but does not apply them, so a backend can be claimed and marked ready despite violating its negotiated capacity. Validate the marshaled request and returned payload against the advertised limits before accepting the ownership result.
Useful? React with 👍 / 👎.
| if next.ActualMode != first.ActualMode || !next.SnapshotExpiresAt.Equal(first.SnapshotExpiresAt) { | ||
| return nil, errors.New("pagination continuation changed snapshot mode or expiry") | ||
| } | ||
| if err := appendUniqueSnapshotRecords(&result, seenRecords, next.Records); err != nil { |
There was a problem hiding this comment.
When an adapter advertises a small maxSnapshotRecords but an auto, semantic, or hybrid probe yields more distinct records across pages, this collector keeps accepting records until the protocol-wide hard page bound. Those probe results are discarded by consumeSearchProbe, so unlike the explicit keyword fixture they are never checked for an exact result set, allowing an adapter that exceeds its advertised snapshot capacity to pass conformance. Reject as soon as the cumulative record count exceeds client.limits.MaxSnapshotRecords.
Useful? React with 👍 / 👎.
| if !pageComplete && !request.AllowIncomplete { | ||
| memoryIncompleteTotal.Inc() | ||
| return nil, &IncompleteSearchError{ | ||
| Cause: apierror.New(http.StatusServiceUnavailable, ReasonResultSetIncomplete, | ||
| "memory search scan budget was exhausted"), | ||
| Cursor: cursor, | ||
| } |
There was a problem hiding this comment.
When a strict remote search exhausts the 20-page/candidate budget after finding some eligible items but before filling target, this returns only an error and cursor after saveRemoteSearchContinuation has advanced past every item already accumulated in items. Following the cursor can therefore never return those matches; for example, a valid adapter with maxPageSize=1 and a default 100-result request loses the first 20 matching records on every incomplete attempt. Fresh evidence after the legacy continuation fix is that this remote branch still does not carry undelivered matches into cursor state; persist them for replay before returning the error.
Useful? React with 👍 / 👎.
| if caps.Revision != checkpoint.CapabilitiesRevision { | ||
| result.Message = "capability revision changed between prepare and verify" | ||
| return result |
There was a problem hiding this comment.
When an adapter changes an optional capability or advertised limit across restart but reuses the same revision, verification accepts it because the checkpoint records and compares only that string; it then builds fixtures from the new limits. Such an adapter can pass even though the profile requires one revision to identify one effective behavior and configuration. Persist the capability flags and limits in the prepare checkpoint and compare them during verify, excluding the renewable expiry.
Useful? React with 👍 / 👎.
| continuation, err := collectContinuation(ctx, client, checkpoint.PaginationRequest) | ||
| if err != nil { | ||
| result.Message = err.Error() | ||
| return result |
There was a problem hiding this comment.
When an otherwise valid adapter advertises a very short snapshotTtlSeconds such as 1, Prepare checkpoints a page token but verification unconditionally consumes it only after the operator has restarted the adapter. The protocol validator permits any positive TTL, so the snapshot can legitimately expire before VerifyAfterRestart reaches this call, at which point the adapter must reject the expired token and is incorrectly reported as non-conformant. Define a minimum TTL sufficient for the two-phase restart proof or otherwise ensure verification occurs within the advertised lifetime.
Useful? React with 👍 / 👎.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
core.orka.ai/v1alpha1 MemoryBackend/defaultcontrol plane with explicit staged validation, durable authority cutover, routing fences, lifecycle recovery, and fail-closed SQLite behaviororka.oms.v0alpha1protocol, public Go protocol/conformance packages, durable reference implementation, operation ledger, bounded pagination, governance/audit state, and matched checkpoint requirementsSafety and rollout
Verification
make manifestsmake lint-fixDEVELOPER_DIR=/Library/Developer/CommandLineTools make testhelm lint manifest_staging/charts/orkago run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/helm-chart.yml .github/workflows/release.ymlbash -n scripts/*.shpython3 -m py_compile scripts/update-release-version.pygpt-5.6-sol, max reasoning — cleanopus(canonicalclaude-opus-5), max effort, withCLAUDE_CODE_DISABLE_ADVISOR_TOOL=1; clean Go/release slices, with the sole migration-note suggestion explicitly rejected by Codex because the adapter was never merged, promoted, tagged, or released in-treeLive KD6 conformance is owned by the out-of-tree adapter release environment because it requires the controlled digest-pinned KD6 test image, credentials, and HTTPS endpoint.