Skip to content

♻️ The Rulekeeper - Index from one workflow that starts the Azure job - #2800

Merged
AntPolkanov merged 5 commits into
mainfrom
ap/rulekeeper-one-index-workflow
Nov 13, 2026
Merged

AntPolkanov merged 5 commits into
mainfrom
ap/rulekeeper-one-index-workflow

Conversation

@AntPolkanov

@AntPolkanov AntPolkanov commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Description

✏️
We had two workflows for indexing. "Re-index Rules for The Rulekeeper" started the Azure index job. "Index Rules for The Rulekeeper" ran the indexer on a GitHub runner, signed in as the SQL administrator, and had the re-embed and removal options. The job can take those options itself, so one workflow is enough.

  • "Index Rules for The Rulekeeper" now starts the Azure index job. With "Re-embed" or "Allow removals" ticked, it passes --reembed or --allow-removals to the job.
  • I deleted the runner version and rules-chat-reindex-trigger.yml.
  • az containerapp job start --args sends a container with only the arguments. So the workflow copies the job's own container and adds the arguments. It copies only the name, image, settings and resources, because the job also carries fields newer than the request's API version, such as imageType.
  • The workflow now waits for the run and fails when it ends Failed, Stopped or Degraded. A wait can outlast the Azure login, so when reading the status fails it logs in again with a new GitHub token and tries on the next minute. It fails only after three failed reads in a row.
  • I removed the deploy pipeline's Cognitive Services OpenAI User role on Foundry. Only the runner workflow used it.
  • I updated infra/README.md and the lock comments.

A run that waits 30 minutes for the index lock and gives up still ends Succeeded, so the workflow shows green even though nothing was indexed. I left the index script as it is in this PR and opened #2809 for it.

After merging, someone needs to delete the pipeline's existing role assignment on aif-sswrules-chat-staging and aif-sswrules-chat-prod. The deploy leaves role assignments it no longer declares in place.

Screenshot (optional)

✏️
Not applicable. This change has no UI.

Validation

  • actionlint and az bicep build passed, including on the wait step.
  • pnpm lint passed.
  • I ran the wait loop locally with stubbed status reads. Two failures then a success passed, a good read reset the count, three failures in a row failed, and a Failed run failed.
  • I built the start request from the real staging job. It has the name, image, all seven settings, the resources and the arguments, and nothing else.
  • I haven't started a run against Azure. After merging, I'll run these on staging and check that each workflow waits and then goes green:
    • gh workflow run rules-chat-index.yml -f environment=staging -f allow_removals=true takes a few minutes and covers starting the run with arguments.
    • gh workflow run rules-chat-index.yml -f environment=staging -f reembed=true re-embeds every rule, so it runs long enough to need the second login. It also pays to embed every rule again.

Index Rules for The Rulekeeper now starts the Container Apps index job, and
passes --reembed and --allow-removals to it when they are ticked. Re-index
Rules for The Rulekeeper did the same without the options, and the old Index
Rules ran the indexer on a GitHub runner as the SQL administrator. Both are
gone.
Only the GitHub runner index workflow used the pipeline's Cognitive Services
OpenAI User role on Foundry, and that workflow is gone.
The workflow returned as soon as Azure accepted the run, so a failed re-embed
still showed green. It now waits for the execution and fails on Failed,
Stopped or Degraded. A wait can outlast the Azure CLI's login, so it logs in
again with a new GitHub token when reading the status fails.
One failed status read after logging in again ended the step, even while the
Azure run went on. The wait now logs in again and tries on the next tick, and
fails only after three failed reads in a row. The workflow job is named Index
Rules, since it now waits for the run too.
@AntPolkanov
AntPolkanov marked this pull request as ready for review November 12, 2026 06:44
@AntPolkanov
AntPolkanov enabled auto-merge (squash) November 12, 2026 06:44

@bradystroud bradystroud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The single workflow is a good simplification. Two things need attention before merge.

Blocking

