Skip to content

feat(temporal): env-helper dispatch resolution — source-tiered defaults, allow-list, const-ref - #90

Merged
zzet merged 4 commits into
zzet:mainfrom
avfirsov:pr/temporal-env-helper
Jun 15, 2026
Merged

feat(temporal): env-helper dispatch resolution — source-tiered defaults, allow-list, const-ref#90
zzet merged 4 commits into
zzet:mainfrom
avfirsov:pr/temporal-env-helper

Conversation

@avfirsov

Copy link
Copy Markdown
Contributor

What

Brings Temporal env-helper dispatch resolution onto current main, as a 3-commit stack (kept separate for commit-by-commit review):

  1. env-helper recall + source-tiered env-default — recognise env-var-with-default dispatch helpers (GetEnvOr("ACTIVITY", "Default")-style) and resolve the dispatch name from the literal default, tiered by source confidence:
    • allow-listed helper / os.Getenv → visible, OriginASTInferred @ 0.6
    • name-heuristic guess (helper name contains "env") → hidden, OriginSpeculative @ 0.4
      Also fixes wrapper depth>1 (forwarded-param stubs now resolve via a bounded fixpoint).
  2. config-driven env-helper allow-list.gortex/temporal-allowlist.yaml (git-ignored, opt-in) lets a repo declare its own env-helper function names so their defaults resolve at the visible tier. Config-only, applied cleanly.
  3. constant-reference defaults (Go) — resolve GetEnvOr("ACTIVITY", constants.DEFAULT_NAME) where the default is a const reference, by dereferencing it through the const-deref map.

Why

Dispatch names sourced from env-var-with-default helpers were left broken_dispatch. This recovers them while being honest about confidence: a config-confirmed/os.Getenv default is trustworthy (visible), a name-only heuristic is a guess (speculative/hidden), so it never pollutes high-confidence results.

