Skip to content

Add a rule to match the upstream framework's behavior to AGENTS.md #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
campoy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a rule to match the upstream framework's behavior to AGENTS.md #272

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ overall maturity (for example, OpenAI Agents is Generally Available with preview
- No changelog files. `scripts/release/release_tool.py release-notes` derives notes from commit subjects touching the plugin directory since its previous tag.
- Every GitHub Action is pinned to a full commit SHA with a `# vN` comment (org opengrep rule).

## Plugin behavior

@DABH DABH Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P3: nothing prompts the 'say in the PR' disclosure. CONTRIBUTING.md:14-15 defers to the org PR template (issue, how tested, docs updates); there is no repo PULL_REQUEST_TEMPLATE.md, and the 'Pull requests' section is untouched although the PR body calls this 'the docs update'. A contributor filling the three checklist items never states the contradiction, and reviewers have no cue to ask. Add one line under 'Pull requests' pointing at this section, or fold it into checklist item 3. Separately, 'How was this tested: check_conventions.py passes' is vacuous: the script never reads AGENTS.md, and scripts/ci/detect_changes.py:72 says a root-level AGENTS.md change selects nothing.


- Match the upstream framework's behavior wherever Temporal allows it: what its code does and its tests pin, even where its docs say otherwise. Don't keep or add a plugin-specific rule that departs from it, and say in the PR when upstream's docs contradict its code.

@DABH DABH Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P1: no precedence for replay compatibility over 'Don't keep'. The only workflow.patched sites in plugin source are legacy branches whose old behavior departs from upstream and is kept solely so recorded histories replay: python/deepagents/src/temporalio/deepagents/workflow.py:104-108 ('replaying them without the cache would emit commands history does not have'), python/strands_agents/src/temporalio/strands_agents/_temporal_model.py:244, python/openai_agents/src/temporalio/openai_agents/_openai_runner.py:147-149 and :281. AGENTS.md is a coding agent's verbatim instruction set; an agent that deletes one of those branches to comply produces a nondeterminism error on every Workflow Task of an in-flight run. Every other behavior statement in this file exists for replay (line 59 Go identifiers, line 155 Activity names), and repo practice on gating is split: #204 (b11a86b) gated its refusal, while #270, this rule's own exemplar, flips runConfig.model precedence with no gate ('switches to it, without an error'). Add a sentence that behavior changes affecting recorded histories are gated or called out as breaking with in-flight and Worker Versioning guidance, and that it takes precedence over this bullet.

@DABH DABH Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P2: the carve-out qualifies only the first sentence. 'wherever Temporal allows it' attaches to 'Match the upstream framework's behavior'; the second sentence, 'Don't keep or add a plugin-specific rule that departs from it', carries no qualifier and switches noun from 'behavior' to 'rule', a word this file otherwise uses only for enforced policy (line 84 'org opengrep rule', line 335 'Python rules'). Read on its own it forbids keeping the Temporal-required departures the first sentence permits, such as typescript/openai-agents/README.md:100-101 (runConfig.model must be a string; signal unsupported), and a reader cannot tell whether 'rule' means the README statement or the code behind it (#270 changed both for the same rule). One sentence avoids the drift: 'Match the upstream framework's behavior wherever Temporal allows it, in existing plugin code as well as new: what its code does and its tests pin, even where its docs say otherwise. Say in the PR when upstream's docs contradict its code.'

@DABH DABH Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P2: 'wherever Temporal allows it' is undefined, and the departures the plugins actually make are policy, not impossibility. A literal reader can take 'allows' as 'the Workflow sandbox does not throw'. Upstream paths that run without any Temporal error and are refused anyway: python/openai_agents/src/temporalio/openai_agents/_openai_runner.py:270-278 (a provider set on the run), _trace_interceptor.py:30-31 (#186: the header is stored in Event History, so it must not hold a credential), _temporal_model_stub.py:125-133, and typescript/openai-agents/src/workflow/runner.ts:243-247 (UnsafeSessionError for upstream's MemorySession). Merged plugins also depart from upstream on grounds Temporal permits either way: _invoke_model_activity.py:406-412 defaults to AsyncOpenAI(max_retries=0) where upstream defaults to 2; python/typesafe/src/temporalio/typesafe/_plugin.py:31-36 and python/google_genai/src/temporalio/google_genai/_google_genai_plugin.py:25-42 raise on upstream-permitted retry options; go/googleadk/README.md:213-215 asks users to disable SDK retries; typescript/google-adk/src/hitl.ts:32-37 drops adk_request_credential so secrets never enter history; go/googleadk/activities.go:135-156 adds a zero-config gemini-* fallback that upstream's registry explicitly makes opt-in. A contributor restoring upstream defaults to comply gets SDK backoff multiplied by Temporal retries, secrets in Event History, or a hung graph. Enumerate the grounds (retry ownership, history as the only state store, no secrets in history, determinism) and say that refusals enforcing them are not departures.

@DABH DABH Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P2: the rule codifies #270's symptom, not its cause. #270's stated root cause is 'the plugin's copies of the framework's objects', yet nothing here mentions cloning, re-implementation or extension points, so a faithful copy that matches upstream's current code and tests satisfies every clause and drifts at the next upstream release. The GA Python plugin still does what #270 removed in TypeScript: _openai_runner.py:69 dataclasses.replace(agent), :98 re-wrapped on_invoke_handoff, :118-121 replaced tool _agent_instance, still called at :326 although a provider is installed at :294. That clone silently dropped upstream's Handoff._agent_ref (added in 0.9.0) until #85, and sdk-python #1310 was a second bug of the same class. State the mechanism: run upstream's own code paths and intervene only at Temporal seams through its extension points; never clone framework objects or depend on private fields.


## Python conventions

- Layout: `python/<name>/{pyproject.toml, uv.lock, plugin.toml, Makefile, README.md, LICENSE, src/temporalio/<name>/, tests/}`.
Expand Down
Loading