Skip to content

Get rid of run command and convert all scenarios to use import#2679

Merged
mergify[bot] merged 26 commits intomainfrom
feature/kill-run
Jan 28, 2026
Merged

Get rid of run command and convert all scenarios to use import#2679
mergify[bot] merged 26 commits intomainfrom
feature/kill-run

Conversation

@byorgey
Copy link
Member

@byorgey byorgey commented Jan 24, 2026

In this PR:

  • Get rid of the run command (good riddance!)
  • Get rid of IO from the runtime engine, and introduce new effects for the things we were still using IO for: (1) mutable global cache and (2) getting the current time
  • Split up Language.Syntax.Import into multiple modules
  • Rename getDataDirSafe to getDataDirThrow since it actually throws an exception, and create an actually safe variant
  • Add syntax ~swarm as an import anchor to refer to things in the Swarm data dir, so you can write e.g. import "~swarm/lib/control" in order to import data/lib/control.sw from wherever it got installed
  • Converts all scenarios to use import:
    • Move a bunch of repeated code into the beginnings of a standard library
    • Typically, if we had run "foo.sw" and foo.sw contained def ...; def ...; go we remove go from the end of foo.sw and change run "foo.sw" to import "foo"; go

Closes #495. Closes #328. Closes #2068. Closes #2182. Closes #1560. Closes #2602.

@byorgey byorgey requested a review from xsebek January 25, 2026 00:17
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

Wow, this is awesome! 👍 And you even went ahead and got rid of the IO! 🚀

@byorgey
Copy link
Member Author

byorgey commented Jan 25, 2026

Currently the tournament-host test suite is failing. It used to work by first testing that it could upload arbitrage.yaml as a scenario, then separately testing that it could upload _arbitrage/solution.sw as a solution to the previously uploaded scenario. Previously, arbitrage.yaml contained something like

solution: |
  run "_arbitrage/solution.sw"

and validating the scenario description did not involve loading solution.sw at all, since the run is not interpreted until runtime. Now, however. arbitrage.yaml contains

solution: |
  import "_arbitrage/solution"; solution

and in order to validate the scenario we must typecheck the solution, which includes recursively loading _arbitrage/solution.sw. Predictably this failed when it could not find the file. I tried modifying the test to upload both arbitrage.yaml and the solution, but it still can't find the right file. It seems that it is not even looking for it in the right place, since it complains about not being able to find swarm/_arbitrage/solution. This is what happens when it doesn't know where it is loading the .yaml file from so it can't figure out the right place to load any relative imports from. I don't yet understand what's going on well enough to know how to fix this.

Edited to add: it seems like the gamestateFromScenarioText function here:

gamestateFromScenarioText ::
LBS.ByteString ->
ExceptT ScenarioInstantiationFailure IO (GameState, Scenario Elaborated)

already just takes the content of a .yaml file as input; then it in turn calls initScenarioObject:
scenarioObject <- initScenarioObject scenarioInputs content

which in turn is forced to call parseJSONE' Nothing:
parseEither (parseJSONE' Nothing scenarioInputs) rawYaml

The first argument to parseJSONE' is supposed to be the provenance of the file being parsed, but at this point it is too late, we only have the file's contents.

Edited to add 2: even if I figured out the above issue, I don't think this is going to work... the scenario validation calls processSource, which assumes it will be able to do regular file I/O to recursively load any imports; but in the context of the tournament server it seems like there's some kind of persistence layer providing access to uploaded files, so just doing regular file I/O will not work (I think). If I'm right about that, I suppose the real solution would be to create some kind of filesystem effect with two handlers, one to run filesystem commands via normal IO and one to run it via a provided persistence layer.

@kostmo any thoughts on this?

Edited to add 3: Just commenting out the tournament-host test suite for now: #2681 . Another possibility that was discussed was inlining the solution into the scenario description so that it could be processed without loading an import. However, doing this on the real arbitrage scenario itself would mean that the solution is now duplicated between being inlined in the scenario description and the solution.sw file, which I don't want to do. Alternatively we could copy the scenario to a special location which would only be used for testing the tournament server, but then the tournament server test would not be using a "real" scenario which is guaranteed to be up-to-date. Simply commenting out the test for now seems like the most straightforward and honest thing to do.

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Jan 28, 2026
@mergify mergify bot added the queued label Jan 28, 2026
@mergify mergify bot merged commit 281fe1e into main Jan 28, 2026
17 checks passed
@mergify
Copy link
Contributor

mergify bot commented Jan 28, 2026

Merge Queue Status

✅ The pull request has been merged at 946e4bf

This pull request spent 5 seconds in the queue, with no time running CI.
The checks were run in-place.

Required conditions to merge

@mergify mergify bot deleted the feature/kill-run branch January 28, 2026 03:07
@mergify mergify bot removed the queued label Jan 28, 2026
byorgey added a commit that referenced this pull request Mar 6, 2026
Fixes #2227.  Partially reverts #1877.

The motivation for #1877 was largely because of errors being shown by
the `run` command, when the `.sw` file being loaded did not parse or
type check.  Such errors could be quite large, and were irrelevant
once the `.sw` file had been fixed, so making them "ephemeral" worked
very well.  However, with #2540 and #2679 `run` has now been replaced
by `import`, which generates not *runtime* errors but *compile-time*
errors.  Such errors are shown in the REPL console rather than a robot
log, so the original motivation for #1877 is now moot.  The remaining
runtime errors tend to be short (*e.g* "move: There is a mountain in the
way!") and potentially relevant to see in the log history, so making
them ephemeral does not make as much sense.
mergify bot pushed a commit that referenced this pull request Mar 6, 2026
#2695)

Fixes #2227.  Partially reverts #1877.

The motivation for #1877 was largely because of errors being shown by the `run` command, when the `.sw` file being loaded did not parse or type check.  Such errors could be quite large, and were irrelevant once the `.sw` file had been fixed, so making them "ephemeral" worked very well.  However, with #2540 and #2679 `run` has now been replaced by `import`, which generates not *runtime* errors but *compile-time* errors.  Such errors are shown in the REPL console rather than a robot log, so the original motivation for #1877 is now moot.  The remaining runtime errors tend to be short (*e.g* "move: There is a mountain in the way!") and potentially relevant to see in the log history, so making them ephemeral does not make as much sense.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge me Trigger the merge process of the Pull request.

Projects

None yet

2 participants