Porting notes (post-#82)

PR #82 refactored the resolver (4-arg idx.lookup(kind, name, callerRepo, callerLang), replay-loop goTemporalEnvDefaultName, removed constVal/convention helpers). The stack was re-applied onto that structure:

  • env-default tiering transplanted onto main's loop, preserving main's cross-language branch; MetaSpeculative now keyed on envSpeculative||crossLang so visible env-defaults aren't hidden.
  • const-ref defaults re-implemented on main's derefByName/buildConstDerefMap (replacing the removed constVal).
  • eligibleTemporalCandidates cross-repo *_test.go stub filter kept on the surviving register-confirmed path.

Behavioral note for reviewers

PR #82 silently dropped the "unregistered activity resolves via name convention" guarantee; the two resolver tests here document the registered-path behavior instead (the cross-repo test-stub filter is verified load-bearing). The separate convention fallback is proposed in its own PR (#89).

Tests

New: allow-list config tests, 5 env-helper extractor tests, 3 const-ref tests, env-heuristic + wrapper-depth-2 e2e. go build ./..., go vet, and -race clean; resolver / parser / indexer / config suites pass.

avfirsov and others added 3 commits June 15, 2026 09:58
…ispatch fixes

Recognition + tiering core for the "AST recall → allow-list promote → LLM
clean" pipeline, bundled with three standalone dispatch fixes that live in
the same functions:

- Generic env-helper heuristic: any helper whose name contains "env"
  (cfg.ActivityFromEnv("KEY","Default")) is recognised structurally and the
  dispatch lands at the hidden speculative tier (temporal_env_source=heuristic).
- Source-tiered env-default: goTemporalEnvDefaultName now reports HOW it
  resolved (os_getenv / allowlist / heuristic); the resolver tiers it —
  allowlist/os_getenv → inferred 0.6 (visible), heuristic → speculative 0.4
  (hidden). GoExtractor gains SetEnvHelperNames + an extra-allow-list param
  threaded through the env-default recognition.
- GetEnvOrDefaultValue added to the built-in env-helper allow-list.
- Cross-repo *_test.go stub FP filter: a dispatch in repo A no longer
  resolves to a test mock in repo B (isCrossRepoTestStub), the one confirmed
  false positive from the corpus audit.
- Wrapper depth>1 fix: arg_names are now recorded for a forwarded lowercase
  parameter, so the iterative wrapper pass follows a multi-hop chain to a
  fixpoint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lets a corporate fork PROMOTE its own env-helper names from the generic
"env"-name heuristic (hidden) to the allow-list tier (visible) without
committing those names into the source tree.

- config.LoadLocalTemporalEnvHelpers reads .gortex/temporal-allowlist.yaml
  (env_helpers:) ONLY under GORTEX_ALLOW_LOCAL_TEMPORAL=1, mirroring the
  providers.json repo-local opt-in. Fail-soft: missing gate / file / malformed
  yaml all yield no extra names.
- languages.ConfigureTemporalEnvHelpers installs the loaded names onto the Go
  extractor; wired into `gortex index` (analyze wiring lands with the LLM pass).
- .gortex/ added to .gitignore so the local allow-list and verdict cache are
  never committed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the zzet#1 corpus gap (per the design spec): an env-helper whose default
argument is a constant reference rather than a string literal —
`wfutil.GetEnvOrDefault(config.ACTIVITY_NAME_ENV, config.ACTIVITY_NAME_DEFAULT)`.

- goArgDefaultValue reduces a default argument to its value: a literal
  (isConst=false) or a constant NAME — bare identifier or selector_expression
  (isConst=true). The three env-default extractors (os.Getenv-mixed, allow-list
  helper, env-name heuristic) all report it.
- goTemporalEnvDefaultName returns the const name separately; the parser keeps
  temporal_name as the dispatch variable and emits temporal_default_const, with
  source=const_ref for trusted helpers (allow-list / os.Getenv) and source
  staying heuristic for an unproven env-named helper.
- The resolver substitutes temporal_default_const through its existing constVal
  index (register-confirmed, then convention fallback). const_ref tiers as
  inferred 0.6 (visible), heuristic stays speculative 0.4 (hidden).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@avfirsov
avfirsov force-pushed the pr/temporal-env-helper branch from ccb5db8 to cb6de90 Compare June 15, 2026 06:59
@zzet

zzet commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Strong work on the tiering — the visible-vs-hidden split (os.Getenv / allow-list / const-ref → inferred 0.6; name-heuristic → speculative 0.4) is exactly the right call, and the .gortex/temporal-allowlist.yaml opt-in gated behind GORTEX_ALLOW_LOCAL_TEMPORAL is a clean way to keep corporate helper names out of the tree. The const-ref default channel and the wrapper-depth fixpoint are solid. One blocker before merge, plus a design question it surfaces.

Blocker: the allow-list never loads under the daemon / MCP

ConfigureTemporalEnvHelpers is wired into exactly one place — the one-shot CLI:

  // cmd/gortex/index.go, runIndex()
  languages.RegisterCustomGrammars(reg, cfg.Index.Grammars, idxLogger)
  languages.RegisterExtractorPlugins(reg, cfg.Index.ExtractorPlugins, idxLogger)
  languages.RegisterFallbackChunkers(reg, cfg.Index.FallbackChunkers, idxLogger)
  languages.ConfigureTemporalEnvHelpers(reg, config.LoadLocalTemporalEnvHelpers(path)) // ← only here

But the daemon — the primary path for MCP/agent usage — builds its own registry and never calls it:

  // internal/serverstack/shared_server.go:233-239
  reg := parser.NewRegistry()
  languages.RegisterAll(reg)
  languages.RegisterCustomGrammars(reg, conf.Index.Grammars, logger)
  languages.RegisterExtractorPlugins(reg, conf.Index.ExtractorPlugins, logger)
  languages.RegisterFallbackChunkers(reg, conf.Index.FallbackChunkers, logger)
  // no ConfigureTemporalEnvHelpers
  idx := indexer.New(g, reg, conf.Index, logger)

…and so does the crash-isolated parse worker:

  // internal/parser/crashpool/worker.go, RunWorker()
  reg := parser.NewRegistry()
  languages.RegisterAll(reg)
  registerWorkerGrammars(reg)
  registerWorkerExtractorPlugins(reg)
  registerWorkerFallbackChunkers(reg)
  // no env-helper equivalent

Net effect: under gortex daemon / MCP, the repo-local allow-list is silently inert — only the built-in goEnvHelperNames and the "env"-name heuristic ever apply. The PR description's "config-driven env-helper allow-list" is true for gortex index only.

In fairness: the three sibling helpers are also CLI-only today, so this isn't a regression — it follows an existing pattern. But the allow-list is the headline of commit 2, so it should actually be reachable from the daemon before we call it done.

Why a naive "just add the call to the daemon" won't fully work

The allow-list is stored as instance state on a process-shared singleton:

  • Registry.Register keeps one extractor per language: r.extractors[lang] = e (internal/parser/registry.go).
  • ConfigureTemporalEnvHelpersreg.GetByLanguage("go") returns that single *GoExtractor, and SetEnvHelperNames mutates its envHelperExtra field.
  • The daemon constructs one reg and one indexer.New(g, reg, …) that serve every repo in the workspace.

So SetEnvHelperNames is effectively process-global: in a multi-repo daemon it's last-writer-wins across repos, which contradicts the per-repo design of LoadLocalTemporalEnvHelpers(repoPath) (there's no single repoPath to pass at daemon startup). And because crash-isolated parsing happens in a separate gortex __parse-worker subprocess, even setting it on the parent's extractor wouldn't reach the worker — you'd need a registerWorkerTemporalEnvHelpers plus a way to ship the names over the worker protocol.

