Skip to content

fix(camunda-ai-agents): require explicit provider secrets - #102

Open
bojtospeter wants to merge 16 commits into
mainfrom
feat/issue-99
Open

fix(camunda-ai-agents): require explicit provider secrets#102
bojtospeter wants to merge 16 commits into
mainfrom
feat/issue-99

Conversation

@bojtospeter

Copy link
Copy Markdown
Contributor

Description

Require AI Agent configurations to use a user-selected provider, model, and existing connector-secret names. The skill now stops for missing configuration, explains the Camunda SaaS Console secret boundary, and the outcome eval verifies the requested provider/model/secret reference.

Closes #99

Checklist

  • make lint passes
  • Updated SKILL.md / references/ if behaviour changed

Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 12:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The evaluation needs targeted provider-input checks, explicit environment setup, and coverage for missing configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR requires explicit provider, model, and connector-secret configuration for Camunda AI Agents.

Changes:

  • Documents configuration checkpoints and SaaS secret handling.
  • Extends outcome evaluation for provider configuration.
File summaries
File Summary
skills/camunda-ai-agents/SKILL.md Documents explicit provider, model, and secret requirements.
evals/skills/camunda-ai-agents/outcomes.py Validates provider configuration in generated BPMN.
Review details

Suppressed comments (3)

evals/skills/camunda-ai-agents/outcomes.py:220

  • The new sample exercises the provider checkpoint but still omits the target cluster/profile required by SKILL.md lines 30 and 35. Because this eval has no follow-up user turn, a compliant agent can stop before creating /workspace/process.bpmn, causing the sample to fail; state the local test cluster and default c8ctl connection explicitly (or relax the checkpoint for artifact-only generation).
            "7. Use the OpenAI provider with model 'gpt-4.1-mini' and the "
            "already-configured connector secret 'OPENAI_API_KEY'; do not "
            "invent another provider or secret name.\n"

evals/skills/camunda-ai-agents/outcomes.py:220

  • The only sample supplies provider, model, and secret, so the new behavior in SKILL.md—stopping and asking when any value is missing—is never exercised. A regression to selecting a default provider or inventing a secret would still pass this eval; add a missing-configuration sample/scorer that expects clarification and no provider configuration or artifact.
            "7. Use the OpenAI provider with model 'gpt-4.1-mini' and the "
            "already-configured connector secret 'OPENAI_API_KEY'; do not "
            "invent another provider or secret name.\n"

skills/camunda-ai-agents/SKILL.md:35

  • The new checkpoint is the core regression fix, but the outcome eval only supplies provider, model, and secret in the prompt and then expects a BPMN artifact. It never exercises the missing-configuration path, so an agent that still defaults a provider or invents a secret could pass; add a scenario with those values omitted and assert that it asks and stops before applying provider configuration.
If the user did not specify any of these, ask for the missing values and stop before applying the template or writing provider configuration. Do not default to a provider or model, and do not invent names such as `ANTHROPIC_API_KEY`. Ask for secret names, not secret values, and never put secret material in the BPMN or in the conversation.
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread skills/camunda-ai-agents/SKILL.md Outdated
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🧪 Eval results