1. The retry logic does not match the description.
The description says the wait step "fails only after three failed reads in a row" and that a counter was tested. No pushed commit (a6cbeb4, 7132d62, f47e32b) has a counter. The step runs as bash -eo pipefail (because of shell: bash). After one failed read it logs in again and reads once more with STATUS=$(status) at line 113. If that read also fails, set -e stops the step. One short ARM error after the new login fails the workflow while the job keeps running. Please push the counter, or correct the description if one retry is the intent.

2. The staging test runs have not happened.
The description asks for two staging runs before merge, and no rules-chat-index.yml run exists on ap/rulekeeper-one-index-workflow. The az rest .../start call sends the job's full containers object back, possibly with fields such as probes or volumeMounts. That is the part most likely to fail, and only these runs will show it. Please link the two green runs here.

Non-blocking

  • A run that gives up on the lock still ends Succeeded, so the workflow shows green with nothing indexed. This is most likely during a re-embed, when someone is watching the result. A follow-up could make the script exit with a non-zero code in that case.
  • This PR deletes the runner workflow, which was the fallback when the job image itself is broken. That is probably OK, but the team should agree to it.
  • After merge, remember to delete the pipeline's old OpenAI User role assignment on aif-sswrules-chat-staging and aif-sswrules-chat-prod.

Checked and OK

  • REEMBED and ALLOW_REMOVALS go through env:, not ${{ }} in run:.
  • run.sh forwards "$@", so the container args reach index-rules.mjs.
  • The new login uses the ACTIONS_ID_TOKEN_REQUEST_* variables that id-token: write makes available.
  • Nothing else uses the pipeline's Foundry role, and servicePrincipalObjectId is still used for AcrPush.

The start request copied the job's whole container. The job carries fields
newer than the request's API version, such as imageType, which Azure may
reject. The request now sends the container's name, image, settings and
resources, plus the arguments.
@AntPolkanov

AntPolkanov commented Nov 13, 2026 •

Copy link
Copy Markdown
Member Author

Thanks, Brady.

  1. Retry counter. You reviewed f47e32b, because GitHub hadn't picked up my next push yet. The counter is in 6846faf. If reading the status fails, the step logs in again and tries a minute later. It only fails after three failed reads in a row. The PR shows that commit now.

  2. Staging runs before merging. I don't want to run unmerged code against staging, so I'll do both runs after the merge and post the links here. If they fail, only this manual workflow is broken. The hourly index job doesn't use it. You were right that sending the whole container was risky, though. The job has an imageType field, which is newer than the 2024-03-01 API version the request uses. In 6d452c6 the request sends only the name, image, settings, resources and arguments.

  3. Runs that give up on the lock but end Succeeded. I opened 🐛 The Rulekeeper - An index run that gives up on the lock ends Succeeded #2809 for that.

  4. Removing the runner fallback. If the job image breaks, the next deploy rebuilds it. The runner also signed in as the SQL admin, which is far more access than indexing needs. If the team wants the fallback back, I'm happy to talk about it.

  5. Old role assignments. They're in the description, and I'll delete them after the merge.

@bradystroud bradystroud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, Anton. Both blocking points are resolved.

  • Retry counter: 6846faf adds it, and it works under bash -eo pipefail. The $((…)) increment does not trip set -e, and each command in login_again has its own || return 1, because set -e does not apply inside the if.
  • Staging runs after merge: OK. If this workflow is broken, the hourly job is not affected, and Run now in the portal still works. Sending only name, image, env, resources and args (6d452c6) also removes the main risk I saw.

After merge, please:

  1. Post the links to the two staging runs (allow_removals=true and reembed=true) here.
  2. Delete the pipeline's old OpenAI User role assignment on aif-sswrules-chat-staging and aif-sswrules-chat-prod.

#2809 tracks the lock-timeout case.

@AntPolkanov
AntPolkanov merged commit c8982d0 into main Nov 13, 2026
1 check passed
@AntPolkanov
AntPolkanov deleted the ap/rulekeeper-one-index-workflow branch November 13, 2026 03:42

This branch was previously deployed

1 inactive deployment
staging — 6d452c66 Deployed Nov 13, 2026 by AntPolkanov via Delete PR Slot #205
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