fix(sinatra): persist handler @ivars across before/route/after + MRI-identity oracle vs sinatra gem 4.2.1

internal/vm/object_model.go

@@ -1560,6 +1560,11 @@ func ivarTable(self object.Value) map[string]object.Value {
 		return o.ivars
 	case *object.Main:
 		return o.IvarTable()
+	case *SinatraCtx:
+		if o.ivars == nil {
+			o.ivars = map[string]object.Value{}
+		}
+		return o.ivars
 	}
 	return nil
 }

internal/vm/sinatra.go

@@ -49,6 +49,7 @@ type SinatraCtx struct {
 	c        *sinatra.Context
 	cls      *RClass
 	settings map[string]object.Value // the app's merged set/enable/disable values, for the `settings` helper
+	ivars    map[string]object.Value // instance variables (@x) the handler sets; shared across a request's before/route/after (see sinatraAction)
 }
 
 func (c *SinatraCtx) ToS() string     { return "#<Sinatra::Base request>" }

internal/vm/sinatra_bind.go

@@ -118,6 +118,11 @@ func (vm *VM) registerSinatraDSL(base *RClass) {
 // returns the SPEC [status, headers, body] triple as a Ruby Array.
 func (vm *VM) sinatraCall(cls *RClass, envArg object.Value) object.Value {
 	app := vm.buildSinatraApp(cls)
+	// The per-request handler-self cache is scoped to this one dispatch: reset it
+	// before serving and drop it after so a request's before/route/after share one
+	// SinatraCtx (and its @ivars) without leaking across requests.
+	vm.sinatraCtxCache = nil
+	defer func() { vm.sinatraCtxCache = nil }()
 	status, headers, body := app.CallTuple(rackEnv(envArg))
 	return object.NewArray(object.IntValue(int64(status)), rackHeadersToHash(headers), rackBodyArray(body))
 }
@@ -185,15 +190,33 @@ func (vm *VM) sinatraRegisterRoute(app *sinatra.Sinatra, r sinatraRoute, merged
 // via the library's panic-based control flow, which the dispatcher recovers.
 func (vm *VM) sinatraAction(blk *Proc, merged map[string]object.Value) sinatra.Action {
 	return func(c *sinatra.Context) any {
-		ctx := &SinatraCtx{c: c, cls: vm.cSinatraCtx}
-		ctx.settings = merged
+		ctx := vm.sinatraCtxFor(c, merged)
 		if blk == nil {
 			return nil
 		}
 		return sinatraResult(vm, vm.callBlockSelf(blk, ctx, nil))
 	}
 }
 
+// sinatraCtxFor returns the handler self for the request identified by c, reusing
+// one SinatraCtx across that request's before filter(s), the route body and the
+// after filter(s) — real Sinatra runs the whole request against a single
+// instance, so instance variables an app sets in a `before` block (e.g. @user)
+// are visible in the route and after blocks. The cache is keyed by the library's
+// per-request *sinatra.Context (one per dispatch) and reset around each dispatch
+// by sinatraCall.
+func (vm *VM) sinatraCtxFor(c *sinatra.Context, merged map[string]object.Value) *SinatraCtx {
+	if vm.sinatraCtxCache == nil {
+		vm.sinatraCtxCache = map[*sinatra.Context]*SinatraCtx{}
+	}
+	if ctx, ok := vm.sinatraCtxCache[c]; ok {
+		return ctx
+	}
+	ctx := &SinatraCtx{c: c, cls: vm.cSinatraCtx, settings: merged, ivars: map[string]object.Value{}}
+	vm.sinatraCtxCache[c] = ctx
+	return ctx
+}
+
 // registerSinatraContext installs the request-context helper surface a handler
 // block runs against (self = SinatraCtx).
 func (vm *VM) registerSinatraContext(ctx *RClass) {

internal/vm/sinatra_internal_test.go

@@ -475,6 +475,55 @@ puts s`
 	}
 }
 
+// TestSinatraIvarPersistence covers the request-scoped handler self: an instance
+// variable set in a before filter is visible in the route body and in the after
+// filter (real Sinatra runs the whole request against one instance), and it does
+// NOT leak into the next request (a fresh SinatraCtx per dispatch).
+func TestSinatraIvarPersistence(t *testing.T) {
+	// @uid set in `before` is read by the route and by `after`.
+	src := `require "sinatra/base"
+class A < Sinatra::Base
+  before { @uid = "u42" }
+  after { headers "X-Uid" => @uid.to_s }
+  get("/x"){ "uid=#{@uid}" }
+end
+_,h,b = A.new.call("REQUEST_METHOD"=>"GET","PATH_INFO"=>"/x")
+puts b.join
+puts h["x-uid"]`
+	if got := eval(t, src); got != "uid=u42\nu42\n" {
+		t.Errorf("ivar persistence got=%q want %q", got, "uid=u42\nu42\n")
+	}
+
+	// A second request on the same app must not see the first request's ivar: the
+	// route reads @uid before any before filter would set it, so it is nil.
+	iso := `require "sinatra/base"
+class B < Sinatra::Base
+  get("/set"){ @uid = "first"; "set" }
+  get("/read"){ @uid.inspect }
+end
+app = B.new
+app.call("REQUEST_METHOD"=>"GET","PATH_INFO"=>"/set")
+_,_,b = app.call("REQUEST_METHOD"=>"GET","PATH_INFO"=>"/read")
+puts b.join`
+	if got := eval(t, iso); got != "nil\n" {
+		t.Errorf("ivar isolation got=%q want %q", got, "nil\n")
+	}
+}
+
+// TestSinatraCtxIvarTable covers ivarTable's *SinatraCtx arm, including the
+// lazy allocation of the backing map for a context whose ivars are still nil
+// (writing to a nil map would panic, so the lazy init is load-bearing).
+func TestSinatraCtxIvarTable(t *testing.T) {
+	c := &SinatraCtx{} // ivars nil
+	setIvar(c, "@k", object.NewString("v"))
+	if c.ivars == nil {
+		t.Fatal("ivarTable did not lazily allocate the ivars map")
+	}
+	if got := getIvar(c, "@k"); got == nil || got.ToS() != "v" {
+		t.Errorf("round-trip got=%v", got)
+	}
+}
+
 // TestSinatraParamsHashDirect exercises sinatraParamsHash against a library
 // context built directly, covering the []string (splat) value path.
 func TestSinatraParamsHashDirect(t *testing.T) {

internal/vm/vm.go

@@ -21,6 +21,7 @@ import (
 
 	i18n "github.com/go-ruby-i18n/i18n"
 	money "github.com/go-ruby-money/money"
+	sinatra "github.com/go-ruby-sinatra/sinatra"
 
 	"github.com/go-embedded-ruby/ruby/internal/bytecode"
 	"github.com/go-embedded-ruby/ruby/internal/object"
@@ -241,6 +242,7 @@ type VM struct {
 	cSinatraCtx                            *RClass                 // Sinatra::Base::Context, the self a route/filter block runs against
 	cSinatraSettings                       *RClass                 // Sinatra::Base::Settings, the handler's `settings` view
 	sinatraDefs                            map[*RClass]*sinatraDef // per-Sinatra::Base-subclass route/filter/handler declarations
+	sinatraCtxCache                        map[*sinatra.Context]*SinatraCtx // per-request handler self, shared across before/route/after so @ivars persist
 	cOpenSSLDigest                         *RClass
 	cArray, cHash, cRange                  *RClass
 	cProc                                  *RClass

scripts/conformance/webapp/apps/sinatra_oracle.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+#
+# Sinatra oracle app — the reference app whose response is diffed byte-for-byte
+# against the real `sinatra` gem 4.2.1. It exercises the routing/dispatch core
+# the go-ruby-sinatra binding re-exposes: a :name capture, a query param, a
+# before-filter instance variable that must survive into the route body (real
+# Sinatra runs before/route/after against ONE instance), a splat, an optional
+# param, redirect + Location, halt, a custom error(code) handler, a custom
+# not_found handler and an after-filter that mutates the response headers.
+#
+# The identical file runs unchanged under MRI + the sinatra gem; the expected
+# output in sinatra_oracle_test.go was captured from that run (gem 4.2.1,
+# ruby 4.0.5). To regenerate the golden vector, run this file with `ruby` and
+# the sinatra gem installed.
+#
+# Two response headers are excluded from the diff and tracked as go-ruby-sinatra
+# LIBRARY follow-ups (they are response-finalization details the pure-Go core
+# does not emit yet, not binding concerns):
+#   * Content-Length — the gem sets it; go-ruby-rack does not finalize it.
+#   * X-Cascade: pass — the gem adds it to every 404; go-ruby-sinatra does not.
+
+require "sinatra/base"
+
+class App < Sinatra::Base
+  disable :protection
+  set :host_authorization, { permitted_hosts: [] }
+  set :default_content_type, "text/html"
+
+  before { @uid = "u42" }
+  after  { headers "X-After" => "seen" }
+
+  get "/greet/:name" do
+    content_type :json
+    %({"hello":"#{params['name']}","uid":"#{@uid}","q":"#{params['q']}"})
+  end
+
+  get "/files/*" do
+    "splat=#{params['splat'].join('|')}"
+  end
+
+  get "/opt/:a/?:b?" do
+    "a=#{params['a']} b=#{params['b'].inspect}"
+  end
+
+  get "/go" do
+    redirect "/greet/there", 303
+  end
+
+  get "/boom" do
+    halt 418, "teapot"
+  end
+
+  get "/err" do
+    500
+  end
+  error(500) { "custom-error" }
+
+  not_found { "no-such-route" }
+end
+
+ENVS = [
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/greet/amy", "QUERY_STRING" => "q=1", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/files/a/b/c.txt", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/opt/x", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/go", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/boom", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/err", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+  { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/missing", "QUERY_STRING" => "", "rack.url_scheme" => "http", "HTTP_HOST" => "ex" },
+]
+
+ENVS.each_with_index do |env, i|
+  status, headers, body = App.call(env)
+  parts = []
+  body.each { |p| parts << p }
+  puts "== req #{i} =="
+  puts "status=#{status}"
+  headers.keys.sort.each do |k|
+    next if ["content-length", "x-cascade"].include?(k.downcase) # library follow-ups (see header comment)
+    puts "H #{k.downcase}: #{headers[k]}"
+  end
+  puts "body=#{parts.join}"
+end

scripts/conformance/webapp/doc.go

@@ -11,12 +11,17 @@
 //
 //	Stage 1  Pure Rack lambda                       — runs
 //	Stage 2  Rack + ERB view                        — runs
-//	Stage 3  Sinatra DSL (require "sinatra/base")   — gap: no sinatra binding
+//	Stage 3  Sinatra DSL (require "sinatra/base")   — runs (go-ruby-sinatra binding)
 //	Stage 4  sqlite3 + ERB/JSON data route          — runs
 //	Stage 4b ActiveRecord ORM data route            — gap until go-ruby-activerecord binds
 //
-// The Sinatra and ActiveRecord stages feature-detect the require first: they
-// assert the real response when the binding is present and otherwise record the
-// exact missing feature, so the suite stays green today and lights up
-// automatically when the binding lands.
+// On top of stage 3, TestSinatraGemOracle (sinatra_oracle_test.go) is the web
+// phase's MRI-identity proof: it runs apps/sinatra_oracle.rb through rbgo and
+// asserts the [status, headers, body] dump is byte-for-byte the response the
+// real `sinatra` gem 4.2.1 produced from the same app and request envs.
+//
+// The ActiveRecord stage feature-detects the require first: it asserts the real
+// response when the binding is present and otherwise records the exact missing
+// feature, so the suite stays green today and lights up automatically when the
+// binding lands.
 package webapp

scripts/conformance/webapp/sinatra_oracle_test.go

@@ -0,0 +1,68 @@
+package webapp
+
+import "testing"
+
+// sinatraGem421Golden is the response dump of apps/sinatra_oracle.rb captured
+// from the REAL `sinatra` gem 4.2.1 (MRI ruby 4.0.5): seven requests through the
+// classic Sinatra::Base #call(env) adapter, dumping [status, headers, body] with
+// the two known library-level headers excluded (Content-Length, X-Cascade — see
+// the fixture header comment). This is the golden oracle: rbgo must reproduce it
+// byte-for-byte. It is a captured constant so the suite stays hermetic (no ruby,
+// no gem, runs under qemu on every 64-bit target); regenerate by running the
+// fixture under MRI + the sinatra gem, per apps/sinatra_oracle.rb.
+const sinatraGem421Golden = `== req 0 ==
+status=200
+H content-type: application/json
+H x-after: seen
+body={"hello":"amy","uid":"u42","q":"1"}
+== req 1 ==
+status=200
+H content-type: text/html;charset=utf-8
+H x-after: seen
+body=splat=a/b/c.txt
+== req 2 ==
+status=200
+H content-type: text/html;charset=utf-8
+H x-after: seen
+body=a=x b=nil
+== req 3 ==
+status=303
+H content-type: text/html;charset=utf-8
+H location: http://ex/greet/there
+H x-after: seen
+body=
+== req 4 ==
+status=418
+H content-type: text/html;charset=utf-8
+H x-after: seen
+body=teapot
+== req 5 ==
+status=500
+H content-type: text/html;charset=utf-8
+H x-after: seen
+body=custom-error
+== req 6 ==
+status=404
+H content-type: text/html;charset=utf-8
+H x-after: seen
+body=no-such-route
+`
+
+// TestSinatraGemOracle proves the go-ruby-sinatra binding is MRI-identical
+// *through the rbgo binary*: it runs the same apps/sinatra_oracle.rb the sinatra
+// gem 4.2.1 ran and asserts the response is byte-for-byte the captured gem
+// output. This is the run-conformance headline for the web phase — routing, a
+// :name capture, a query param, before-filter instance-variable persistence into
+// the route body (before/route/after share one instance), a splat, an optional
+// param, redirect + Location, halt, a custom error(code) handler, a custom
+// not_found handler and an after-filter mutating the response — all answered by
+// rbgo exactly as by the reference gem.
+func TestSinatraGemOracle(t *testing.T) {
+	if ok, detail := featureLoadable("sinatra/base"); !ok {
+		t.Fatalf("sinatra/base must be loadable for the oracle: %s", detail)
+	}
+	out := mustRun(t, "sinatra_oracle.rb")
+	if out != sinatraGem421Golden {
+		t.Fatalf("rbgo output is not MRI-identical to sinatra gem 4.2.1\n got:\n%s\nwant:\n%s", out, sinatraGem421Golden)
+	}
+}