Triggers 2/2 · Outcomes 2/3⚠️ 2 need attention. Non-blocking signal (doesn't block merge).

Model anthropic/claude-sonnet-4-6 · 1,068,566 tokens [I: 112, CW: 68,050, CR: 972,560, O: 27,844]
I input · CW cache-write · CR cache-read · O output. Cost gate keys on I+O; CW/CR are diagnostic.

Outcome evals

Eval Outcome I+O (vs baseline)
camunda-ai-agents ⚠️ 1/3 — regenerate baseline · 1 no baseline
camunda-development ✅ 7/7 14k (+6% vs 14k)
rocket-launch ✅ 1/1 🔴 8k (+65% vs 5k)

Needs attention

  • camunda-ai-agentsmissing-provider-configuration — missing_configuration_guard 0.00 (< 1.0)
  • camunda-ai-agentsticket-triage-subprocess — ai_agent_shape_valid 0.00 (< 1.0)
  • rocket-launchtimer-countdown — I+O 8k > ceiling 7k (baseline 5k)

Trigger evals (skill routing)

Skill Routing
camunda-ai-agents ✅ 3/3
camunda-development ✅ 3/3

Per-eval token usage and full logs → run summary

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The outcome scorer has multiple moderate validation gaps, and the skill’s missing-value guard does not cover partial configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

evals/skills/camunda-ai-agents/outcomes.py:290

  • This literal-marker gate rejects a valid clarification such as “Which provider, model, and existing secret name should I use?”: it contains all three required terms and is a direct question, but none of the markers is present. Detect question/imperative wording or remove this brittle gate so valid requests do not score as failures.
        if not any(
            marker in assistant_text
            for marker in ("ask", "need", "missing", "provide")
        ):

evals/skills/camunda-ai-agents/outcomes.py:290

  • The guard only checks that the three words and any one generic marker occur somewhere in the assistant transcript. A response such as “I need to stop because provider/model/secret are unspecified” passes without asking the user for any values, so this eval does not enforce the new stop-and-ask behavior. Assert a request for each missing value, or define explicit structured markers for the required questions.
        assistant_text = _assistant_text(state).casefold()
        missing_terms = [
            term
            for term in ("provider", "model", "secret")
            if term not in assistant_text
        ]
        if missing_terms:
            return Score(
                value=0.0,
                explanation=(
                    "clarification did not identify missing "
                    f"configuration: {missing_terms}"
                ),
            )
        if not any(
            marker in assistant_text
            for marker in ("ask", "need", "missing", "provide")
        ):

evals/skills/camunda-ai-agents/outcomes.py:329

  • The skill now documents provider-specific authentication and that providers can require multiple secrets, but the only positive sample is OpenAI and this scorer hardcodes one authentication.apiKey target. A regression that omits Bedrock credentials/region or another provider's additional secret references would remain green; add a multi-secret provider sample or make the expected authentication targets part of the sample metadata.
            "7. Use the OpenAI provider with model 'gpt-4.1-mini' and the "
            "already-configured connector secret 'OPENAI_API_KEY'; do not "
            "invent another provider or secret name.\n"

skills/camunda-ai-agents/SKILL.md:35

  • Using “did not specify any of these” makes this an all-or-nothing check. If the user supplies a provider but omits the model or secret name, this condition is false, so the skill is no longer instructed to stop and ask for those missing values—the guessing behavior this change is meant to prevent can still occur. Check whether any required value is missing instead.
If the user did not specify any of these, ask for the missing values and stop before applying the template or writing provider configuration. Do not default to a provider or model, and do not invent names such as `ANTHROPIC_API_KEY`. Ask for secret names, not secret values, and never put secret material in the BPMN or in the conversation.
  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread skills/camunda-ai-agents/SKILL.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Four unresolved moderate findings remain in the outcome validation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

evals/skills/camunda-ai-agents/outcomes.py:334

  • The presence checks only require that the assistant asks a question containing each term; they do not reject choosing a fallback in the same response. For example, “Which provider, model, and connector secret name should I use? If you do not answer, I’ll use Anthropic and ANTHROPIC_API_KEY” passes this scorer despite the sample and skill explicitly forbidding defaults and invented names. Add an assertion that no provider/model/secret fallback is selected while configuration is missing, or use structured clarification fields that cannot encode a default.
        assistant_text = _assistant_text(state).casefold()
        request_sentences = re.split(r"(?<=[?.!])\s+|\n+", assistant_text)
        missing_terms = []
        for term, pattern in (
            ("provider", r"\bprovider\b"),
            ("model", r"\bmodel\b"),
            ("connector-secret name", r"\b(?:connector[- ]secret|secret) names?\b"),
        ):
            if not any(
                _is_request_sentence(sentence) and bool(re.search(pattern, sentence))
                for sentence in request_sentences
            ):
                missing_terms.append(term)

evals/skills/camunda-ai-agents/outcomes.py:95

  • _is_request_sentence treats any occurrence of words such as provide, identify, or share as a request, so a declarative answer like “I can provide the provider, model, and connector-secret names” can pass without asking the user. It also rejects natural clarification such as “I need your provider, model, and connector-secret names” because need is not a marker. This can both admit unsafe behavior and fail valid responses; use explicit request forms or structured clarification fields instead of arbitrary word presence.
def _is_request_sentence(sentence: str) -> bool:
    return "?" in sentence or any(
        re.search(rf"\b{re.escape(marker)}\b", sentence)
        for marker in REQUEST_MARKERS
    )

evals/skills/camunda-ai-agents/outcomes.py:329

  • This pattern only recognizes the literal forms secret name/secret names, so valid questions such as “Which connector secret should I use?” or “What are the names of the connector secrets?” fail the guard. Conversely, it does not require the requested name to be existing/already configured, so a response asking for a name to invent can pass even though the skill contract requires an existing secret name. Broaden the accepted clarification forms and separately require existing/configured semantics.
        for term, pattern in (
            ("provider", r"\bprovider\b"),
            ("model", r"\bmodel\b"),
            ("connector-secret name", r"\b(?:connector[- ]secret|secret) names?\b"),
        ):
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Reject credential-value requests and fallback configuration while accepting natural requests for existing connector-secret names.

Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Six moderate findings remain in the outcome evaluator.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

evals/skills/camunda-ai-agents/outcomes.py:76

  • SECRET_MATERIAL_PATTERN does not recognize the common secret key/secret-key wording. A response such as “provide the provider, model, existing connector-secret name, and the provider secret key” satisfies every positive check and scores 1.0, even though it asks the user to disclose secret material. Add this form to the material detector (while retaining the existing ... key name exemption).
SECRET_MATERIAL_PATTERN = re.compile(
    r"\b(?:"
    r"secret\s+values?|secret\s+material|api\s+keys?|access\s+keys?|"
    r"tokens?|credentials?|passwords?|private\s+keys?"

evals/skills/camunda-ai-agents/outcomes.py:141

  • This checks for a negation anywhere in the preceding 60 characters, rather than in the clause containing the matched term. Consequently, a response like “Do not provide an API key, but please provide the provider, model, existing connector-secret name, and an API key” can have both API-key matches treated as negated and score successfully; the same logic also misses positive/negative boundaries for fallback actions. Limit negation to the relevant clause (and account for forms such as “no API key”) before using it to suppress a violation.
def _is_negated_term(sentence: str, start: int) -> bool:
    return bool(NEGATED_TERM_PREFIX_PATTERN.search(sentence[:start]))

evals/skills/camunda-ai-agents/outcomes.py:423

  • The configuration check searches a three-sentence window and treats generic words such as available as proof that the secret is already configured. For example, Please provide the provider, model, and connector-secret name. The available tools ... passes without asking for an existing secret name. Bind the qualifier to the secret-name phrase or require it in the same clause so this guard enforces the confirmed-secret requirement.
                    term != "connector-secret name"
                    or _has_secret_configuration_semantics(request_contexts[index])
                )
                for index, sentence in enumerate(request_sentences)

evals/skills/camunda-ai-agents/outcomes.py:77

  • The material detector still misses indirect requests such as Please provide the value of the existing connector secret. If that appears alongside provider/model/name requests, _requests_secret_material returns false and the guard accepts a request for secret material, contrary to the skill's clarification rule. Extend the detector to cover value/contents-of-secret phrasing.
SECRET_MATERIAL_PATTERN = re.compile(
    r"\b(?:"
    r"secret\s+values?|secret\s+material|api\s+keys?|access\s+keys?|"
    r"tokens?|credentials?|passwords?|private\s+keys?"
    r")\b"

evals/skills/camunda-ai-agents/outcomes.py:81

  • NEGATED_TERM_PREFIX_PATTERN omits cannot and can't, so a compliant clarification such as I can't choose a default provider; please provide ... is classified as fallback selection because choose and default match. Add these negated forms so the guard preserves the explicit refusal to choose a default.
NEGATED_TERM_PREFIX_PATTERN = re.compile(
    r"\b(?:do not|don't|never|not|without|rather than|instead of|"
    r"will not|won't|should not|shouldn't)\b[^,;:?.!\n]{0,60}$"
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread evals/skills/camunda-ai-agents/outcomes.py
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py Outdated
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread skills/camunda-ai-agents/SKILL.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved evaluator validation issues, including secret-file handling and clarification/fallback detection, must be addressed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

evals/skills/camunda-ai-agents/outcomes.py:592

  • When no provider/model/secret target is present, _has_concrete_token treats any non-generic noun as a concrete configuration choice. Thus a compliant clarification such as “I’ll use the local c8run profile; please provide the provider, exact model identifier, and existing connector-secret name” sees local/c8run after use and is rejected as a fallback. Restrict this path to recognized configuration values or clauses tied to a provider/model/secret target so ordinary cluster/profile acknowledgements do not fail the guard.
    if not target_matches:
        return _has_concrete_token(_configuration_fragment(action_tail))

evals/skills/camunda-ai-agents/outcomes.py:442

  • SECRET_MATERIAL_PATTERN matches API key before the suffix check, but the exemption only recognizes a following whitespace-separated name. A compliant request such as “Please provide the API key's name” is therefore classified as requesting secret material; accept possessive name-only suffixes before rejecting credential requests.
        for match in SECRET_MATERIAL_PATTERN.finditer(clause):
            if re.match(r"\s+names?\b", clause[match.end() :]):
                continue

evals/skills/camunda-ai-agents/outcomes.py:48

  • The new guard is driven by a large set of interdependent regex and context helpers, but this skill has no adjacent test_*.py covering them. CI runs pytest evals/skills, and similar outcome modules have focused unit tests; add deterministic cases for valid name-only requests, negated credential wording, and fallback/profile acknowledgements so scoring regressions are caught without an LLM run.
REQUEST_ACTION_PATTERN = (
    r"(?:ask|provide|specify|confirm|tell me|let me know|identify|indicate|share|supply)"
)
REQUEST_VERB_PATTERN = re.compile(
    rf"^(?:(?:please|kindly|also|now|just|then)\s+)*"

evals/skills/camunda-ai-agents/outcomes.py:123

  • The standalone defaults? token makes this reject unrelated defaults. The missing-configuration sample explicitly names the default c8ctl profile, so a compliant reply such as “I’ll use the default c8ctl profile; please provide …” is scored 0 even though no provider/model/secret fallback was selected. Scope this token to provider/model/secret configuration instead of matching every default.
    r"defaults?|"

evals/skills/camunda-ai-agents/outcomes.py:724

  • Appending every previous non-list sentence to the current context lets a request for one field authorize later mentions of other fields. For example, Please provide the provider. The exact model identifier is unspecified. The existing connector-secret name is required. can satisfy all three checks even though only the provider was requested, so this guard may return 1.0 without asking for the model or secret name. Carry only an actual request lead or otherwise associate the request with the term being checked.
            if previous_context and not is_list_item:
                contexts.append(f"{previous_context} {context}")

evals/skills/camunda-ai-agents/outcomes.py:315

  • This provider-specific detector misses a common valid clarification such as Which LLM provider would you like to use?: the prefix before provider is which llm, and the suffix is would you like to use, so neither narrow branch matches even though the sentence is an explicit question. The missing-configuration sample would therefore fail a compliant response; recognize question/request context around qualified provider phrases rather than requiring which provider or would I use verbatim.
def _contains_requested_provider(text: str) -> bool:
    for clause in _split_clauses(text.casefold()):
        if not _is_request_sentence(clause):
            continue
        for provider in re.finditer(r"\bprovider\b", clause):
            prefix = clause[: provider.start()]
            qualifier = re.search(
                r"\b(?:for|of|about|with|using|from)\s+"
                r"(?:the|an?|your|their|this|that)?\s*$",
                prefix,
            )
            if qualifier and not re.search(
                r"\bask(?:\s+(?:you|me))?\s+for\s+"
                r"(?:the|an?|your|their|this|that)?\s*$",
                prefix,
            ):
                continue
            if (
                re.search(
                    rf"\b(?:{REQUEST_ACTION_PATTERN}|"
                    r"ask(?:\s+(?:you|me))?\s+for)\b[^.?!\n]*$",
                    prefix,
                )
                or re.search(
                    r"\b(?:i|we)(?:['’](?:ll|d))?\s+"
                    r"(?:need|require)\b[^.?!\n]*$",
                    prefix,
                )
                or re.search(r"\b(?:which|what)\s+(?:the\s+)?$", prefix)
            ):
                return True
            if re.match(
                r"\s+(?:should|would|could|can|do)\s+i\s+use\b",
                clause[provider.end() :],
            ):
                return True
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Add deterministic checks for secret-file reads, names-only requests, profile acknowledgements, fallback continuations, and qualified provider questions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical secret-access guard gaps and unresolved parser, clarification, and SaaS-boundary evaluation issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

evals/skills/camunda-ai-agents/outcomes.py:322

  • _is_request_sentence only recognizes direct imperatives/questions and first-person I/we need forms. A normal compliant clarification such as “The user needs to provide the provider, exact model identifier, and existing connector-secret name” is therefore treated as containing no requests and scores 0. Broaden the request detection and add a regression test for third-person/impersonal wording.
    return any(
        REQUEST_VERB_PATTERN.search(clause.strip())
        or REQUEST_PREAMBLE_PATTERN.search(clause)
        or REQUEST_NEED_PATTERN.search(clause)
        or re.search(r"\b(?:could|can|would)\s+you\b", clause)

evals/skills/camunda-ai-agents/outcomes.py:798

  • The synthesized context still contains the lead's trailing :. _split_clauses() therefore splits please provide: - the provider before the list item, and _is_request_sentence() sees only - the provider without a request verb; the new test_associates_request_lead_with_markdown_list_items consequently fails for the provider, model, and secret checks. Strip the lead's trailing clause punctuation before concatenating it with the list item (or preserve the lead through parsing).
                f"{request_lead} {normalized}"

evals/skills/camunda-ai-agents/outcomes.py:677

  • This treats ordinary clarification wording as a concrete fallback selection. For example, in Please provide the exact model identifier you want to use, the token want is considered concrete, so _has_fallback_selection rejects an otherwise compliant request for the missing model. Restrict this check to actual provider/model/secret values (or exclude request-intent tails) and add a regression test for this phrasing.
        if _has_concrete_token(_configuration_fragment(action_tail[target.end() :])):
            return True

evals/skills/camunda-ai-agents/outcomes.py:525

  • SECRET_MATERIAL_PATTERN classifies API key as secret material unless the words name immediately follow it. Consequently, a compliant clarification such as Please provide the name of the existing API key is rejected, and SECRET_NAME_PATTERN does not recognize that non-possessive API-key-name form. Handle name of the API key/equivalent name wording before applying the material check, with a regression test.
        for match in SECRET_MATERIAL_PATTERN.finditer(clause):
            if re.match(r"(?:\s+|['’]s\s+)names?\b", clause[match.end() :]):
                continue
            if not _is_negated_term(clause, match.start()):
                return True

skills/camunda-ai-agents/SKILL.md:39

  • The SaaS-specific Console-only secret boundary is introduced here, but the new behavioral eval only exercises a local c8run prompt and checks generic clarification terms/no artifacts. It would still pass if the assistant omitted or misstated the SaaS constraint. Add a SaaS-targeted sample or transcript assertion so this required guidance is regression-tested.
For Camunda 8 SaaS, connector secrets are managed in Camunda Console, not created or populated through c8ctl. Surface this constraint before deployment and ask the user to confirm that the user-provided secret names already exist in the target cluster. For local c8run, follow **camunda-c8ctl** without reading real secret values.
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Peter Bojtos <peter.bojtos@camunda.com>
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread evals/skills/camunda-ai-agents/outcomes.py
Comment thread skills/camunda-ai-agents/SKILL.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Validation gaps remain, and the new SaaS sample is missing from the baseline.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

evals/skills/camunda-ai-agents/outcomes.py:46

  • A compliant clarification such as "Please select a provider, exact model identifier, and existing connector-secret name" is rejected here: _is_request_sentence uses this pattern for imperative requests, but choose/select/pick are absent, so without a question mark or provide/need wording the new guard reports the requested terms as missing. Include the common selection verbs and add a regression case.
REQUEST_ACTION_PATTERN = (
    r"(?:ask|provide|specify|confirm|tell me|let me know|identify|indicate|share|supply)"

evals/skills/camunda-ai-agents/outcomes.py:577

  • This helper treats any request containing connector-secret name as a request for the exact name. For example, Please confirm the existing connector-secret name is already configured. satisfies both checks even though it asks only for an existence confirmation and never asks the user to provide the name, so the missing-configuration eval can pass without the required secret identifier. Distinguish name-providing/which-name questions from confirmation-only wording.
def _has_secret_name_request_semantics(text: str) -> bool:
    return _contains_requested_term(text, SECRET_NAME_PATTERN)

evals/skills/camunda-ai-agents/outcomes.py:301

  • SECRET_SEARCH_OPERATION_PATTERN is applied to the tool name itself, so every grep/rg call is treated as a secret read unless its arguments happen to match an allowlisted names-only source. That blocks legitimate searches of skill, documentation, or BPMN content and is broader than the stated policy of forbidding real secret-file/value reads. Restrict this branch and the per-segment check to commands whose path or query is actually secret-sensitive.
    if (
        SECRET_ENV_READ_PATTERN.search(function)
        or SECRET_SEARCH_OPERATION_PATTERN.search(function)
    ) and not _is_approved_secret_source(source):
        return True

evals/skills/camunda-ai-agents/outcomes.py:736

  • This treats any recognized provider/model token in the whole action tail as a concrete selection, even when it is only an example. A valid clarification such as “Please provide a provider (for example, OpenAI), exact model identifier, and existing secret name” is therefore classified as fallback configuration. Exclude example clauses from the selection test and add a regression case for this wording.
    if CONFIGURATION_VALUE_PATTERN.search(action_tail):
        return True
  • Files reviewed: 4/4 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment on lines +145 to +151
SECRET_READ_OPERATION_PATTERN = re.compile(
r"(?:"
r"\b(?:cat|head|tail|less|more|sed|awk|grep|rg|cut|sort|strings|source)\b|"
r"\b(?:open|read_text|read_bytes|load_dotenv|dotenv_values)\s*\(|"
r"(?:^|[\s;&|])\.\s+"
r")"
)
Comment on lines +152 to +154
SECRET_ENV_READ_PATTERN = re.compile(
r"(?:^|[\s;&|])(?:printenv|env|set|export\s+-p|declare\s+-p)\b"
)
"""Require clarification before creating an agent with missing configuration."""

async def score(state: TaskState, target: Target) -> Score:
if state.sample_id != MISSING_CONFIGURATION_SAMPLE_ID:
Comment on lines +16 to +25
"missing-provider-configuration": {
"tokens": {
"input": 4,
"cache_write": 9023,
"cache_read": 16503,
"output": 458
},
"turns": 2,
"tool_calls": 2,
"duration_s": 26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Unresolved evaluation guard, secret-detection, artifact-coverage, and baseline issues remain.

Review details

Suppressed comments (8)

evals/skills/camunda-ai-agents/outcomes.py:1133

  • This guard is scoped only to MISSING_CONFIGURATION_SAMPLE_ID, but the SaaS sample below also deliberately omits the provider, model, and secret name and requires stopping before configuration. saas_secret_boundary_guidance checks only the Console/c8ctl wording and a secret-name mention, so a response that skips provider/model (or selects them) can still pass the SaaS sample. Include the SaaS sample in the shared missing-configuration, fallback, and secret-material checks.
        if state.sample_id != MISSING_CONFIGURATION_SAMPLE_ID:

evals/skills/camunda-ai-agents/outcomes.py:736

  • This treats any provider/model-like token in the request tail as a concrete fallback selection, including examples or an option list. A compliant clarification such as “Please provide the provider (for example, OpenAI) and exact model identifier (for example, gpt-4.1-mini)” will therefore make missing_configuration_guard fail even though no value was selected. Distinguish explicit selection/configuration from examples and add an end-to-end test for that wording.
    action_tail = CONFIRMATION_PHRASE_PATTERN.sub("", clause[action.end() :])
    target_matches = list(CONFIGURATION_TARGET_PATTERN.finditer(action_tail))

    if CONFIGURATION_VALUE_PATTERN.search(action_tail):
        return True

evals/skills/camunda-ai-agents/outcomes.py:1155

  • The artifact check excludes /workspace/skills/*, but the sample explicitly requires that no BPMN artifact be created anywhere under /workspace. A write under that directory can evade this check (and the artifact collector uses the same exclusion), so the missing-configuration eval can pass after creating a BPMN file in the excluded subtree.
                "-path",
                f"{workspace}/skills/*",
            ],

evals/skills/camunda-ai-agents/outcomes.py:287

  • The names-only and example-file exceptions are based solely on the filename, so cat approved-secret-names.txt or grep SECRET_ credentials.example is accepted without any evidence that the file contains only names/placeholders. That lets a value-bearing file bypass the no-secret-material guard; require an explicitly trusted names-only source rather than treating arbitrary suffixes as safe.
def _is_approved_secret_source(text: str) -> bool:
    return bool(
        SECRET_EXAMPLE_PATH_PATTERN.search(text)
        or SECRET_NAMES_ONLY_PATH_PATTERN.search(text)
        or NAMES_ONLY_SECRET_SOURCE_PATTERN.search(text)

evals/skills/camunda-ai-agents/outcomes.py:154

  • The environment-read check misses interpreter APIs. A permitted bash call such as python -c 'import os; print(os.environ["SECRET_OPENAI_API_KEY"])' does not match printenv|env|..., contains no secret-file path, and _prohibited_configuration_action therefore returns None, so the missing/SaaS samples can expose secret material before configuration. Detect common interpreter environment access and add a regression test rather than only matching shell built-ins.
SECRET_ENV_READ_PATTERN = re.compile(
    r"(?:^|[\s;&|])(?:printenv|env|set|export\s+-p|declare\s+-p)\b"
)

evals/skills/camunda-ai-agents/outcomes.py:846

  • _CONFIGURATION_TOKEN_PATTERN keeps hyphenated phrases as one token, while the generic set contains only separate user and selected entries. A valid response such as “I can proceed with the user-selected provider after you confirm the exact model identifier and existing connector-secret name” is then treated as a concrete fallback by _has_fallback_selection, so the missing-configuration scorer can reject an agent that merely defers to the user's choice. Normalize such compounds or add the compound terms, with a regression test.
    for token in tokens[:6]:
        normalized = token.group().rstrip(".,;:!?")
        if normalized not in _GENERIC_CONFIGURATION_TOKENS:
            return True

evals/skills/camunda-ai-agents/outcomes.py:614

  • In a compliant response like I won't choose a default provider or invent a secret name, invent is treated as an un-negated fallback action: the between text contains or, which this separator regex ignores. _has_fallback_selection therefore rejects the missing-configuration response. Treat or as a boundary in the negation check and add this regression case.
    if re.search(r"[,;:]|\b(?:and|but|if|when|unless)\b", between):
        return False

evals/skills/camunda-ai-agents/outcomes_baseline.json:16

  • The new saas-secret-boundary sample is absent from this baseline. pass_fail.py treats a missing sample baseline as a warning while leaving the cost gate passed, so this new path has no input/output ceiling. Regenerate the baseline from an all-green run after adding the sample.
      "missing-provider-configuration": {
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

camunda-ai-agents: skill picks an LLM provider and invents a secret name instead of asking the user

2 participants