DEV Community: Akanksha Trehun

Week 16: The Editor That Learned When to Say No

Akanksha Trehun — Thu, 01 Oct 2026 12:29:03 +0000

Week 16: The Editor That Learned When to Say No

With the testbench model and the runner both fixed and moving, the test case
editor finally got its own review round this week. It found the same class
of bug the model found last week, in the client side JSON this time instead
of the server side validation, and fixing it properly meant redesigning a
piece of the editor rather than patching around the symptom. A second,
unrelated bug turned up in a completely different pull request, caught only
because a setting this phase just added made an old code path reachable for
the first time.

The overwrite that would have erased a title on every save

First finding, on Assignment#testbench_data=:

(testbench || build_testbench).data = parsed

This replaces the entire data hash with whatever the editor's JSON posts.
The editor only ever authors type and groups. If an existing suite had
any other top level key, a title, say, something the simulator itself reads
at testbench.js:872, it survives right up until the next time an instructor
edits an unrelated setting and saves the form. Then it is gone, not because
anyone touched it, but because the write path never preserved what it was not
explicitly given.

tb = testbench || build_testbench
tb.data = (tb.data || {}).merge(parsed)

Merge instead of replace. I wrote a regression test that seeds a suite with a
title, saves an unrelated change through the editor, and asserts the title
survives, then deleted the merge to confirm the test catches the loss. It
does.

The same binding bug, one layer up

The second finding was the model's groups[0] binding problem again, except
this time the mismatched pins would come from the editor itself, from an
instructor adding a second test case row with different inputs than the
first:

each row becomes its own group, but the simulator binds pins from
groups[0] only. a row with a different pin set than the first makes
group.outputs.find(...).results.push return undefined and throw.

Same root cause as last week, different code path producing the malformed
suite. The fix is a shape comparison in the JavaScript controller, mirroring
what the Ruby model already does server side:

sameShape(groups) {
    const shape = (group) => JSON.stringify(['inputs', 'outputs'].map((side) => group[side].map((p) => [p.label, p.bitWidth])));
    return groups.every((group) => shape(group) === shape(groups[0]));
}

Submission blocks client side now if any row's pins do not match the first
row's, with the same pattern used for incomplete rows: preventDefault,
stopPropagation because rails-ujs still disables the submit button on a
merely prevented submit as it bubbles to its document level listener, show
the error, stop.

A third finding, a values regex that accepted any non-empty string instead
of exact-width binary digits, was the exact bug from last week's model
review, just not yet enforced in the form that produces the JSON in the
first place. Tightened the same way: [01]+, checked against the declared
bit width.

Why I did not try to merge the unsupported case in

The harder finding was about suites the editor cannot fully represent:
multi vector test cases, where one row of the JSON actually encodes several
values per pin instead of one. My first fix tried to preserve these by
tracking each group's original array position, splitting edited groups from
untouched ones, and re-merging them back into the original order on save.
It worked, but CodeRabbit found two more bugs in that merge logic within
a day: the merge could still reorder an unsupported group relative to an
edited one, and it silently rewrote the suite's type from seq to comb
on every save because the serialization always hardcoded comb.

Two fixes for a problem that a different design does not have in the first
place made me stop and reconsider the approach rather than the code. The
editor cannot safely round trip a multi vector suite. Trying to merge it
back together precisely is solving a harder problem than the feature needs:
tracking position, preserving type, reconciling two sources of truth. The
simpler rule is to never let the editor touch what it cannot fully
represent. If any group in a suite has more than one value per pin, the
whole editor goes read only for that assignment, with a note saying why,
instead of rendering an editable interface that would corrupt the suite the
moment someone saved through it.

def test_case_editable?(assignment)
  test_case_groups(assignment).all? do |group|
    (group["n"] || 1) == 1 &&
      (group["inputs"] || []).all? { |pin| Array(pin["values"]).size <= 1 } &&
      (group["outputs"] || []).all? { |pin| Array(pin["values"]).size <= 1 }
  end
end

This deleted the position tracking, the merge-back logic, and every test
written against it, and replaced all of it with one predicate and an
if/else in the view. The line count dropped by about sixty lines for a
bigger guarantee: nothing this editor cannot represent can be silently
damaged by it, because it is never editable in the first place.

Getting back under budget without cutting a fix

This phase holds pull requests to roughly two hundred hand written lines.
Chasing every one of these findings, plus the accessibility fixes an
automated check flagged separately, pushed this diff past four hundred at
its peak. Cutting it back down meant finding the difference between a line
that is load bearing and a line that is just verbose.

Consolidating five near identical validation specs into one table driven
test recovered real lines without losing a single case. Dropping a helper
spec file for a follow up PR, per this phase's own stated rule about
splitting spec edge cases rather than thinning validation, recovered more.
The read only redesign recovered the most of all, because it deleted code
that existed to solve a problem a different design does not have. What did
not move: the testbench model's shape and label validation, the values
regex, the malformed pin token check. Those stayed exactly as strict as the
reviews asked for, because a line count is a budget for how a feature is
built, not a ceiling on whether a reported bug gets fixed.

The bug that was always possible and just became reachable

A different pull request from this same run, the one adding autograde
settings like max attempts, got a review comment that had nothing to do with
any of the new columns:

when a mentor submits max_attempts: 0, the new validation rejects
@assignment.update. the action renders :edit, but
after_action :check_reopening_status still runs. if the assignment has a
forked submission, Assignment#check_reopening_status can move its
assignment association and destroy the fork despite the failed update.

after_action callbacks run regardless of whether the action they follow
succeeded. This one always had that property. It just never mattered before,
because almost nothing about updating an assignment used to fail validation.
Adding a max_attempts column with an actual numericality check gave
mentors, for the first time, an easy way to submit a rejected update, which
is the one condition this pre-existing callback was never guarded against.

def check_reopening_status
  return if @assignment.errors.present?

  @assignment.check_reopening_status
end

Two lines. I reverted them and reran the new regression test to confirm a
rejected update really does destroy a student's forked submission without
the guard, which it does, then restored the fix and confirmed it survives.
The bug was not introduced this phase. The conditions for triggering it
were.

Where the project stands

Three pull requests carry review fixes now instead of open findings: the
testbench model, the runner, and the test case editor, the last one smaller
and safer than when it went up. The autograde settings pull request gained
an unrelated but real fix along the way. Underneath all of it, the actual
grading pipeline, a result model that snapshots the suite it ran against
rather than pointing at the live one, a job that calls the runner and
respects max attempts, a mapper from raw score to whatever scale the
assignment actually uses, and a results page that redacts hidden test cases
unless an instructor has chosen to reveal them, went up this week as well.

Next week: watching whether any of these five pull requests actually merge,
and what the first one to land changes for everything still stacked behind
it.

Week 15: What the Simulator Actually Binds

Akanksha Trehun — Thu, 01 Oct 2026 12:26:33 +0000

Week 15: What the Simulator Actually Binds

