Skip to content

refactor(extensions): one owner for the connector token shape - #5361

Open
karenchuu wants to merge 3 commits into
loopx-project:mainfrom
karenchuu:codex/connector-token-shape-owner
Open

karenchuu wants to merge 3 commits into
loopx-project:mainfrom
karenchuu:codex/connector-token-shape-owner

Conversation

@karenchuu

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / anchor: no pre-existing issue. Measured on the intended base 3b73108e3, the shape [A-Za-z0-9][A-Za-z0-9._:-]{0,199} -- "may this value be carried as a connector token" -- was compiled three times: loopx/extensions/external_connector_runtime.py:31, loopx/extensions/external_connector_provider.py:41 and loopx/extensions/lark/document_comment_provider.py:48. All three apply it with fullmatch, so the copies answered the same question with the same text: SAFE_TOKEN_PATTERN.

  • Owner, by the tree's own evidence rather than preference: external_connector_provider.py already imports eleven other names from it (CONNECTOR_SCHEMA_VERSION, ExternalConnectorCapability, ExternalResponsePolicy, build_external_event_response_receipt, decide_external_event_ack, settle_external_connector_event and others) from external_connector_runtime.py. The runtime module was already the place the connector boundary asked questions; only the token shape had a second local copy.

  • Observable before → after: two local re.compile calls deleted, two imports added, and a guard that fails if either file restates the shape or keeps the import while deciding some other way. Every accepted and rejected value is unchanged: the guard measures the bound at its edge (200 in, 201 out) and asserts the class rejects /, @, space, tab and non-ASCII.

  • Intended base: 3b73108e3.

Scope And Continuation

  • Done: the convergence plus a 34-case guard, in three files.
  • Not merged, and pinned as probe cases that must not be reported:
    • SAFE_SCOPE_PATTERN (external_connector_provider.py:42) allows /; SAFE_PROFILE_PATTERN (lark/document_comment_provider.py:48) drops : and bounds at 99. Different character classes are different answers, so a "consolidation" that swallowed them would be a product change wearing this PR's clothes.
    • The same character class carries other bounds across the tree -- {0,127}, {0,159}, {0,255} -- each with its own rejection text at its own surface.
  • Named successor, sized by the same value scan: ^[A-Za-z0-9._:-]{1,200}$ -- the opaque-reference shape -- is compiled in five modules on this base (chat_action_store.py:57, chat_actions.py:51, control_plane/goals/deletion_service.py:49, control_plane/goals/botmux_runtime.py:31, control_plane/goals/source_session_registry_state.py:14). It has no single owner yet, and it is the largest same-value family the connector shape sits next to.
  • Slice boundary / successor: complete within scope; reverting is two constant definitions restored.

Validation

  • Tested revision: 57e78cc04 (3 commits, 3 files, +404 -2).
  • Run state: finished.
  • Input classes: synthetic fixtures; no live connector or Lark calls.
Check kind Result Public-safe evidence / limitation
unit passed pytest tests/architecture/test_connector_token_shape_owner.py -> 34 passed in 0.55s: value scan over loopx/ with anchor normalization and same-file constant folding, per-consumer identity plus a real reference, an assertion that the provider still imports from the runtime (the guard's premise), 7 accept / 10 reject cases including the 200/201 boundary, and 9 spelling probes (5 must be reported, 4 must not).
integration passed 1122 passed, 0 failed in 1m55s at c1cef09c8; the follow-up commit 57e78cc04 only rewords a docstring inside the new guard file, which was then re-run green (34 passed).
regression_parity passed pytest over the test files that import the three touched modules -> 367 passed, 0 failed in 15s. Head has no failure to attribute, so no base replay was needed; no test file was edited.
static passed python -m ruff check on the three changed paths: clean. python -m mypy (no arguments, as CI runs it): Success: no issues found in 19 source files.
static passed loopx check --scan-path for each changed path through this tree's own entrypoint: ok: true, "public boundary scan clean: 3 files"; both warnings concern the absent local .loopx/registry.json.
semantics budget passed examples/semantic-vocabulary-drift-smoke.py on an unmodified 3b73108e3 worktree and on this head, same venv, same Node 22.23.2, same node_modules: output byte-identical, conflicting_definitions=55/55. Removing two duplicate definitions moved no ratchet because a re.compile value is not one of the inventory's counted kinds, and no new constant name is introduced here.
canary passed loopx canary premerge with the changed files passed explicitly: selected 13 / executed 13 / failures 0, status: passed, no manual holds.
mutation passed 8 mutations, one at a time in a separate worktree at this head, all files restored before every round, control round green (34 passed) before and after: 7 caught / 1 survived. Caught: the provider restating its own copy (M1, 2 cases); the document consumer restating it anchored (M2, 2 cases -- anchors are normalized because fullmatch makes them redundant); a new module assembling the shape from two same-file constants (M3); the owner tightening the bound by one character (M4, 4 cases); the owner dropping : from the class (M5, 3 cases); a consumer keeping the import while deciding with a weaker local check (M6); an unfoldable construction in a new module (M7). Survived, and why it is equivalent: M8 adds normalized.isprintable() alongside the owner check; no value the class accepts is non-printable, so the answer is identical for every input.
frontend none No UI, projection or rendered surface changes.

Type Of Change

  • Internal refactor / single-owner convergence with an anti-regression guard. No behaviour change.

LoopX Area

  • loopx/extensions/external_connector_provider.py, loopx/extensions/lark/document_comment_provider.py, tests/architecture/.

Technical Direction

  • The owner is chosen from the direction the code already imports in, and that direction is itself asserted, so a future re-parenting has to say so out loud.
  • Adjacent shapes that differ by one character or one bound are declared as not this decision and carried as negative probes. A consolidation guard that over-reports gets switched off; one that states its boundary gets extended.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
Decided on the folded value with anchors normalized, so an anchored or assembled
restatement is reported the same as a literal copy, and a construction whose
value cannot be folded has to be declared.

The runtime module is the owner because the provider already imports eight other
connector contracts from it; a probe asserts that direction, because the whole
premise rests on it. The scope and profile shapes stay separate on purpose --
they answer with a different character class, not a different spelling.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
…premise

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
@karenchuu

Copy link
Copy Markdown
Contributor Author

CI triage for exact head 57e78cc040e7955da00a592bfab41411c5479568:

All three failing shards point to the same source-fingerprint churn boundary, not to this PRs connector-token changes:

  • test-shard (2): test_runtime_source_churn_has_a_stable_readiness_diagnostic
  • test-shard (3): test_runtime_fingerprint_rescans_when_a_snapshotted_file_disappears_while_reading
  • test-shard (4): test_runtime_request_source_churn_raises_a_stable_startup_diagnostic

Those failures are in loopx/control_plane/effect_runtime.py and tests/control_plane/test_turn_journal_runtime_readiness.py; neither path is changed by this PR. This PR only changes two connector providers and adds the connector-token single-owner architecture guard. Its TypeScript shards and the other required checks passed.

The branch is currently 25 commits behind main. Current main includes 647756d21 / #5367, which specifically updates the source-fingerprint snapshot implementation and these readiness tests. Please update the branch from current main and rerun CI rather than adding an unrelated runtime fix here.

This explains the red checks only; it is not an approval. The updated exact head still needs review.

This branch has not been deployed

No deployments
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