Suggested resolution
Cleanest for multi-repo (my preference): stop storing the allow-list on the parser singleton and move the tier decision to resolve time. Have the extractor emit the env-helper candidate with the helper name tagged (no visible/hidden decision yet), and let ResolveTemporalCalls — which already runs in-process with per-repo prefixes — promote to the visible tier when the dispatch's repo opts in. That decouples it from both the shared extractor and the subprocess boundary, and makes the allow-list genuinely per-repo.

Smaller question

  • Was the git-ignored .gortex/temporal-allowlist.yaml (separate file + env gate) chosen over extending cfg.Index deliberately? The "don't leak helper names upstream" rationale is reasonable, but it means the allow-list can't ride the existing conf.Index.* plumbing that's already threaded into the daemon. If the names aren't actually sensitive, folding env_helpers into the indexed .gortex.yaml config would let it reuse the wiring the siblings already have.

Happy either way — just want the choice to be intentional.

Everything else (tiering, const-ref, wrapper fixpoint, the _test.go cross-repo stub filter) looks good to me. Resolve the wiring story and I think this is ready.

Reconcile env-helper dispatch resolution with the convention fallback now
on main:
- temporal_calls.go ladder keeps both tiers (env-default const-ref →
  convention fallback) before the cross-language join
- lookupConvention now drops cross-repo *_test.go stubs like the register
  path, so a convention match never lands on a cross-repo test fixture
- reconstructed the two appended e2e tests (WrapperDepth2 + unregistered-
  by-convention) as separate functions
@zzet
zzet merged commit 6cc12ec into zzet:main Jun 15, 2026
7 of 9 checks passed
zzet added a commit to avfirsov/gortex that referenced this pull request Jun 15, 2026
Reconcile var-trace / const-alias / exact-name signature dispatch (zzet#98) with
the convention (zzet#89) and env-helper (zzet#90) tiers now on main:
- resolution ladder keeps exactSig (speculative) after the cross-language
  join; the env-default tier-override now also covers var_trace origins and
  preserves zzet#90's source-tiering (allowlist/os_getenv/const_ref stay visible)
- single temporal_resolution_via stamp (exact_sig | convention) instead of two
  blocks that clobbered each other
- fold the funcExact (zzet#98) and funcByName (zzet#89) indexes into one node sweep
- goConstLiteralValue: keep main's grouped-const selection (drop zzet#98's
  equivalent duplicate)
- lookupConvention drops cross-repo *_test.go stubs (matches main's fix)
- regression test for parallel-assignment var-trace (x, act := _, "Name")
zzet added a commit to avfirsov/gortex that referenced this pull request Jun 15, 2026
Reconcile Java cross-language dispatch (zzet#97) with var-trace/env-helper/
convention resolver now on main:
- buildConstDerefMap unified: keep main's const-alias following (aliasRef +
  bounded fixpoint) AND zzet#97's Java string-field collection (javaFieldVals),
  both feeding one order-independent ingest with the shared ambiguity rule;
  no early-return on a non-ConstantValueReader store so Java fields still load
- register.go keeps both ConfigureTemporalJavaInvokers (zzet#97) and
  ConfigureTemporalEnvHelpers (zzet#90)
- temporal_e2e_test.go is the union of both test sets (4 Java cross-lang tests
  added alongside main's var-trace/convention/env tests)
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.

2 participants