#7746 merged clean, rebase and all, closing out last week's story. That
freed the week for the thing I actually promised: picking the test case
editor back up. Except the editor cannot be evaluated on its own terms until
its two dependencies, the testbench model (#7836) and the runner (#7906),
are actually mergeable, and both came back from review this week with
findings that needed fixing before either could move.

A label that is not a string

CodeRabbit's first flag on the testbench model was on the signal validator:

def validate_signal(signal, cases)
  return errors.add(:data, "every signal needs a label") unless signal.is_a?(Hash) && signal["label"].present?
  ...

present? rejects nil, empty strings, and whitespace-only strings. It does
not reject 1. A JSON body with "label": 1 sails through this check, gets
stored, and later reaches the simulator, which calls .trim() on every
signal's label while binding pins to the circuit. Numbers do not have a
.trim() method in JavaScript. The failure is not "this test case behaves
oddly," it is a TypeError that kills the run.

The fix is one clause:

return errors.add(:data, "every signal needs a label") unless signal.is_a?(Hash) && signal["label"].is_a?(String) && signal["label"].present?

Small, but it is the kind of small that only shows up if you trace the value
forward into the file that actually consumes it, rather than stopping at "the
model validates its own shape."

The binding a human reviewer caught that a linter would not

The second finding was not from CodeRabbit. A reviewer left this on the same
file:

nothing checks that later groups use the same input/output labels as
groups[0]. the simulator binds pins from groups[0] only
(testbench.js:818-823) and then does inputs[label].state and
group.outputs.find(...).results.push per group, so a suite that passes
this validator can throw a TypeError mid-run.

I went and read testbench.js before touching anything, because the claim is
specific enough to verify rather than trust. It is exactly right:

function bindIO(data, scope) {
    const inputs = {};
    data.groups[0].inputs.forEach((dataInput) => {
        inputs[dataInput.label.trim()] = scope.Input.find(...);
    });
    ...
}

Only data.groups[0] ever gets read here. Every other group is bound
against whatever bindIO built from the first one. If group two declares an
input labeled c that group one never had, inputs['c'] is undefined, and
setInputValues dies trying to set .state on it. The model's own
validation, one group at a time, had no way to catch this, because the bug is
not in any single group, it is in the relationship between groups.

The fix compares each group after the first against the first's shape:

def validate_group(group, reference)
  ...
  next unless reference

  shape = ->(list) { list.map { |s| [s["label"], s["bitWidth"]] } }
  next if shape.call(signals) == shape.call(reference[side])

  errors.add(:data, "#{side} must match the first group's signals")
end

Values that are the right length and the wrong alphabet

Same reviewer, same file, a second finding: values were only length checked
against the case count, never checked for actually being binary digits. The
simulator feeds inputs through parseInt(v, 2) and compares outputs against
dec2bin(value, bitWidth) as strings. [0, 1] as integers, "2" on a one
bit pin, all of it passes the old check and then fails every single case at
run time, silently, which reads to a student as "I got every case wrong"
rather than "the test data itself is malformed."

def valid_values?(values, cases, width)
  values.is_a?(Array) && values.size == cases &&
    values.all? { |v| v.is_a?(String) && width.is_a?(Integer) && v.match?(/\A[01]{#{width}}\z/) }
end

Exact width, exact alphabet, exact type. I added a regression case for a
numeric label, one for wrong-width values, and one for a second group with a
mismatched pin set, then reverted each fix in turn to confirm its matching
test actually fails without it. All three did. A test that cannot fail is
not a test, it is a comment that happens to run.

The runner's two findings were a different flavor entirely

#7906, the runner, got two findings that had nothing to do with the shape
of any data structure:

with this fallback a prod box missing SIMULATOR_RUNNER_URL silently posts
student circuits to its own localhost and every run just shows as
"unreachable".

def endpoint
  "#{ENV.fetch('SIMULATOR_RUNNER_URL', DEFAULT_URL)}/run"
end

The default was meant for local development convenience and quietly doubled
as a production misconfiguration that fails silent instead of loud. Dropping
the fallback entirely, ENV.fetch('SIMULATOR_RUNNER_URL') with no second
argument, means a missing variable raises immediately instead of routing
every grading attempt to a service that was never there.

The second finding was that the request carries no authentication at all,
so anything that can reach the runner can make it simulate arbitrary
circuits. I added a bearer token, ENV.fetch('SIMULATOR_RUNNER_TOKEN'), sent
on every request. The runner service on the other end still needs to
actually check it, which is a different pull request's job, but the caller
no longer has an excuse not to send one.

Two different ways of being right

The testbench findings came from reading JavaScript nobody asked the model
to know about. The runner findings came from reading the word "fallback" and
asking what happens when the thing it falls back to is wrong. Neither is
something a type checker or a schema validator would have caught on its
own; both needed someone to actually trace a value from where it is written
to where it is read. That is becoming the throughline of this whole review
cycle: static analysis catches shapes, a reader who follows the data catches
contracts.

Where the project stands

Both the testbench model and the runner have their review fixes pushed, all
cases verified failing without their fix and passing with it. The test case
editor is still sitting locally, unstacked, waiting for these two to actually
merge before its own compare page can show one clean commit instead of three
borrowed ones.

Next week: the editor's own review round, which turns out to have found the
same class of bug in the client side JSON that this week found in the model,
plus one unrelated bug that only became reachable because of a setting this
phase never had before.

Week 14: The Approved Pull Request That Stopped Passing

Akanksha Trehun — Tue, 08 Sep 2026 17:38:43 +0000

#7746, the OIDC login
initiation, has been approved and waiting since week 10. This week I went back
to actually merge it and found its test suite failing on an error that had
nothing to do with anything I remembered writing.

A NameError in code I had already shipped

CI was red across the whole oidc_login context, ten examples, all the same
failure:

NameError:
  uninitialized constant LtiController::LTI_STATE_PURPOSE

That should have been impossible. oidc_login and lti_state_verifier both
reference LTI_STATE_PURPOSE, and the request spec references
LtiController::LTI_STATE_PURPOSE directly to verify the state it gets back.
If that constant had never existed, this whole context should have failed the
day it was written, back in week 10. It did not. Something had removed a
constant that used to be there.

Tracing it

git log -p on the controller file gave a clean timeline. The constants first
appear on 5 August, in the commit that adds oidc_login itself, with a comment
explaining why the state is signed rather than kept in the session. They survive
the review fixes from week 10. They even survive a rough merge from master on 9
August that briefly left the file unparseable, a splice that dropped the closing
end of oidc_login and spliced two of master's filter lines into the method
body. The very next commit repairs that, and its diff still shows the constants
sitting untouched at the top of the class, comment and all.

The actual disappearance happened somewhere I was not looking. On 21 August, a
different contributor merged a repository wide change to master, adding
after_action :verify_authorized safety nets across dozens of controllers. On
this controller, on master's copy of the file, that meant inserting
skip_after_action :verify_authorized as the very first line inside the class
body. Master's copy at that point had no LTI_STATE_PURPOSE constants, because
none of this branch's work had merged into master yet.

On 5 September I merged master into my branch again, to catch up before opening
the merge. Both sides had inserted a new line in exactly the same spot, right
after class LtiController < ApplicationController: master's
skip_after_action, and my branch's constants with their comment. Diffing the
file immediately before and after that merge shows exactly what happened:

 class LtiController < ApplicationController
-  # The state is signed rather than kept in the session: the launch returns as a
-  # cross-site POST that a SameSite cookie would not survive, and the signature
-  # is what proves the launch answers an initiation we made.
-  LTI_STATE_PURPOSE = "lti.launch.state"
-  LTI_STATE_TTL = 5.minutes
+  skip_after_action :verify_authorized

No conflict markers survive in the final commit, which is the part that makes
this easy to miss. Two branches touching the same handful of lines usually forces
a manual conflict resolution, and somewhere in resolving that one I kept
master's line and let mine go, without noticing the branch had put something in
that exact spot too. A merge that finishes clean does not look like something
that needs a second look, which is exactly why this one went unnoticed for two
weeks.

The fix

Both constants, reinstated:

LTI_STATE_PURPOSE = "lti_oidc_state"
LTI_STATE_TTL = 5.minutes

The purpose string itself does not need to match the original literal, nothing
outside this file ever reads it, only the two internal call sites and the spec
need to agree on the same constant, which they now do by definition. The TTL
does need to stay at five minutes, since the spec travels six minutes forward
and checks the state is gone by then. I confirmed nothing else in the codebase
referenced either constant, ran the project's RuboCop config against the change
to be sure the addition was clean, and pushed. CI went from ten failures back to
green across every job, not just the one that had been failing.

What this says about "approved and waiting"

An approved pull request sitting unmerged is not a pull request that is done.
It is a pull request whose last green check run is quietly getting further from
the truth every time master moves underneath it, and nothing about the GitHub UI
makes that visible. The checkmark from three weeks ago looks exactly like a
checkmark from this morning.

This is the same lesson as week 10's development database drift, just at a
different layer. State that nobody actively re-verifies accumulates damage that
stays invisible until something finally runs against it again. There, it was a
migration nobody had rolled back. Here, it was a merge nobody had rerun. Both
times the fix was cheap once found and the finding was the entire cost.

The practical rule I am taking from this: before merging anything that has sat
approved for more than a couple of weeks, rebase it against current master and
rerun the suite first, rather than trusting a check that ran against a branch
state that no longer exists.

Where the project stands

#7746 is rebased, fixed, and green again, waiting on nothing but the merge
button. #7764, the access token exchange, is still open from last week. The
testbench model and runner from two weeks ago are both under review. The test
case editor is still finished and waiting locally.

Next week: actually watching the login initiation merge, and picking the editor
back up now that it has an access token to eventually authenticate against.

Week 13: A Kid Only the Platform Can Read

Akanksha Trehun — Tue, 08 Sep 2026 17:29:36 +0000

The access token work that jumped the queue last week went up as
#7764. It took a
security minded review to catch a bug that would have made every single token
request fail, and a second reviewer's confusion, arriving weeks later, to remind
me that a review thread can point at code that no longer exists.

What the tool needs before it can call anything

Every LTI Advantage service call, posting a grade, reading a roster, is
authenticated the same way. The tool signs a JWT assertion with its own private
key, POSTs it to the platform's token endpoint under the OAuth 2.0 client
credentials grant, and gets back a bearer token scoped to whatever it asked for.
None of that is CircuitVerse specific. It is 1EdTech's Security Framework
section 5.4.1 built on top of RFC 7523, the JWT profile for client assertions,
which itself sits on RFC 6749's client credentials grant.

Tokens are cached per deployment and per scope set, refreshed thirty seconds
before they expire rather than exactly at expiry, so a request landing in that
window does not race a token that is technically still valid when read and
expired by the time it reaches the platform. A blank or non string
access_token in the response is refused instead of cached, and a response with
no expires_in at all gets no token stored, rather than a guessed default
lifetime. Both of those came from reading the spec's failure modes rather than
just its happy path.

The bug a reviewer caught before merge

The first version signed the assertion like this:

JWT.encode(
  { iss: deployment.client_id, sub: deployment.client_id, ... },
  signing_key, "RS256", { kid: JWT::JWK.new(signing_key).kid }
)

A reviewer flagged the kid within a day. Lti::KeyManager, the class that
publishes CircuitVerse's public key at /lti/jwks, derives its key id from a
JSON::JWK thumbprint. JWT::JWK.new(signing_key).kid computes a kid using a
different digest entirely. The two never agree, which means the assertion would
have named a key that does not match anything the platform can find at our own
published JWKS endpoint. Every token request would fail, not occasionally, every
time, because the mismatch is structural rather than a bug that only shows up
under some inputs.

The fix reuses the exact computation KeyManager already does:

def kid_for(signing_key)
  JSON::JWK.new(signing_key.public_key).thumbprint
end

Same input, the public key, same digest, JSON::JWK's thumbprint. I checked
this by rereading KeyManager#public_jwk line by line rather than trusting that
two thumbprint calls with similar names must produce the same output, and they
do:

def public_jwk
  jwk = JSON::JWK.new(private_key.public_key)
  jwk[:kid] ||= jwk.thumbprint
  jwk.merge(use: "sig", alg: "RS256")
end

A comment that arrived three weeks late, pointed at code that no longer existed

The fix shipped within a day of the review comment. Three weeks later, a second
reviewer reread the thread from the top and asked why I was "still" using
JWT::JWK.new(signing_key), pointing straight at the original code block.

GitHub pins a review comment to the diff position it was written against. Once
that hunk changes, the thread gets marked "Outdated," but the stale code stays
visible inline right where the comment is anchored, and the label is easy to
miss if you are reading a long thread quickly rather than checking the Files
Changed tab. The question was reasonable. The premise, that the flagged line
was still in the branch, was not.

The reply pointed at the fix commit by hash, quoted the current kid_for
method, and reread KeyManager's source alongside it to show the two
computations are the same, rather than just asserting "it's fixed, trust me."
That habit, rederiving a claim instead of restating it, is the same one from
week 9's JWT validator work: a test that passes for the wrong reason is worse
than no test, and an explanation that only restates the conclusion is worse than
one that shows the work.

A question with a better answer than "I just know"

The same reviewer asked, on a different thread, how I knew which fields
belonged in the token request body at all, and whether it was written down
anywhere or just something I had picked up.

It is written down, in three places that stack on each other. grant_type and
scope come from RFC 6749 section 4.4, the client credentials grant itself.
client_assertion and client_assertion_type come from RFC 7523 section 2.2,
the general mechanism for authenticating with a signed JWT instead of a shared
secret. The specific URI value platforms expect for
client_assertion_type, and the requirement that the JWT's iss and sub both
equal the tool's own client id, come from the IMS Security Framework, which is
the layer that actually ties LTI Advantage to that OAuth mechanism.

I answered with the three documents and the section numbers rather than a
summary of what they say, because a summary is something to remember and a
citation is something to reopen. Anyone auditing this code later, including me
in six months, needs the second thing.

Where the project stands

The access token pull request is open with both review threads addressed, the
kid bug fixed and explained, and the spec provenance question answered with
citations rather than recollection. Roster sync and grade passback can now, once
this merges, actually authenticate the calls they have been assuming all along.

Next week: going back to merge an older, already approved pull request, and
finding out it had stopped passing while nobody was looking at it.

Week 12: Two Small Pull Requests

Akanksha Trehun — Tue, 08 Sep 2026 17:26:16 +0000

Both pieces of the autograding groundwork from last week's "pipeline nobody could
feed" post finally went up for review. Getting them out the door taught me more
about the packaging rule I set for myself than the underlying feature did, and
along the way I found a gap in my own merged code that rhymes uncomfortably with
the one I found last week.

The rule I made myself follow

Somewhere back in this phase I decided to open small pull requests one at a time
and hold dependent branches locally until the parent merges, instead of stacking
them or folding a dependency into one oversized diff. I have rejected my own
compare pages twice for showing more than one commit, which is a strange thing to
do to yourself but a useful one.

The test case editor is the clearest reason why. Its branch holds a cherry pick
of the testbench model's commit plus the editor's own commit, because the editor
needs the model's table to exist to run its tests at all. Opening it now would
show two commits on the compare page, and a reviewer would have to work out which
one is actually new. So it stays local until the model merges, at which point I
rebase, the borrowed commit drops out, and what is left is one clean diff over
the editor's own files.

Which one first, and why it wasn't obvious

That left a smaller decision: open the testbench model or the runner first.

The runner's spec is deliberately built so it does not need the model to exist.
It signs its double with Struct.new(:data) instead of pulling in the real
Testbench factory, so the runner is green against master on its own. Nothing
about it forces the model to land first.

I opened the model first anyway, for three reasons that all point the same way.
The test case editor is stacked on the model, not the runner, so unblocking the
editor means merging the model first regardless. The model is also the smaller,
easier diff: one migration, one class, one spec, no external HTTP contract to
explain to a reviewer. And both branches touch db/schema.rb, which is exactly
the kind of file you do not want two open pull requests editing at once, since
whichever merges second has to rebase past a version bump it did not cause.

What actually lives in the Testbench model

One row per assignment, JSON in a jsonb column, validated in three layers that
mirror the shape of the data itself: suite, then group, then signal.

def data_is_runnable
  return errors.add(:data, "must be a testbench object") unless data.is_a?(Hash)

  errors.add(:data, "type must be comb or seq") unless TYPES.include?(data["type"])
  return errors.add(:data, "must define a group") if groups.empty?

  groups.each { |group| validate_group(group) }
end

Each group needs a case count and at least one input and one output. Each signal
needs a label to bind to a circuit pin, a positive bit width, and exactly as many
values as its group's case count. That last check is why validation lives in
Ruby instead of a database constraint. A jsonb column schema check can enforce
shape, but it cannot easily say "this array's length must equal that other
field's value," and that cross-field relationship is most of what makes a
testbench actually runnable rather than merely well formed.

assignment_id is unique at both levels, a database index and a model
validation, because a race on create could otherwise slip a second row past a
Ruby only check. TYPES is closed to comb and seq, the two suite types the
simulator can actually execute, so a suite claiming verilog fails at save time
instead of failing later, mid grade, in front of a student.

The runner, and the decision that already happened

Most of the runner's design was decided last week, before a line of it existed.
This week was writing it down.

def self.call(project:, testbench:)
  new(project, testbench).call
end

One entrypoint. It posts the project's circuit JSON and the testbench's suite
JSON to a sidecar service over HTTP, with timeouts split into connect, write,
and read rather than one blanket number, because a hung connection should fail
fast while an actual simulation run legitimately needs more time than the
handshake. Every way this can go wrong, a bad status, an unreachable host, a
body that is not JSON, a response with no groups, collapses into one
Autograder::Runner::RunnerError, so the grading job that calls this later only
ever needs to rescue one thing.

Result = Struct.new(:passed, :total, :groups) do
  def score
    total.zero? ? 0.0 : passed.fdiv(total)
  end
end

The raw groups array rides along inside the result even though only score
matters right now, because the results page a few pull requests from now needs
to show which specific case failed, not just the fraction. The endpoint's host
comes from SIMULATOR_RUNNER_URL, following the same undocumented environment
variable pattern the codebase already uses for YOSYS_PATH, so anyone who has
configured one external tool location here already knows how to configure this
one.

What "next" turned out to mean

Last week's post ended by promising the editor would land next. It has not, and
writing the runner's pull request description is what changed my mind.

I went back to reread the roster sync pull request from two weeks ago, the one
that calls the platform's Names and Roles service, to check whether it needed
anything from this new access token machinery. It does. So does grade passback.
Both of those already assume an Authorization: Bearer header carrying a token
scoped to the right service, and until this week nothing in the codebase could
produce one. I had built two pull requests that read from and write to platform
services that require an OAuth exchange neither of them performs.

This is the same shape of gap as last week's missing test case editor: a feature
description satisfied on paper by code that quietly assumes a piece nobody has
built yet. The difference is I caught this one myself, by rereading my own
merged work with the new piece in mind, rather than having a mentor's evaluation
catch it for me.

Where the project stands

The testbench model and the runner are both open for review, in that order.
Roster sync and grade passback are merged but cannot actually authenticate
against a platform yet. The test case editor is finished and waiting locally for
the model to merge.

Next week: the access token exchange that both of those merged pull requests
have been silently depending on.

Week 11: Welcome to my Midterm evaluation

Akanksha Trehun — Sun, 16 Aug 2026 06:06:06 +0000

Midterm evaluations ran from the 10th to the 14th. Deep linking and roster sync both
shipped, two more pull requests merged, and then I sat down to write an honest account of
where the project stands — which is how I found out that the plan I have been working
from for a month builds a machine nobody can switch on.

Deep linking, end to end

Two pull requests, two days apart.

#7772 handles the request
side: a launch arrives with LtiDeepLinkingRequest as its message type instead of
LtiResourceLinkRequest, and everything downstream has to route differently. The
platform sends along deep_linking_settings — where to return the response, whether
multiple items are allowed, whether the platform wants a title back — and that has to be
stashed for the response, which happens on a completely separate request.

#7777 is what the instructor
actually sees: a picker listing their circuits and assignments, with the selection posted
back to the LMS as a signed JWT.

The satisfying part is that neither pull request needed anything new from the protocol
layer. The JWT signing is the key manager from week 4. The validation on the way in is
the validator from week 9. Deep linking is a different conversation with the platform,
but it is the same handshake underneath, and by this point the handshake is just there.

The roster

#7779 reads a course roster
over the Names and Roles service — pagination via Link headers, roles parsed out of the
IMS role URIs. #7781 turns
that into group members.

The interesting decision in the second one is how users get matched. Not on email — that
was week 5's lesson, and the reason for it holds here too: a platform's privacy settings
may mean no email arrives at all, and even when one does, it might belong to a different
CircuitVerse account than the person clicking. The match is on sub plus issuer, the
same identity pair the launch signs the user in with. A roster import that mapped on
email could silently add a stranger to a course.

Members created this way get marked as LTI-synced, which matters for the drop-handling
pull request that follows: when someone disappears from the LMS roster, only synced
members get deactivated. Somebody who joined the CircuitVerse group directly should not
be removed because they were never in Canvas to begin with.

Two merges

#7715, the JWT validator,
merged on the 11th. #7763,
the resource link model, merged on the 12th. Both had been sitting approved for a while —
the validator since the end of July.

Five merged now: the 1.1 passback fix, the deployment model, the JWKS and tool
configuration endpoints, the validator, and the resource link. Eight more open and
waiting.

The evaluation

What the evaluation actually asked me to do was describe the state of the project, which
sounds trivial and was not. Writing "here is what works" forces a different question than
"here is what I built this week," and the two answers were not the same shape.

So I went back to the three issues this project exists to close and read them line by
line against what is in the repository. Not the proposal — the issues, as filed.

A pipeline nobody could feed

The autograding phase of my plan had six pull requests: a test case model, a headless
runner wrapper, a grading result, a job that runs on submission, a results page, and a
push to the LMS gradebook. Student submits, autograder runs, grade lands in Canvas.

Every one of those is about executing test cases. Not one of them is about creating
test cases.

The issue says it plainly — "a test case editor in the assignment settings UI where
instructors can add, edit and delete test cases" — and I had somehow read that as
satisfied by a database table. It is not. A model with no UI means the only way to define
a test suite is the Rails console, which is not a feature an instructor can use. I had
planned six pull requests that would ship a grading pipeline with no way to put anything
into it.

Reading further with that lens, four more gaps fell out. There was no visible/hidden flag
on test cases, which both issues ask for. There was nothing for the autograde
configuration — partial credit, allowed attempts, whether students see the cases before
submitting. "Versioned with the assignment" had no plan behind it at all, and my model
actively worked against it: one row per assignment, edited in place, so changing a suite
would quietly invalidate every grade already produced from it.

And the least visible one, which I only found by reading the Grade model rather than
remembering it: nothing in the plan mapped a score onto an assignment's grading scale.
The autograder produces a fraction. Grade validates against letter, percent, or
custom, and rejects no_scale outright. Autograding an assignment with no grading
scale would have failed at the last step, after the run, with the student watching.

Twenty-four becomes twenty-seven

Week 7's post was called "Twenty-Four Pull Requests." That plan is now twenty-seven, all
three additions in the autograding phase: the test case editor, the autograde settings,
and the grading-scale mapping — plus reworking the grading result to carry an attempt
number and a snapshot, and folding hidden-case redaction into the results page.

The snapshot is how "versioned with the assignment" gets solved, and it is the change I
am happiest with. Rather than a version table with all the machinery that implies, each
grading result stores a copy of the suite it ran against. Edit the suite afterwards and
old results stay reproducible, because the thing that produced them is still sitting
right there next to them.

Three of the nine are written. The model and the runner are pushed; the editor is done
and waiting locally, because it needs the model's table to exist and I would rather open
one clean pull request after the first merges than one showing two commits.

Reusing a format instead of inventing one

The nicest thing I wrote this week is four words long.

A test case needs a name, input pin values, expected output pin values, and a hidden
flag. The simulator's testbench format already has groups — a named collection of cases
with input and output signals. My first instinct was to add fields: a name array, a
hidden array, indices lining up across them.

Then: a test case is just a group with n: 1.

One case, one name, one hidden boolean. It validates against the model I had already
written without changing a line of it, it invents no schema, and when the suite reaches
the simulator it is a format the simulator already knows how to run. The editor
serialises its table into exactly that on submit.

I have written the other version of this before — the one with parallel arrays and a
comment explaining how the indices correspond. It works right up until someone deletes a
row from one array and not the others.

Not reimplementing the simulator

The runner wrapper was the piece I was most nervous about, and the design question was
where the simulation actually happens.

The tempting answer was Ruby: run the circuit server-side, in the language the rest of
the grading code is written in. I talked myself out of it in about ten minutes. It means
reimplementing thousands of lines of simulation logic, and then maintaining two engines
that will drift — and the failure mode of that drift is a student's circuit passing in
the browser and failing at grading. For a grading tool there is no worse bug.

Driving a real browser was the next candidate, and capybara-playwright-driver is
already in the Gemfile, so it looked nearly free. A browser process per submission is not
nearly free.

Shelling out to Node was the closest call, because there is precedent: the Yosys
integration in this repository does exactly that. I went with an HTTP call to a sidecar
service instead, mirroring the other half of that same integration — a URL from the
environment, the same timeout-and-post shape the simulator controller already uses. It
keeps the engine on the frontend repository's release cadence, where that code and the
people who know it live.

Every failure collapses into one error class: bad circuit data, a non-2xx, a timeout, a
non-JSON body, an empty result. That is deliberate. The only caller is a background job,
and there is exactly one right answer to all of them — mark the run failed, leave the
student's grade alone.

Where the project stands

Five merged, eight open, three written and waiting. The protocol layer is done and
merged. Grade passback, deep linking, and roster sync are all in the queue. Autograding is
started.

The plan grew by three pull requests this week, which on a burndown chart is a bad week.
It is the most useful thing I did. I have spent eleven weeks getting good at making
mechanisms work, and the evaluation caught me having built most of a system that no
instructor could have used, because I kept reading a feature request as a description of
a machine instead of a description of a person trying to get something done.

Next week: the editor lands, and then the settings and the grading result behind it.

Week 10: The Handshake Begins

Akanksha Trehun — Sat, 08 Aug 2026 02:21:38 +0000

This week the first leg of the LTI 1.3 handshake shipped as
#7746, a static analyser and I
disagreed about CSRF, and I spent an afternoon cleaning up a database mess I had made
for myself weeks earlier.

OIDC login initiation

The endpoint is /lti/login, and it is the URL that the tool configuration from
#7659 has been advertising as
oidc_initiation_url for three weeks while returning 404.

An LMS calls it with iss, login_hint, and optionally client_id and
lti_deployment_id. CircuitVerse resolves those to a registered deployment, mints a
one-time nonce and a signed state, and redirects the browser to the platform's
authorization endpoint:

302 Found
location: http://canvas.docker/api/lti/authorize_redirect?scope=openid
  &response_type=id_token&response_mode=form_post&prompt=none
  &client_id=...&redirect_uri=.../lti/launch&login_hint=demo-student
  &nonce=ddbf28cc...&state=eyJfcmFpbHMi...--e3a4d84925a8df11c8dc6a...

response_mode=form_post and prompt=none are what make the platform POST the token
straight back rather than showing a login screen. The -- in the state is the HMAC
separator — that is the signed-state design from week 6 finally in production form.

64 lines of controller code, about 150 lines of tests.

Arguing with CodeQL

CI came back with CodeQL failing: CSRF protection weakened or disabled.

It was pointing at this line, which I had edited to add the new action:

skip_before_action :verify_authenticity_token, only: %i[launch oidc_login]

My first reaction was that this is a false positive. Of course there is no CSRF token —
the request originates from Canvas. There is no way for an LMS to hold a token minted by
our forms.

My second reaction, which took longer, was that the scanner was pointing at something
real even if its conclusion did not fit. skip_before_action is a blunt instrument: it
turns the check off for the entire action, for every request shape, forever. What I
actually wanted to say is narrower — this particular kind of request does not need a
token, and everything else still does.

So instead of skipping, I overrode the predicate the check uses:

def verified_request?
  super || (action_name == "oidc_login" && params[:iss].present? && params[:login_hint].present?)
end

Token verification stays fully active for every other action and every other request
shape. Only a well-formed LTI initiation is treated as verified. And the justification
holds up: the action reads no session, writes nothing, and the signed state it returns is
what actually protects the launch that follows.

I checked this was real rather than incidental by removing the override and running the
tests — the cross-site POST spec fails with InvalidAuthenticityToken. Worth doing,
because a test that passes for the wrong reason is worse than no test. The test
environment disables forgery protection by default, so that spec has to switch it back
on explicitly or it proves nothing at all.

CodeQL went green, and the code is genuinely narrower than what I started with. The
scanner was more right than I initially gave it credit for.

What review caught

Three findings on the pull request, all worth having:

Ambiguous deployment matches. My lookup ended with scope.order(:id).first. Since
client_id is optional in an initiation, a platform with several registrations under
one issuer would silently resolve to whichever row happened to have the lowest ID — and
the launch would be sent to a different registration's client ID and authorization
endpoint. Now an ambiguous match is refused with a 404 rather than guessed at.

Non-HTTPS authorization endpoints. The LtiDeployment model only validates that
auth_login_url is present, not that it is a sane URL. Since the redirect uses
allow_other_host: true, a malformed or hostile value in that column goes straight to
the browser. A javascript: URL is now rejected before it can reach redirect_to, and
TLS is required in production while plain http still works locally.

Registered query parameters being discarded. Some platforms register an
authorization endpoint that already carries query parameters. I was replacing the query
string wholesale. The suggested fix merged instead of replaced — and introduced a subtle
bug of its own by merging symbol keys into a hash with string keys, so a platform's own
scope= would have survived alongside ours as a duplicate parameter. String keys
throughout fixed it.

That last one is a good reminder that a suggested patch is a suggestion. Both PRs this
week had a fix applied through the GitHub web UI that did not splice cleanly — one
duplicated the tail of a method, leaving four unreachable statements and a duplicate
rescue clause that failed lint. Convenient, but it still needs reading.

The database mess

Somewhere in this week I tried to run the new endpoint against my development database
and got a NOT NULL violation on a column that does not exist in the merged schema.

The cause was archaeology. My abandoned POC branch from week 2 had a migration creating
lti_deployments with a platform_id column. I ran it against my dev database in June.
The branch was closed; the migration was never rolled back. The merged version of that
table, from a completely different pull request, has no platform_id — so my dev
database had a table that matched no branch anyone was working on.

Tracing it was more interesting than expected. The physical column order gave it away:
platform_public_key sat after the timestamps, which meant it had been added by a
later add_column rather than declared inline — which pinned it to the March POC
migration rather than the June one. Twenty-six migration versions were recorded in my
dev database with no corresponding file on any branch, along with an orphaned
lti_platforms table whose migration no longer exists anywhere.

I cleaned it surgically rather than dropping the database: exported the two Canvas
registrations my demo depends on, dropped the drifted columns and tables, cleared the
stale migration stamps, ran the real migration, and re-imported the registrations
against the new schema.

The lesson is cheap to state and I had to learn it the expensive way: migrations you
run from a branch that never merges do not clean themselves up. A dev database is
long-lived state that accumulates the residue of every experiment.

Where the project stands

Merged: the 1.1 grade passback fix, and the deployment model. Approved and waiting: the
JWKS and tool configuration endpoints, the JWT validator, and now the OIDC login.

That is four of the twenty-four, with the whole of phase 1's protocol layer either
merged or in the queue.

Next is the launch itself — verify the state, validate the token through the validator
from week 9, bind the token's deployment to the one the state was issued for, and sign
the user in on their sub. It is the piece where all the separate bricks finally do
something a user can see: a Canvas click landing a signed-in user inside CircuitVerse.

It is also the one I need to be most careful with, and I have a list of open questions
to settle before writing it — what happens when a platform's privacy settings mean it
sends no email address at all, what happens when that email already belongs to a
CircuitVerse account, and where the nonce gets recorded so a state cannot be replayed
inside its five-minute window.

Ten weeks ago I would have just written it and found out.

Week 9: Verifying the Token

Akanksha Trehun — Sat, 08 Aug 2026 02:19:37 +0000

Two pull requests merged this week and the most security-critical piece of the whole
project went out for review on its own. This was the week the plan from week 7 started
visibly paying off.

Two merges

On 28 July, #7654 merged — the
LtiDeployment model. CircuitVerse can now store which LMS platforms it trusts: the
issuer, the client ID, the deployment ID, and the platform's endpoint URLs, unique per
issuer + client + deployment because that triple is how LTI 1.3 identifies a deployment.

Nothing reads the table yet. That is the point of building this way, and it is the thing
that needs explaining in every PR description.

The validator

The main work was #7715:
Lti::JwtValidator, a class with one public method.

Lti::JwtValidator.validate!(token, deployment:, nonce:)

Give it an id_token, the deployment it should belong to, and the nonce we issued. It
either returns the verified payload or raises. No controller, no session, no Rails
request object. That isolation was forced by the constraint that it had to be its own
pull request, and it is a much better boundary than I would have drawn otherwise.

What it has to refuse

The tests are mostly attacks:

A token signed by the wrong key. The baseline.

A token with alg: none. Early JWT libraries would accept an unsigned token if it
declared its algorithm as none. Any validator that reads the algorithm out of the
token and trusts it is exploitable.

An HS256 token where RS256 is expected. This is algorithm confusion, and it is the
subtle one. RS256 is asymmetric: the platform signs with a private key, we verify with
their public key, which is published. HS256 is symmetric: the same secret signs and
verifies. If a validator takes the algorithm from the token header and passes the
platform's public key as the key, an attacker can sign a token using that public key
as an HMAC secret — and the validator verifies it, because it is doing exactly what it
was told. The public key is public, so anyone can do this.

The defence is to never read the algorithm from the token. Pass an explicit allow-list:

encoded_token.verify_signature!(algorithm: "RS256", key: key)

Expired, wrong issuer, wrong audience, missing subject, replayed nonce, and the
multi-audience case: when aud is an array with more than one entry, the azp claim
must name our client ID.

Resolving the platform's key

The other half is fetching the right public key. A platform publishes a JWKS endpoint;
the token header carries a kid identifying which key signed it.

Two things made this more interesting than expected:

Key rotation. Platforms rotate keys. A cached key set will not contain a newly
rotated key. So a kid that is missing from the cache is treated as a signal that
rotation may have happened, and triggers a refetch rather than a wait for expiry.

Caching. Without it, every single launch makes an outbound HTTP request to the
platform before anyone can sign in. If the platform's JWKS endpoint is slow, every login
is slow; if it is down, every login is down. A short-lived cache keyed on the JWKS URL
fixes that, with the rotation behaviour above as the escape hatch.

What review found

The bot review flagged two failure modes I had not handled, and both were correct:

A blank jwks_url. A deployment configured with only a stored public key and no
JWKS URL would still make an HTTP request, because a blank URL parses as a relative path
rather than failing. So every launch for that deployment took a pointless network trip
before falling back. One guard clause.

Malformed stored key data. OpenSSL::PKey::RSA.new raises OpenSSL::PKey::RSAError
on bad PEM data, and I was only rescuing JWT::DecodeError. So a misconfigured
deployment record would propagate an OpenSSL exception out of a method whose entire
contract is "raises ValidationError". Callers written against that contract would not
catch it.

That second one is a good example of a bug that is invisible while you are writing the
code and obvious once named. My mental model was "this method validates tokens, so it
raises validation errors." The actual behaviour was "this method raises validation
errors unless the database row is malformed, in which case it raises something else
entirely."

I fixed both in fix(lti): harden JwtValidator key resolution per review.

The refactor request

Late in the week, a maintainer asked something more interesting than a bug report:

could you suggest refactoring this class using JWT::Token or JWT::EncodedToken

My implementation called JWT.decode twice — once with verification disabled to read
the header and pick a key by kid, then again with verification on. That works, and it
has a smell: there is a window where the code holds an unverified, parsed token, and
nothing but my own discipline stops me from reading a claim out of it.

JWT::EncodedToken closes that window at the library level. #payload refuses to
decode until both the signature and the claims have been verified:

def payload
  raise JWT::DecodeError, '...' unless @signature_verified
  raise JWT::DecodeError, '...' unless @claims_verified
  decoded_payload
end

The header is still read unverified, which is unavoidable — you cannot pick a key
without knowing which key was used — but it is only ever used to select a candidate key,
and the ordering the class used to depend on by convention is now enforced by the
library.

That is a strictly better property: the difference between "the code is correct" and
"the code cannot easily be made incorrect by the next person to edit it."

What I am taking from the week

The isolation was the win. Because the validator was its own pull request, with no
controller and no session attached, both the review comments were about the validator
— its error contract, its failure modes — rather than about whether the launch flow
around it made sense.

Compare that to my POC, where the same class was buried in 2,000 lines and nobody looked
at it at all.

Next week: the first half of the handshake ships, and a scanner disagrees with me about
CSRF.

Week 8: The First Merge, and Learning to Wait

Akanksha Trehun — Sat, 08 Aug 2026 02:18:20 +0000

On 26 July, #7647 merged. It
was the first piece of my project to land in CircuitVerse's master branch, fourteen
days after I opened it and about seven weeks into the project.

It is a small change. It makes sure an LTI 1.1 grade is written to CircuitVerse's
database before it is pushed to the LMS gradebook, so the two cannot silently disagree
when a local save fails. Plus the fix from last week's review — clearing stale grading
context only after a launch has been verified, so a forged request cannot wipe a
teacher's session on its way to being rejected.

Most of this week was spent not writing code, which is a thing I want to write about
honestly rather than skip over.

What actually shipped

The merged change has two halves, and the second only exists because of review.

The first is ordering: persist the grade, then push it to the LMS. Previously the
passback ran first, so a local save that failed validation left the gradebook holding a
score CircuitVerse had no record of. The LMS is the more visible system, which makes
this the worse direction to be wrong in — a student and an instructor both see a grade
that does not exist anywhere in our database.

The second is the fix from anxkhn's review: stale grading context is now cleared only
after a launch has been verified, not before. An unauthenticated request that gets
rejected can no longer wipe a teacher's in-progress grading session on its way out.

Both are small. Neither is glamorous. Together they mean the gradebook and CircuitVerse
cannot silently disagree, in an integration that schools are using today, which is worth
more than another thousand lines of the 1.3 work sitting unmerged on a branch.

The shape of the week

My commit graph for 16–25 July is empty. Two pull requests were open and waiting
(#7654 and
#7659), one was awaiting the
final look before merge, and the next thing in the plan — the JWT validator — depended
on decisions inside them.

Early in the project, a week like this would have gone one of two ways. Either I would
have sat refreshing the PR page, or I would have started building three PRs ahead and
piled up work that might need reworking depending on how the review went.

I had done the second thing already, in weeks 2 and 3, and it is how I ended up with a
2,000 line branch nobody could review.

What review latency actually is

The reframe that helped: maintainer response time is not an obstacle in the process. It
is the process.

CircuitVerse's maintainers review contributions alongside their own jobs. A day or two
of latency on a pull request is not neglect, it is the normal operating speed of a
volunteer-maintained project. Every plan I make has to be built for that speed rather
than in spite of it.

Concretely, that changes what "being blocked" means. If PR N is in review and PR N+1
depends on it, I am not blocked from working — I am blocked from committing to a
particular implementation of N+1. Those are different, and the gap between them is
where the useful work of a waiting week lives.

What I did with it

Read the review comments properly. Not just applying them, but working out the
general rule behind each. anxkhn's note about clearing grading context before
verification was specifically about one method, but the rule it implies — nothing that
mutates state may happen before the security check, including deletes — is one I could
then apply to code they had not looked at. A review comment is a sample from a
distribution of mistakes I am prone to. Fixing the sample is the minimum; finding the
distribution is the value.

Wrote the next PR's tests before its code. The JWT validator was next, and while I
did not want to commit to its implementation before the deployment model merged, the
attacks it must survive did not depend on any of that. A token signed with the wrong
key. A token with alg: none. An HS256 token where the validator expects RS256 —
algorithm confusion, where an attacker signs with the public key as an HMAC secret and a
naive validator accepts it. Expired tokens, wrong issuer, wrong audience, missing
subject, replayed nonce.

Writing those first meant that when the validator got written, its shape was determined
by what it had to refuse rather than by what I found convenient to implement.

Read other people's LTI implementations. Specifically for how they handle key
rotation — a platform can publish a new signing key at any time, and a tool that caches
the key set has to notice. The pattern I settled on: cache the key set, but if a token
arrives with a kid that is not in the cache, refetch immediately rather than waiting
for the cache to expire. Rotation is exactly the case where the cache is stale, so
treating a cache miss as a rotation signal is the right default.

Kept the queue moving. The point of the twenty-four PR plan is a steady stream of
small reviewable things. That only works if the queue is never empty from my side, and
never so full that it becomes a burden. Two open at a time, both small, both green,
turned out to be about right.

On the size of the win

Fourteen days from open to merge, for a change of about 150 lines that fixes a real bug
in a feature real users have today.

Measured in lines of code per day it is not much. But the previous seven weeks produced
zero merged lines, and the thing that changed was not that I started working harder — it
was that I started sending work in a form somebody could say yes to.

That is the actual lesson of the first half of this project, and I could not have
learned it by reading about it. I had to spend three weeks building something good that
could not merge.

There is a version of this project where I never write the POC, go straight to small
PRs, and have six things merged by now. I do not think I would have understood why
they had to be small. The POC being closed is what made the constraint make sense rather
than feel like bureaucracy.

Where things stand

Merged: the 1.1 grade passback fix.
Approved and waiting: the deployment model, and the JWKS and tool configuration endpoints.
Next: the JWT validator — the security-critical brick, reviewed on its own.

Next week: the token validator, and finding out my key-resolution code had two failure
modes I had not handled.

Week 7: Creating phases for the tasks

Akanksha Trehun — Sat, 08 Aug 2026 02:16:10 +0000

This was the week the project got a plan it could actually execute, and the week a
reviewer found a security bug in a fix I had already convinced myself was correct.

The review

I had opened #7647 at the end
of last week — the small fix ensuring an LTI 1.1 grade is saved locally before being
pushed to the LMS.

On 14 July, anxkhn reviewed it. The first comment was housekeeping: rebase and look
at failing CI. I rebased, tests went green, and I said so.

The second comment is the one worth writing about:

storing the context is fixed, but it is still cleared before valid_request?. for
example, a fake launch could clear a teacher's valid grading session and prevent the
next grade passback. please clear it only after a valid launch too.

Let me unpack what that means, because it is genuinely subtle.

An LTI 1.1 launch stores some grading context in the session — the outcome service URL
the LMS wants scores posted to, and which assignment is being graded. My change made
sure that context is written after the launch signature is verified. Good.

But the clearing of stale context happened in a before_action, which runs before
valid_request? — before we know whether the launch is genuine at all.

So: a teacher launches from Canvas, gets a valid grading session, starts grading.
Meanwhile anyone can POST garbage at /lti/launch with no valid signature. That request
gets rejected — 401, correct — but on its way to being rejected it has already wiped
the teacher's grading context. Their next grade passback silently fails.

An unauthenticated request cannot sign in, cannot read anything, cannot write
anything — and can still degrade a legitimate user's session as a side effect of being
rejected.

I had read that code many times. I had written the fix for the adjacent bug in the same
method. I did not see it, because I was asking "is the write ordered correctly?" and the
bug was in the delete.

What I take from it: when you fix ordering around a security check, audit every
operation on that path, not just the one you came for. The invariant is not "the write
happens after verification." It is "nothing that affects state happens before
verification." I had fixed one instance of a general rule while leaving another in
place.

I also want to record that this is the value of review that no amount of care
substitutes for. Not because the reviewer knew something I did not, but because they
came to the code without my assumption about which line was interesting.

The other review comment

The same day, anxkhn reviewed
#7654, the LtiDeployment
model, and approved it — with a nitpick:

could you change Fixes #7405 to Part of #7405? this PR is the first foundation
step, so merging it should not close the larger issue.

Small, and it corrected a real misunderstanding. I had been writing Fixes #7405 on
every LTI pull request out of habit. GitHub's auto-close keywords mean the first one to
merge would have closed the umbrella issue tracking the entire LTI 1.3 upgrade — with
about 90% of it unbuilt.

Part of #7405 links without closing. I have used that phrasing on every LTI PR since.

Twenty-four pull requests

The bigger piece of the week was planning. On 14 July I wrote out the remaining work as
a sequence of pull requests, and the constraint I set was that no single one should
exceed roughly 200 hand-written lines. Generated files like Gemfile.lock and
db/schema.rb do not count, but get flagged for reviewers.

It came to twenty-four.

The number was startling and also clarifying. What used to be "implement LTI 1.3" became
a dependency-ordered queue, with phases:

Phase 0 — the credibility work: fix the 1.1 bug that already affects real users.
Phase 1 — the launch core, five PRs: gems and the deployment model; key manager and public endpoints; the JWT validator alone; OIDC login initiation; the launch itself; then the resource link model.
Phase 2 — Assignment and Grade Services, so grades flow to the LMS gradebook.
Phase 3 — Deep Linking, so an instructor can embed specific CircuitVerse content.
Phase 4 — Names and Roles, so the roster syncs.
Phase 5 — autograding.

The thing that surprised me is how much the constraint improved the design, not just
the delivery. Forced to make Lti::JwtValidator a standalone pull request, it has to be
a class that takes a token and a deployment and returns a payload — no controller, no
session, no Rails request. That is a better boundary than I would have drawn if I were
free to let it reach into whatever it wanted.

The same happened to the OIDC login. Made to stand alone, it has to hand back a state
that the launch can verify later with no shared memory between them — which is exactly
the signed-state design from last week. The constraint and the correct architecture
pointed the same way.

Two rules I wrote down alongside the plan:

If a PR crosses ~200 lines, split the spec edge cases into a follow-up — never thin out security validation to fit the budget.
Every PR description names which future PR depends on it, so a maintainer can see why a change with no user-visible behaviour deserves to merge.

That second rule is doing a lot of work. The hardest thing about this approach is that
most of the PRs do nothing on their own. A deployments table nothing reads. A validator
nothing calls. Without the dependency note, each looks like dead code, and "we don't
merge unused code" is a reasonable instinct for a maintainer to have.

Shipping against the plan

Two PRs went out under the new scheme:

#7654 — the LtiDeployment
model: issuer, client ID, deployment ID, and the platform's endpoint URLs, unique per
issuer + client + deployment. 97 lines. Storage only, nothing reads it yet. Approved
the day it was reviewed.

#7659 — the JWKS and tool
configuration endpoints, plus a key manager that owns CircuitVerse's own RSA keypair.
This is what makes CircuitVerse registerable: an LMS admin fetches the tool
configuration, and the platform can fetch our public key to verify anything we sign.
193 lines, behind the flag.

Both were reviewable in a sitting. Which, compared to a 2,000 line POC closed the next
day, is the whole difference.

Next week: waiting, and what I did with it.

Week 6: Signing the State, and Closing My Own Pull Requests

Akanksha Trehun — Sat, 08 Aug 2026 02:13:42 +0000

Three things happened this week: I undid the cookie change from week 5 with something
better, I put the entire LTI 1.3 feature behind a flag that defaults to off, and I
closed two of my own pull requests.

Undoing the cookie

Last week I set SameSite=None on the session cookie so it would survive the
cross-site POST that carries the id_token back from the platform. It worked, and it
weakened a CSRF defence across the whole application to serve one endpoint.

This week's commit is
fix(lti): sign the OIDC state instead of weakening the session cookie.

The reframe that unlocked it: I had been treating "the state must survive the round
trip" as "the state must be stored somewhere that survives the round trip." Those are
not the same requirement. The state does not need to be stored at all. It needs to be
recognisable when it comes back.

So instead of keeping a value in the session and comparing, the state becomes a signed,
self-contained token carrying its own payload:

LTI_STATE_PURPOSE = "lti.launch.state"
LTI_STATE_TTL = 5.minutes

state = lti_state_verifier.generate(
  { "nonce" => nonce, "deployment_id" => deployment.id },
  purpose: LTI_STATE_PURPOSE,
  expires_in: LTI_STATE_TTL
)

Rails.application.message_verifier signs it with the application's secret. When the
launch returns, we verify the signature, and if it checks out we know the contents came
from us and have not been altered. The nonce and the deployment ID ride inside the
token itself.

What this buys:

No session, so no cookie problem. Nothing needs to survive the cross-site POST except the parameter the platform is already sending back to us.
No cookie policy change. SameSite=Lax stays as it is for every other page on the site.
No server-side state. No table, no cleanup job, no expiry sweeper.
The signature is the CSRF defence. Only CircuitVerse can mint a valid state, so a launch carrying one is answering an initiation we started.
It expires. Five minutes, enforced by the verifier.

The purpose argument is a detail worth calling out. It scopes the signature to this
one use, so a token signed elsewhere in the application with the same secret will not
verify here. Same secret, different purposes, no cross-use.

The general lesson: when a constraint blocks you, check whether you are solving the
constraint or the requirement behind it. "Survive a cross-site POST" sounded like a
cookie problem. It was a state-management problem, and moving the state out of the
session dissolved the cookie question entirely.

The flag

The commit feat(lti): gate LTI 1.3 behind a default-off lti_advantage flag does what
it says: every 1.3 endpoint returns 404 unless an operator turns the flag on.

The reason is specific. The 1.3 launch auto-provisions user accounts. A launch from
a registered platform creates a CircuitVerse user, confirmed, ready to sign in. That is
correct behaviour for the feature and it is also a significant thing to have live in
production the moment a branch merges.

A default-off flag means the code can land, be reviewed, be tested, and sit there inert
until somebody deliberately switches it on. It decouples "this code is merged" from
"this code is running", which for an auth path is exactly the separation you want.

There was also a small commit removing some stale comments about CSRF token handling —
notes I had written to myself that had stopped being true after the state change.
Comments that describe an older version of the code are worse than no comments.

Closing two pull requests

On 12 July I closed #7510,
the lti-advantage gem addition, and
#7632, the subgroups feature.

The gem one was straightforward: I had already reversed that decision in week 4 and
gone direct to ruby-jwt. Leaving the PR open was just noise in the queue. Closing your
own obsolete pull requests is a small courtesy — every open PR is something a maintainer
has to look at at least once to decide it is not for them.

Subgroups was harder, because that one was finished. Data layer, UI, controller, API
endpoints, scoping, tests. Earlier in the week I had added the REST API v1 endpoints and
fixed a bug where a synced user who was also a subgroup mentor got a duplicate
membership row inserted.

But it had the same problem as my original POC: it was a large change to a core model,
arriving as one piece, at a moment when the maintainers' attention on my work was
pointed at LTI. It was not going to get the review it needed, and holding it open would
not change that.

The work is not lost — the branch is intact and the design still holds up. It just is
not the thing I should be spending review capital on right now.

That framing — review capacity as a budget you spend — was new to me this week.
Maintainer attention is the scarcest resource on the project. Every PR I open spends
some, and if I spend it on something that is not on the critical path, the thing that
is on the critical path waits longer.

Opening the first small one

Right at the end of the week I opened
#7647, and it is deliberately
tiny.

It fixes a bug in the existing LTI 1.1 code: when a teacher grades a project in an LTI
session, the score was pushed to the LMS before the grade was saved locally. If the
local save then failed validation, the LMS would show a grade that CircuitVerse had
never stored. The gradebook and the source of truth silently disagree.

It has nothing to do with 1.3. I picked it as the first thing to send precisely because
it is small, self-contained, and improves the integration that already has real users —
and because it demonstrates I can send something a maintainer can review in ten minutes.

Which, after week 3, felt like the point.

Next week: a reviewer finds a bug in it that I had looked straight past, and I cut the
remaining work into twenty-four pull requests.

Community Bonding: Before the First Line of Project Code

Akanksha Trehun — Sat, 08 Aug 2026 02:01:18 +0000

Coding starts on 1 June. This post is about the three weeks before that — what I did
with them, and what I thought the project was going to be, which turned out to be only
partly right.

How I got here

By the time the accepted projects were announced, I had already sent CircuitVerse about
forty pull requests.

That number sounds more impressive than it is. A lot of them were typo fixes in
documentation and specs — genuinely trivial changes, the kind you send while you are
still working out where anything lives. Some were closed. Some are still open.

But they did their job, which was less about the changes themselves than about learning
the shape of the project: how the app is laid out, what the review process feels like,
which parts of the codebase are load-bearing and which are quiet corners nobody has
touched in three years.

The ones I am actually pleased with came later and were small bug fixes rather than
typos — scoping an assignments index to the current user's groups, removing a duplicate
update! call, adding includes(:user) to some notification callbacks that were firing
N+1 queries.

One of them turned out to be a signpost: a partial unique index on
assignments.lti_consumer_key, because LTI 1.1 launches look assignments up by that
column and there was nothing enforcing uniqueness. That was the first time I read
CircuitVerse's LMS integration properly, months before I proposed to rewrite it.

Turning a proposal into issues

The most useful thing I did during bonding was on 25 May, and it produced no code at
all. I broke the project down into four separate feature issues and filed them:

#7405 — upgrade the LMS integration from LTI 1.1 to LTI 1.3
#7406 — roster sync, so a class list flows from the LMS into a CircuitVerse group
#7407 — grade passback via Assignment and Grade Services
#7408 — deep linking, so an instructor can embed specific CircuitVerse content in a course page

A proposal is a document you write for a small audience during a selection process, and
then it mostly sits still. Issues are different. They live in the repository where the
work happens, anyone can comment on them, and they force you to state what you are
actually going to build in terms a maintainer can disagree with before you have built
anything.

Writing them also clarified something for me. In the proposal, "LTI 1.3 support" was one
line item. Split into four issues, the dependency structure became obvious: the launch
has to exist before grades can flow back, because grade passback needs the deployment
and resource-link context that a launch establishes. Roster sync and deep linking both
need the same. #7405 is not one of four features, it is the foundation the other three
stand on.

That ordering shaped everything since.

Reading the code I was proposing to replace

The other substantial piece of bonding was reading CircuitVerse's existing LTI 1.1
integration line by line — the controller, the score submission service, the launch
views, the specs.

I went in expecting to find it neglected, because 1.1 is the old standard and my project
is the new one. That is not what I found. It is compact and it works, and it is used by
real courses today.

I did find things I noted for later. Grades were being pushed to the LMS before being
saved locally, so a failed local save could leave the gradebook holding a score
CircuitVerse never stored. A NameError in a code path that only triggers under
specific conditions. Small gaps, mostly in error handling rather than in the happy path.

Two of them became my first pull requests of the coding period.

Reading it also settled a design question I had been vague about in the proposal.
Nothing about my project requires removing 1.1, and plenty of institutions are still on
it. So the plan became strictly additive: 1.3 arrives alongside 1.1, both work, and
nobody's existing setup breaks. "Backward compatible" stopped being a nice phrase in a
proposal and became a constraint I could check every change against.

The Mergathon

On 26 May, CircuitVerse ran a Mergathon — a coordinated push across all the org's
repositories to deal with the pull request backlog. I spent the day on triage.

I have permissions to label, close and comment, not to merge, so the work was: read
every open PR, work out whether it was still alive, and close the ones that were not
with an explanation.

The numbers were bracing. On the Interactive-Book repository I closed around forty-six
stale pull requests, some open since 2019. Thirteen more on the Vue frontend, five on
the blog. On the mobile app there were ten Dependabot PRs all failing for the same
reason — a dependency requiring a newer Flutter SDK than CI was running — so I traced
the root cause once and commented it on all ten. A handful of others were genuinely
green and just needed someone to say so, which I labelled for a maintainer to merge.

Reading dozens of dead pull requests in one sitting is an unusual way to spend a day and
I learned more from it than from anything else in bonding. Almost none of them died
because the code was wrong. They died from going quiet — the author moved on, the branch
drifted behind master, a maintainer asked a question that was never answered, and then
too much time passed for anyone to want to pick it up.

The pattern I noticed and did not fully absorb: the bigger a pull request was, the more
likely it was to be in that pile. Small ones either merged or got closed quickly. Large
ones lingered until they were unmergeable.

I understood that as an observation about other people's pull requests. It took until
week 3 of the coding period, and having my own large one closed, to understand it was
about mine too.

Setting up something to test against

The last piece was practical: getting an LMS I could actually launch from. Reading the
LTI specification tells you what the protocol says. Watching a real Canvas instance
refuse your launch for reasons the spec does not mention teaches you a different set of
things.

This took longer than expected, which I gather is normal. LTI is a protocol where two
servers have to agree about URLs, keys and identifiers before anything works at all, and
every mismatch surfaces as the same unhelpful failure.

What I expected to be hard

Going into 1 June, I thought the difficulty of this project was the protocol —
signatures, key rotation, the security model, getting the cryptography right.

Some of that has been hard. But the protocol is a written specification with a right
answer; if you read it carefully you converge on it.

The part I did not budget for is the one this whole first stretch has actually turned
on: getting the work into a shape somebody else can be confident about. That is not in
any specification, and it is not something I understood until I had done it wrong.

Next: week 1, and actually writing some code.